diff --git a/weed/command/filer_copy.go b/weed/command/filer_copy.go index 4b814116d..5525ebddd 100644 --- a/weed/command/filer_copy.go +++ b/weed/command/filer_copy.go @@ -491,7 +491,22 @@ func (worker *FileCopyWorker) uploadFileInChunks(task FileCopyTask, f *os.File, return uploadError } - manifestedChunks, manifestErr := filer.MaybeManifestize(worker.saveDataAsChunk, chunks) + // A fold that fails midway aborts the copy, so the blobs its earlier + // batches wrote go the same way as the chunks of a failed upload. + deleteManifestChunks := func(saved []*filer_pb.FileChunk) { + if len(worker.options.masters) == 0 { + return + } + var fileIds []string + for _, chunk := range saved { + fileIds = append(fileIds, chunk.GetFileIdString()) + } + operation.DeleteFileIds(func(_ context.Context) pb.ServerAddress { + return pb.ServerAddress(worker.options.masters[0]) + }, false, worker.options.grpcDialOption, fileIds) + } + + manifestedChunks, manifestErr := filer.MaybeManifestize(worker.saveDataAsChunk, deleteManifestChunks, chunks) if manifestErr != nil { return fmt.Errorf("create manifest: %w", manifestErr) } diff --git a/weed/filer/filechunk_manifest.go b/weed/filer/filechunk_manifest.go index f7c69766a..73dc79751 100644 --- a/weed/filer/filechunk_manifest.go +++ b/weed/filer/filechunk_manifest.go @@ -19,10 +19,6 @@ import ( util_http "github.com/seaweedfs/seaweedfs/weed/util/http" ) -const ( - ManifestBatch = 10000 -) - var bytesBufferPool = sync.Pool{ New: func() interface{} { return new(bytes.Buffer) @@ -228,14 +224,42 @@ func retriedStreamFetchChunkData(ctx context.Context, writer io.Writer, urlStrin } -func MaybeManifestize(saveFunc SaveDataAsChunkFunctionType, inputChunks []*filer_pb.FileChunk) (chunks []*filer_pb.FileChunk, err error) { +// MaybeManifestize folds a flat chunk list into manifest chunks once it passes +// ManifestBatch, so an entry's chunk list stays within what the metadata store +// will hold. A fold that fails partway returns inputChunks unchanged -- never +// the half-folded list, which drops the manifests the caller came in with -- +// and hands the blobs it had already saved to deleteChunks, since the flat list +// it returns references none of them. deleteChunks may be nil where the caller +// has no deleter to offer; then the blobs are only named in the log. +func MaybeManifestize(saveFunc SaveDataAsChunkFunctionType, deleteChunks func([]*filer_pb.FileChunk), inputChunks []*filer_pb.FileChunk) (chunks []*filer_pb.FileChunk, err error) { // Don't manifestize SSE-encrypted chunks to preserve per-chunk metadata for _, chunk := range inputChunks { if chunk.GetSseType() != 0 { // Any SSE type (SSE-C or SSE-KMS) return inputChunks, nil } } - return doMaybeManifestize(saveFunc, inputChunks, ManifestBatch, mergeIntoManifest) + + var saved []*filer_pb.FileChunk + record := func(reader io.Reader, name string, offset int64, tsNs int64, expectedDataSize uint64) (*filer_pb.FileChunk, error) { + chunk, saveErr := saveFunc(reader, name, offset, tsNs, expectedDataSize) + if saveErr == nil { + saved = append(saved, chunk) + } + return chunk, saveErr + } + + chunks, err = doMaybeManifestize(record, inputChunks, ManifestBatch, mergeIntoManifest) + if err == nil { + return chunks, nil + } + if len(saved) > 0 { + if deleteChunks != nil { + deleteChunks(saved) + } else { + glog.V(0).Infof("manifestize failed, %d manifest blobs left unreferenced: %v", len(saved), err) + } + } + return inputChunks, err } func doMaybeManifestize(saveFunc SaveDataAsChunkFunctionType, inputChunks []*filer_pb.FileChunk, mergeFactor int, mergefn func(saveFunc SaveDataAsChunkFunctionType, dataChunks []*filer_pb.FileChunk) (manifestChunk *filer_pb.FileChunk, err error)) (chunks []*filer_pb.FileChunk, err error) { @@ -253,7 +277,9 @@ func doMaybeManifestize(saveFunc SaveDataAsChunkFunctionType, inputChunks []*fil for i := 0; i+mergeFactor <= len(dataChunks); i += mergeFactor { chunk, err := mergefn(saveFunc, dataChunks[i:i+mergeFactor]) if err != nil { - return dataChunks, err + // dataChunks is what is left after the manifests the caller + // already had were separated out; returning it would drop them + return inputChunks, err } chunks = append(chunks, chunk) remaining -= mergeFactor diff --git a/weed/filer/filechunk_manifest_rollback_test.go b/weed/filer/filechunk_manifest_rollback_test.go new file mode 100644 index 000000000..61c899480 --- /dev/null +++ b/weed/filer/filechunk_manifest_rollback_test.go @@ -0,0 +1,147 @@ +package filer + +import ( + "fmt" + "io" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" +) + +func flatTestChunks(n int) []*filer_pb.FileChunk { + chunks := make([]*filer_pb.FileChunk, n) + for i := range chunks { + chunks[i] = &filer_pb.FileChunk{ + FileId: fmt.Sprintf("1,%x", i+1), + Offset: int64(i) * 8, + Size: 8, + } + } + return chunks +} + +type fakeManifestStore struct { + saves int + failOn int // 1-based save call to fail on; 0 = never + deleted []*filer_pb.FileChunk +} + +func (s *fakeManifestStore) save(reader io.Reader, name string, offset int64, tsNs int64, expectedDataSize uint64) (*filer_pb.FileChunk, error) { + s.saves++ + if s.saves == s.failOn { + return nil, fmt.Errorf("save %d failed", s.saves) + } + if _, err := io.Copy(io.Discard, reader); err != nil { + return nil, err + } + return &filer_pb.FileChunk{FileId: fmt.Sprintf("2,%x", s.saves), Offset: offset}, nil +} + +func (s *fakeManifestStore) delete(chunks []*filer_pb.FileChunk) { + s.deleted = append(s.deleted, chunks...) +} + +func TestMaybeManifestizeFolds(t *testing.T) { + store := &fakeManifestStore{} + chunks := flatTestChunks(ManifestBatch + 50) + + result, err := MaybeManifestize(store.save, store.delete, chunks) + if err != nil { + t.Fatalf("MaybeManifestize: %v", err) + } + + manifests, data := SeparateManifestChunks(result) + if len(manifests) != 1 || len(data) != 50 { + t.Fatalf("expected 1 manifest + 50 flat chunks, got %d + %d", len(manifests), len(data)) + } + if store.saves != 1 || len(store.deleted) != 0 { + t.Errorf("expected 1 save and no deletes, got %d saves, %d deleted", store.saves, len(store.deleted)) + } + if manifests[0].Offset != 0 || manifests[0].Size != uint64(ManifestBatch*8) { + t.Errorf("manifest span [%d,%d) wrong", manifests[0].Offset, manifests[0].Size) + } +} + +func TestMaybeManifestizeBelowThreshold(t *testing.T) { + store := &fakeManifestStore{} + chunks := flatTestChunks(ManifestBatch - 1) + + result, err := MaybeManifestize(store.save, store.delete, chunks) + if err != nil { + t.Fatalf("MaybeManifestize: %v", err) + } + if len(result) != len(chunks) || store.saves != 0 { + t.Fatalf("expected untouched flat list, got %d chunks, %d saves", len(result), store.saves) + } +} + +func TestMaybeManifestizeSkipsSse(t *testing.T) { + store := &fakeManifestStore{} + chunks := flatTestChunks(ManifestBatch + 50) + chunks[0].SseType = filer_pb.SSEType_SSE_S3 + + result, err := MaybeManifestize(store.save, store.delete, chunks) + if err != nil { + t.Fatalf("MaybeManifestize: %v", err) + } + if len(result) != len(chunks) || store.saves != 0 { + t.Fatalf("SSE chunks must not be folded, got %d chunks, %d saves", len(result), store.saves) + } +} + +// A fold that fails midway must hand back the caller's own list and delete the +// manifest blobs its earlier batches already uploaded. +func TestMaybeManifestizeRollsBackPartialFold(t *testing.T) { + store := &fakeManifestStore{failOn: 2} + chunks := flatTestChunks(2*ManifestBatch + 50) + + result, err := MaybeManifestize(store.save, store.delete, chunks) + if err == nil { + t.Fatal("expected the failed save to be reported") + } + if len(result) != len(chunks) { + t.Fatalf("expected fallback to %d flat chunks, got %d", len(chunks), len(result)) + } + if HasChunkManifest(result) { + t.Error("fallback list must not contain manifest chunks") + } + if len(store.deleted) != 1 || store.deleted[0].FileId != "2,1" { + t.Fatalf("expected the first saved blob deleted, got %+v", store.deleted) + } +} + +// The manifests a caller came in with are not re-folded, and must survive a +// fold that fails on the flat remainder: returning only the data chunks would +// lose everything they cover. +func TestMaybeManifestizeKeepsExistingManifestsOnFailure(t *testing.T) { + store := &fakeManifestStore{failOn: 2} + existing := &filer_pb.FileChunk{FileId: "m,1", IsChunkManifest: true, Offset: 0, Size: 24} + chunks := append([]*filer_pb.FileChunk{existing}, flatTestChunks(2*ManifestBatch+50)...) + + result, _ := MaybeManifestize(store.save, store.delete, chunks) + + if len(result) != len(chunks) { + t.Fatalf("expected original %d chunks, got %d", len(chunks), len(result)) + } + if result[0].FileId != "m,1" || !result[0].IsChunkManifest { + t.Fatalf("existing manifest dropped: %+v", result[0]) + } + if len(store.deleted) != 1 || store.deleted[0].FileId != "2,1" { + t.Fatalf("expected the first saved blob deleted, got %+v", store.deleted) + } +} + +// Without a deleter the fold still falls back to the caller's list; the blobs +// it saved are only reported. +func TestMaybeManifestizeRollbackWithoutDeleter(t *testing.T) { + store := &fakeManifestStore{failOn: 2} + chunks := flatTestChunks(2*ManifestBatch + 50) + + result, err := MaybeManifestize(store.save, nil, chunks) + if err == nil { + t.Fatal("expected the failed save to be reported") + } + if len(result) != len(chunks) || HasChunkManifest(result) { + t.Fatalf("expected fallback to %d flat chunks, got %d", len(chunks), len(result)) + } +} diff --git a/weed/filer/foundationdb/foundationdb_store.go b/weed/filer/foundationdb/foundationdb_store.go index b413d260f..52c74677a 100644 --- a/weed/filer/foundationdb/foundationdb_store.go +++ b/weed/filer/foundationdb/foundationdb_store.go @@ -35,8 +35,6 @@ import ( ) const ( - // FoundationDB transaction size limit is 10MB - FDB_TRANSACTION_SIZE_LIMIT = 10 * 1024 * 1024 // Safe limit for batch size (leave margin for FDB overhead) FDB_BATCH_SIZE_LIMIT = 8 * 1024 * 1024 // Maximum number of entries to return in a single directory listing @@ -396,10 +394,8 @@ 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) + if err := errIfValueTooLarge(string(entry.FullPath), value); err != nil { + return err } // Check if there's a transaction in context @@ -696,6 +692,10 @@ func (store *FoundationDBStore) ListDirectoryPrefixedEntries(ctx context.Context func (store *FoundationDBStore) KvPut(ctx context.Context, key []byte, value []byte) error { fdbKey := store.kvDir.Pack(tuple.Tuple{key}) + if err := errIfValueTooLarge(string(key), value); err != nil { + return err + } + // Check if there's a transaction in context if tx, exists := store.getTransactionFromContext(ctx); exists { tx.Set(fdbKey, value) diff --git a/weed/filer/foundationdb/value_size.go b/weed/filer/foundationdb/value_size.go new file mode 100644 index 000000000..7e07234d6 --- /dev/null +++ b/weed/filer/foundationdb/value_size.go @@ -0,0 +1,16 @@ +package foundationdb + +import "fmt" + +// FDB_VALUE_SIZE_LIMIT is Apple's documented maximum value size: 100,000 +// bytes, not 100 KiB. A value over it is rejected by FoundationDB itself with +// error 2103, so the store checks it and names it. +const FDB_VALUE_SIZE_LIMIT = 100 * 1000 + +func errIfValueTooLarge(name string, value []byte) error { + if len(value) > FDB_VALUE_SIZE_LIMIT { + return fmt.Errorf("entry %s exceeds FoundationDB value size limit (%d > %d bytes)", + name, len(value), FDB_VALUE_SIZE_LIMIT) + } + return nil +} diff --git a/weed/filer/foundationdb/value_size_test.go b/weed/filer/foundationdb/value_size_test.go new file mode 100644 index 000000000..27256cf38 --- /dev/null +++ b/weed/filer/foundationdb/value_size_test.go @@ -0,0 +1,32 @@ +package foundationdb + +import ( + "strings" + "testing" +) + +func TestFDBValueSizeLimitConstant(t *testing.T) { + if FDB_VALUE_SIZE_LIMIT != 100*1000 { + t.Fatalf("FDB_VALUE_SIZE_LIMIT = %d, want 100000", FDB_VALUE_SIZE_LIMIT) + } +} + +func TestErrIfValueTooLarge(t *testing.T) { + if err := errIfValueTooLarge("/p", make([]byte, FDB_VALUE_SIZE_LIMIT)); err != nil { + t.Fatalf("100000-byte value should pass, got %v", err) + } + err := errIfValueTooLarge("/buckets/b/o", make([]byte, FDB_VALUE_SIZE_LIMIT+1)) + if err == nil { + t.Fatal("100001-byte value should fail") + } + msg := err.Error() + if !strings.Contains(msg, "value size limit") { + t.Fatalf("error should name value size limit, got %q", msg) + } + if strings.Contains(msg, "transaction size limit") { + t.Fatalf("error must not mention transaction size limit, got %q", msg) + } + if !strings.Contains(msg, "100001") || !strings.Contains(msg, "100000") { + t.Fatalf("error should include both sizes, got %q", msg) + } +} diff --git a/weed/filer/manifest_batch.go b/weed/filer/manifest_batch.go new file mode 100644 index 000000000..95c36aba3 --- /dev/null +++ b/weed/filer/manifest_batch.go @@ -0,0 +1,10 @@ +//go:build !foundationdb + +package filer + +// ManifestBatch is how many data chunks are folded into one manifest chunk, so +// that an entry's chunk list stays within the largest value its metadata store +// will accept. Stores reached by this build hold values far larger than the +// list 10000 chunks encode to; see manifest_batch_foundationdb.go for the +// FoundationDB build, whose values stop at 100,000 bytes. +const ManifestBatch = 10000 diff --git a/weed/filer/manifest_batch_foundationdb.go b/weed/filer/manifest_batch_foundationdb.go new file mode 100644 index 000000000..7b39795dc --- /dev/null +++ b/weed/filer/manifest_batch_foundationdb.go @@ -0,0 +1,24 @@ +//go:build foundationdb + +package filer + +// ManifestBatch is how many data chunks are folded into one manifest chunk. A +// FoundationDB value stops at 100,000 bytes (foundationdb.FDB_VALUE_SIZE_LIMIT) +// and an entry's whole chunk list is one value, which at ~100 bytes per chunk +// record is about 1000 chunks -- so folding at 10000 never ran before the write +// was already too large to store. +// +// A single fold level leaves (chunks/batch) manifest pointers plus up to +// (batch-1) unfolded chunks in the entry, so the reachable chunk count is +// highest when the two terms are near equal. 500 is that optimum for a +// 100,000-byte budget: it keeps an entry inside the limit up to ~250,000 +// chunks, ~1 TB at the default -maxMB=4, against ~4 GB before. Larger files +// need nested packing, which no batch size substitutes for. +// +// This is a property of the build rather than of the configured store: the +// FoundationDB image builds one binary for the filer and for every client that +// folds (S3, mount, WebDAV, weed shell, filer.copy), and each of them has to +// agree on a list the filer's store can hold. A binary built with this tag but +// pointed at another store folds earlier than that store requires, which costs +// one manifest blob per 500 chunks and one read to resolve it. +const ManifestBatch = 500 diff --git a/weed/mount/weedfs_file_sync.go b/weed/mount/weedfs_file_sync.go index 2e7790b6a..834e3827c 100644 --- a/weed/mount/weedfs_file_sync.go +++ b/weed/mount/weedfs_file_sync.go @@ -247,7 +247,8 @@ func (wfs *WFS) flushMetadataToFiler(ctx context.Context, fh *FileHandle, dir, n manifestChunks = nil } - chunks, manifestErr := filer.MaybeManifestize(wfs.saveDataAsChunk(fileFullPath), chunks) + // no chunk deleter here: a failed fold reports the blobs it saved + chunks, manifestErr := filer.MaybeManifestize(wfs.saveDataAsChunk(fileFullPath), nil, chunks) if manifestErr != nil { // not good, but should be ok glog.V(0).Infof("MaybeManifestize: %v", manifestErr) diff --git a/weed/mount/weedfs_metadata_flush.go b/weed/mount/weedfs_metadata_flush.go index 3ff2fa9f7..2a4f3eee9 100644 --- a/weed/mount/weedfs_metadata_flush.go +++ b/weed/mount/weedfs_metadata_flush.go @@ -139,7 +139,8 @@ func (wfs *WFS) flushFileMetadata(fh *FileHandle) error { compactedChunks, _ := filer.CompactFileChunks(context.Background(), wfs.LookupFn(), nonManifestChunks) // Try to create manifest chunks for large files - compactedChunks, manifestErr := filer.MaybeManifestize(wfs.saveDataAsChunk(fileFullPath), compactedChunks) + // no chunk deleter here: a failed fold reports the blobs it saved + compactedChunks, manifestErr := filer.MaybeManifestize(wfs.saveDataAsChunk(fileFullPath), nil, compactedChunks) if manifestErr != nil { glog.V(0).Infof("flushFileMetadata MaybeManifestize: %v", manifestErr) } diff --git a/weed/s3api/s3api_chunk_manifest.go b/weed/s3api/s3api_chunk_manifest.go index d8eaaa9e8..c60d39ed7 100644 --- a/weed/s3api/s3api_chunk_manifest.go +++ b/weed/s3api/s3api_chunk_manifest.go @@ -33,29 +33,13 @@ func (s3a *S3ApiServer) saveManifestChunk(filePath string, bucket string, ttlSec } // manifestizeChunks folds a large flat chunk list into manifest chunks. A -// failed fold falls back to the flat list, deleting any blobs it saved. +// failed fold falls back to the flat list, which is still a correct object. func (s3a *S3ApiServer) manifestizeChunks(filePath string, bucket string, ttlSec int32, chunks []*filer_pb.FileChunk) []*filer_pb.FileChunk { - return manifestizeOrKeepFlat(s3a.saveManifestChunk(filePath, bucket, ttlSec), s3a.deleteOrphanedChunks, filePath, chunks) -} - -func manifestizeOrKeepFlat(save filer.SaveDataAsChunkFunctionType, deleteChunks func([]*filer_pb.FileChunk), filePath string, chunks []*filer_pb.FileChunk) []*filer_pb.FileChunk { - var saved []*filer_pb.FileChunk - record := func(reader io.Reader, name string, offset int64, tsNs int64, expectedDataSize uint64) (*filer_pb.FileChunk, error) { - chunk, err := save(reader, name, offset, tsNs, expectedDataSize) - if err == nil { - saved = append(saved, chunk) - } - return chunk, err - } - manifested, err := filer.MaybeManifestize(record, chunks) + folded, err := filer.MaybeManifestize(s3a.saveManifestChunk(filePath, bucket, ttlSec), s3a.deleteOrphanedChunks, chunks) if err != nil { glog.V(0).Infof("MaybeManifestize %s: %v", filePath, err) - if len(saved) > 0 { - deleteChunks(saved) - } - return chunks } - return manifested + return folded } // flattenManifestChunks resolves manifest chunks into flat data chunks, diff --git a/weed/s3api/s3api_chunk_manifest_test.go b/weed/s3api/s3api_chunk_manifest_test.go index 8c3f373f5..5035b5278 100644 --- a/weed/s3api/s3api_chunk_manifest_test.go +++ b/weed/s3api/s3api_chunk_manifest_test.go @@ -1,107 +1,11 @@ package s3api import ( - "fmt" - "io" "testing" - "github.com/seaweedfs/seaweedfs/weed/filer" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" ) -func flatChunks(n int) []*filer_pb.FileChunk { - chunks := make([]*filer_pb.FileChunk, n) - for i := range chunks { - chunks[i] = &filer_pb.FileChunk{ - FileId: fmt.Sprintf("1,%x", i+1), - Offset: int64(i) * 8, - Size: 8, - } - } - return chunks -} - -type fakeManifestStore struct { - saves int - failOn int // 1-based save call to fail on; 0 = never - deleted []*filer_pb.FileChunk -} - -func (s *fakeManifestStore) save(reader io.Reader, name string, offset int64, tsNs int64, expectedDataSize uint64) (*filer_pb.FileChunk, error) { - s.saves++ - if s.saves == s.failOn { - return nil, fmt.Errorf("save %d failed", s.saves) - } - if _, err := io.Copy(io.Discard, reader); err != nil { - return nil, err - } - return &filer_pb.FileChunk{FileId: fmt.Sprintf("2,%x", s.saves), Offset: offset}, nil -} - -func (s *fakeManifestStore) delete(chunks []*filer_pb.FileChunk) { - s.deleted = append(s.deleted, chunks...) -} - -func TestManifestizeOrKeepFlatFolds(t *testing.T) { - store := &fakeManifestStore{} - chunks := flatChunks(filer.ManifestBatch + 50) - - result := manifestizeOrKeepFlat(store.save, store.delete, "/buckets/b/o", chunks) - - manifests, data := filer.SeparateManifestChunks(result) - if len(manifests) != 1 || len(data) != 50 { - t.Fatalf("expected 1 manifest + 50 flat chunks, got %d + %d", len(manifests), len(data)) - } - if store.saves != 1 || len(store.deleted) != 0 { - t.Errorf("expected 1 save and no deletes, got %d saves, %d deleted", store.saves, len(store.deleted)) - } - if manifests[0].Offset != 0 || manifests[0].Size != uint64(filer.ManifestBatch*8) { - t.Errorf("manifest span [%d,%d) wrong", manifests[0].Offset, manifests[0].Size) - } -} - -func TestManifestizeOrKeepFlatBelowThreshold(t *testing.T) { - store := &fakeManifestStore{} - chunks := flatChunks(filer.ManifestBatch - 1) - - result := manifestizeOrKeepFlat(store.save, store.delete, "/buckets/b/o", chunks) - - if len(result) != len(chunks) || store.saves != 0 { - t.Fatalf("expected untouched flat list, got %d chunks, %d saves", len(result), store.saves) - } -} - -func TestManifestizeOrKeepFlatSkipsSse(t *testing.T) { - store := &fakeManifestStore{} - chunks := flatChunks(filer.ManifestBatch + 50) - chunks[0].SseType = filer_pb.SSEType_SSE_S3 - - result := manifestizeOrKeepFlat(store.save, store.delete, "/buckets/b/o", chunks) - - if len(result) != len(chunks) || store.saves != 0 { - t.Fatalf("SSE chunks must not be folded, got %d chunks, %d saves", len(result), store.saves) - } -} - -// A fold that fails midway must fall back to the flat list and delete the -// manifest blobs its earlier batches already uploaded. -func TestManifestizeOrKeepFlatRollsBackPartialFold(t *testing.T) { - store := &fakeManifestStore{failOn: 2} - chunks := flatChunks(2*filer.ManifestBatch + 50) - - result := manifestizeOrKeepFlat(store.save, store.delete, "/buckets/b/o", chunks) - - if len(result) != len(chunks) { - t.Fatalf("expected fallback to %d flat chunks, got %d", len(chunks), len(result)) - } - if filer.HasChunkManifest(result) { - t.Error("fallback list must not contain manifest chunks") - } - if len(store.deleted) != 1 || store.deleted[0].FileId != "2,1" { - t.Fatalf("expected the first saved blob deleted, got %+v", store.deleted) - } -} - func TestPartRange(t *testing.T) { chunks := []*filer_pb.FileChunk{ {FileId: "1,a", Offset: 0, Size: 8}, diff --git a/weed/server/filer_grpc_server.go b/weed/server/filer_grpc_server.go index b86d82475..b0cf88345 100644 --- a/weed/server/filer_grpc_server.go +++ b/weed/server/filer_grpc_server.go @@ -769,11 +769,13 @@ func (fs *FilerServer) cleanupChunks(ctx context.Context, fullpath string, exist "", "", ) // ignore readonly error for capacity needed to manifestize - chunks, err = filer.MaybeManifestize(fs.saveAsChunk(ctx, so), chunks) - if err != nil { - // not good, but should be ok - glog.V(0).InfofCtx(ctx, "MaybeManifestize: %v", err) + // A failed fold leaves a flat list, which is still a correct entry -- + // and the store rejects it by name if it cannot hold one this size. + folded, manifestErr := filer.MaybeManifestize(fs.saveAsChunk(ctx, so), fs.filer.DeleteChunksNotRecursive, chunks) + if manifestErr != nil { + glog.V(0).InfofCtx(ctx, "MaybeManifestize %s: %v", fullpath, manifestErr) } + chunks = folded } chunks = append(manifestChunks, chunks...) @@ -824,10 +826,10 @@ func (fs *FilerServer) AppendToEntry(ctx context.Context, req *filer_pb.AppendTo glog.WarningfCtx(ctx, "applyStorageDefaultsToEntry: %v", err) return &filer_pb.AppendToEntryResponse{}, err } - entry.Chunks, err = filer.MaybeManifestize(fs.saveAsChunk(ctx, so), entry.GetChunks()) + entry.Chunks, err = filer.MaybeManifestize(fs.saveAsChunk(ctx, so), fs.filer.DeleteChunksNotRecursive, entry.GetChunks()) if err != nil { - // not good, but should be ok - glog.V(0).InfofCtx(ctx, "MaybeManifestize: %v", err) + // the append is still correct with the flat list + glog.V(0).InfofCtx(ctx, "MaybeManifestize %s: %v", fullpath, err) } err = fs.filer.CreateEntry(context.Background(), entry, nil, false, false, nil, false, fs.filer.MaxFilenameLength) diff --git a/weed/server/filer_server_handlers_write_autochunk.go b/weed/server/filer_server_handlers_write_autochunk.go index 34a70d3ef..28b14893e 100644 --- a/weed/server/filer_server_handlers_write_autochunk.go +++ b/weed/server/filer_server_handlers_write_autochunk.go @@ -306,7 +306,7 @@ func (fs *FilerServer) saveMetaData(ctx context.Context, r *http.Request, fileNa } // maybe compact entry chunks - mergedChunks, replyerr = filer.MaybeManifestize(fs.saveAsChunk(ctx, so), mergedChunks) + mergedChunks, replyerr = filer.MaybeManifestize(fs.saveAsChunk(ctx, so), fs.filer.DeleteChunksNotRecursive, mergedChunks) if replyerr != nil { glog.V(0).InfofCtx(ctx, "manifestize %s: %v", r.RequestURI, replyerr) return diff --git a/weed/server/webdav_server.go b/weed/server/webdav_server.go index 31d138508..7acac1d2f 100644 --- a/weed/server/webdav_server.go +++ b/weed/server/webdav_server.go @@ -495,7 +495,8 @@ func (f *WebDavFile) Write(buf []byte) (int, error) { } f.bufWriter.CloseFunc = func() error { - manifestedChunks, manifestErr := filer.MaybeManifestize(f.saveDataAsChunk, f.entry.GetChunks()) + // no chunk deleter here: a failed fold reports the blobs it saved + manifestedChunks, manifestErr := filer.MaybeManifestize(f.saveDataAsChunk, nil, f.entry.GetChunks()) if manifestErr != nil { // not good, but should be ok glog.V(0).Infof("file %s close MaybeManifestize: %v", f.name, manifestErr) diff --git a/weed/shell/command_fs_distribute_chunks.go b/weed/shell/command_fs_distribute_chunks.go index 96baaa740..17c6f4fdd 100644 --- a/weed/shell/command_fs_distribute_chunks.go +++ b/weed/shell/command_fs_distribute_chunks.go @@ -191,7 +191,8 @@ func (c *commandFsDistributeChunks) Do(args []string, commandEnv *CommandEnv, wr finalChunks := chunks if hadManifest { - remanifested, manErr := filer.MaybeManifestize(newShellSaveAsChunk(commandEnv), chunks) + // no chunk deleter here: a failed fold reports the blobs it saved + remanifested, manErr := filer.MaybeManifestize(newShellSaveAsChunk(commandEnv), nil, chunks) if manErr != nil { fmt.Fprintf(writer, "WARNING: re-manifestize failed: %v. Writing flat chunk list.\n", manErr) } else {