From ee61451fb8f4943628b331bbc6bcb3f00f17351a Mon Sep 17 00:00:00 2001 From: chrislu Date: Thu, 6 Nov 2025 18:31:50 -0800 Subject: [PATCH] address comments --- go.mod | 4 +- weed/filer/foundationdb/foundationdb_store.go | 41 +++++++++++++++---- .../foundationdb/foundationdb_store_test.go | 14 ++----- 3 files changed, 37 insertions(+), 22 deletions(-) diff --git a/go.mod b/go.mod index 2dbad6035..1e09c9106 100644 --- a/go.mod +++ b/go.mod @@ -123,7 +123,7 @@ require ( github.com/Jille/raft-grpc-transport v1.6.1 github.com/ThreeDotsLabs/watermill v1.5.1 github.com/a-h/templ v0.3.943 - github.com/apple/foundationdb/bindings/go v0.0.0-20250828195015-ba4c89167099 + github.com/apple/foundationdb/bindings/go v0.0.0-20240515141816-262c6fe778ad github.com/arangodb/go-driver v1.6.7 github.com/armon/go-metrics v0.4.1 github.com/aws/aws-sdk-go-v2 v1.39.5 @@ -466,5 +466,3 @@ require ( ) // replace github.com/seaweedfs/raft => /Users/chrislu/go/src/github.com/seaweedfs/raft - -replace github.com/apple/foundationdb/bindings/go v0.0.0-20250828195015-ba4c89167099 => github.com/apple/foundationdb/bindings/go v0.0.0-20240515141816-262c6fe778ad diff --git a/weed/filer/foundationdb/foundationdb_store.go b/weed/filer/foundationdb/foundationdb_store.go index a9f3f0054..d9c2925ee 100644 --- a/weed/filer/foundationdb/foundationdb_store.go +++ b/weed/filer/foundationdb/foundationdb_store.go @@ -4,6 +4,7 @@ package foundationdb import ( + "bytes" "context" "fmt" "strings" @@ -20,7 +21,6 @@ import ( ) const ( - DIR_FILE_SEPARATOR = byte(0x00) // FoundationDB transaction size limit is 10MB FDB_TRANSACTION_SIZE_LIMIT = 10 * 1024 * 1024 ) @@ -94,7 +94,7 @@ func (store *FoundationDBStore) GetName() string { func (store *FoundationDBStore) Initialize(configuration util.Configuration, prefix string) error { // Set default configuration values configuration.SetDefault(prefix+"cluster_file", "/etc/foundationdb/fdb.cluster") - configuration.SetDefault(prefix+"api_version", 630) + configuration.SetDefault(prefix+"api_version", 740) configuration.SetDefault(prefix+"timeout", "5s") configuration.SetDefault(prefix+"max_retry_delay", "1s") configuration.SetDefault(prefix+"directory_prefix", "seaweedfs") @@ -210,6 +210,12 @@ func (store *FoundationDBStore) UpdateEntry(ctx context.Context, entry *filer.En value = util.MaybeGzipData(value) } + // Check transaction size limit + if len(value) > FDB_TRANSACTION_SIZE_LIMIT { + return fmt.Errorf("entry %s exceeds FoundationDB transaction size limit (%d > %d bytes)", + entry.FullPath, len(value), FDB_TRANSACTION_SIZE_LIMIT) + } + // Check if there's a transaction in context if tx, exists := store.getTransactionFromContext(ctx); exists { tx.Set(key, value) @@ -323,25 +329,43 @@ func (store *FoundationDBStore) ListDirectoryPrefixedEntries(ctx context.Context limit = 1000 } + // Construct tight range based on directory and prefix directoryPrefix := store.genDirectoryKeyPrefix(string(dirPath), prefix) - startKey := store.genDirectoryKeyPrefix(string(dirPath), startFileName) - if !includeStartFile { - startKey = append(startKey, 0x00) + // Determine the actual start key + var startKey fdb.Key + if startFileName != "" { + // Start from the specified file within the prefix range + startKey = store.genDirectoryKeyPrefix(string(dirPath), startFileName) + if !includeStartFile { + startKey = append(startKey, 0x00) + } + // Ensure startKey is within the prefix range + if !bytes.HasPrefix(startKey, directoryPrefix) { + // If startFileName is before the prefix, start at prefix + if bytes.Compare(startKey, directoryPrefix) < 0 { + startKey = directoryPrefix + } + } + } else { + startKey = directoryPrefix } + // End key is the prefix range end + endKey := prefixEndFallback(directoryPrefix) + var kvs []fdb.KeyValue var rangeErr error // Check if there's a transaction in context if tx, exists := store.getTransactionFromContext(ctx); exists { - kr := fdb.KeyRange{Begin: fdb.Key(startKey), End: prefixEndFallback(directoryPrefix)} + kr := fdb.KeyRange{Begin: startKey, End: endKey} kvs, rangeErr = tx.GetRange(kr, fdb.RangeOptions{Limit: int(limit)}).GetSliceWithError() if rangeErr != nil { return "", fmt.Errorf("scanning %s: %v", dirPath, rangeErr) } } else { result, err := store.database.ReadTransact(func(rtr fdb.ReadTransaction) (interface{}, error) { - kr := fdb.KeyRange{Begin: fdb.Key(startKey), End: prefixEndFallback(directoryPrefix)} + kr := fdb.KeyRange{Begin: startKey, End: endKey} kvSlice, err := rtr.GetRange(kr, fdb.RangeOptions{Limit: int(limit)}).GetSliceWithError() if err != nil { return nil, err @@ -360,7 +384,8 @@ func (store *FoundationDBStore) ListDirectoryPrefixedEntries(ctx context.Context continue } - if !strings.HasPrefix(fileName, prefix) { + // Prefix filtering is now handled by the range scan, but double-check for safety + if prefix != "" && !strings.HasPrefix(fileName, prefix) { continue } diff --git a/weed/filer/foundationdb/foundationdb_store_test.go b/weed/filer/foundationdb/foundationdb_store_test.go index 431f17f63..69bc0c156 100644 --- a/weed/filer/foundationdb/foundationdb_store_test.go +++ b/weed/filer/foundationdb/foundationdb_store_test.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "os" + "strings" "testing" "time" @@ -365,7 +366,7 @@ func createBenchmarkStore(b *testing.B) *FoundationDBStore { } store := &FoundationDBStore{} - err := store.initialize(clusterFile, 720) + err := store.initialize(clusterFile, 740) if err != nil { b.Skipf("Failed to initialize FoundationDB store: %v", err) } @@ -374,14 +375,5 @@ func createBenchmarkStore(b *testing.B) *FoundationDBStore { } func containsString(s, substr string) bool { - return len(s) >= len(substr) && (s == substr || len(substr) == 0 || - (len(s) > len(substr) && (s[:len(substr)] == substr || s[len(s)-len(substr):] == substr || - func() bool { - for i := 0; i <= len(s)-len(substr); i++ { - if s[i:i+len(substr)] == substr { - return true - } - } - return false - }()))) + return strings.Contains(s, substr) }