Skip to content

Commit 55c890f

Browse files
committed
resilver: relocate chunked objects (major upd)
* chunk manifest: add Relocate() method; add internal types and helpers - step-wise impl. with rollback/cleanup upon any error * chunk manifest: consolidate validation; add validation: - revise/consolidate Check(completed); add ValidateLocations(completed) - GetChunk(): locking is a caller's resp. - storeCompleted(): allow (resilvering caller) to overwrite completed manifest - consistently use firstChunk(); add (and use) chunk1Path() -------- * resilver: - RunResilver() now uses manifest.Relocate() to relocate chunks -------- * tests: add TestUfestRelocate_ChunkedOne and helpers -------- * part three, prev. commit: fe9033b Signed-off-by: Alex Aizman <alex.aizman@gmail.com>
1 parent fe9033b commit 55c890f

13 files changed

Lines changed: 577 additions & 189 deletions

‎ais/s3/mpt.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,10 @@ func ListParts(manifest *core.Ufest) (parts []types.CompletedPart, ecode int, er
7474
manifest.Lock()
7575
parts = make([]types.CompletedPart, 0, manifest.Count())
7676
for i := range manifest.Count() {
77-
c := manifest.GetChunk(i+1, true)
77+
c, err := manifest.GetChunk(i + 1)
78+
if err != nil {
79+
return nil, http.StatusNotFound, err
80+
}
7881
etag := c.ETag
7982
if etag == "" {
8083
if c.MD5 != nil {

‎ais/tgtmpt.go‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ func (*ups) parsePartNum(s string) (int32, error) {
186186

187187
// (under manifest lock)
188188
func validateChecksumEtag(lom *core.LOM, manifest *core.Ufest, parts apc.MptCompletedParts, isS3 bool) (string, error) {
189-
if err := manifest.Check(); err != nil {
189+
if err := manifest.Check(true /*completed*/); err != nil {
190190
return "", err
191191
}
192192
if _, err := _checkParts(parts, manifest.Count()); err != nil {
@@ -211,13 +211,6 @@ func validateChecksumEtag(lom *core.LOM, manifest *core.Ufest, parts apc.MptComp
211211
if remote /*computed by remote*/ || !isS3 /*native caller*/ {
212212
return "" /*etag*/, nil
213213
}
214-
215-
// NOTE [convention]
216-
// for chunks `isS3` overrides bucket-configured checksum always requiring MD5
217-
// (see putPart)
218-
219-
debug.Assert(manifest.Count() == 0 || len(manifest.GetChunk(1, true).MD5) == cos.LenMD5Hash)
220-
221214
return manifest.ETagS3()
222215
}
223216

@@ -240,7 +233,7 @@ func _checkParts(parts apc.MptCompletedParts, count int) (int, error) {
240233
}
241234
return 0, nil
242235

243-
slow:
236+
slow: // slow path (same +sort)
244237
sort.Slice(parts, func(i, j int) bool {
245238
return parts[i].PartNumber < parts[j].PartNumber
246239
})
@@ -508,7 +501,9 @@ func (ups *ups) complete(args *completeArgs) (string, int, error) {
508501
if lom.Bck().IsRemoteS3() || lom.Bck().IsRemoteOCI() || lom.Bck().IsRemoteGCP() {
509502
for i := range args.parts {
510503
if args.parts[i].ETag == "" {
511-
args.parts[i].ETag = manifest.GetChunk(args.parts[i].PartNumber, true).ETag
504+
c, err := manifest.GetChunk(args.parts[i].PartNumber)
505+
debug.AssertNoErr(err)
506+
args.parts[i].ETag = c.ETag
512507
}
513508
}
514509
}

‎ais/tgts3mpt.go‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -317,11 +317,8 @@ func (t *target) getPartMptS3(w http.ResponseWriter, r *http.Request, bck *meta.
317317
}
318318

319319
// get specific chunk
320-
manifest.Lock()
321-
chunk := manifest.GetChunk(int(partNum), true)
322-
manifest.Unlock()
323-
if chunk == nil {
324-
err := fmt.Errorf("part %d not found", partNum)
320+
chunk, err := manifest.GetChunk(int(partNum))
321+
if err != nil {
325322
s3.WriteErr(w, r, err, http.StatusNotFound)
326323
return
327324
}

‎core/lchunk_test.go‎

Lines changed: 22 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,8 @@ var _ = Describe("Ufest Core Functionality", func() {
116116
Expect(err).NotTo(HaveOccurred())
117117

118118
// Verify chunk was added through public interface
119-
retrievedChunk := manifest.GetChunk(1, false)
119+
retrievedChunk, err := manifest.GetChunk(1)
120+
Expect(err).NotTo(HaveOccurred())
120121
Expect(retrievedChunk).NotTo(BeNil())
121122
Expect(retrievedChunk.Path()).To(Equal(chunk.Path()))
122123
Expect(retrievedChunk.Size()).To(Equal(chunkSize))
@@ -144,7 +145,8 @@ var _ = Describe("Ufest Core Functionality", func() {
144145

145146
// Verify all chunks are accessible
146147
for _, c := range chunks {
147-
retrievedChunk := manifest.GetChunk(int(c.num), false)
148+
retrievedChunk, err := manifest.GetChunk(int(c.num))
149+
Expect(err).NotTo(HaveOccurred())
148150
Expect(retrievedChunk).NotTo(BeNil())
149151
Expect(retrievedChunk.Size()).To(Equal(c.size))
150152
}
@@ -170,9 +172,12 @@ var _ = Describe("Ufest Core Functionality", func() {
170172
}
171173

172174
// Verify chunks are retrievable by number
173-
chunk1 := manifest.GetChunk(1, false)
174-
chunk2 := manifest.GetChunk(2, false)
175-
chunk3 := manifest.GetChunk(3, false)
175+
chunk1, err := manifest.GetChunk(1)
176+
Expect(err).NotTo(HaveOccurred())
177+
chunk2, err := manifest.GetChunk(2)
178+
Expect(err).NotTo(HaveOccurred())
179+
chunk3, err := manifest.GetChunk(3)
180+
Expect(err).NotTo(HaveOccurred())
176181

177182
Expect(chunk1).NotTo(BeNil())
178183
Expect(chunk2).NotTo(BeNil())
@@ -197,7 +202,8 @@ var _ = Describe("Ufest Core Functionality", func() {
197202
Expect(err).NotTo(HaveOccurred())
198203

199204
// Verify replacement
200-
retrievedChunk := manifest.GetChunk(1, false)
205+
retrievedChunk, err := manifest.GetChunk(1)
206+
Expect(err).NotTo(HaveOccurred())
201207
Expect(retrievedChunk).NotTo(BeNil())
202208
Expect(retrievedChunk.Path()).To(Equal(replacementChunk.Path()))
203209
Expect(retrievedChunk.Size()).To(Equal(int64(2 * cos.MiB)))
@@ -235,7 +241,8 @@ var _ = Describe("Ufest Core Functionality", func() {
235241

236242
// Verify all chunks were added
237243
for i := 1; i <= numWorkers; i++ {
238-
chunk := manifest.GetChunk(i, false)
244+
chunk, err := manifest.GetChunk(i)
245+
Expect(err).NotTo(HaveOccurred())
239246
Expect(chunk).NotTo(BeNil())
240247
Expect(chunk.Num()).To(Equal(uint16(i)))
241248
}
@@ -264,7 +271,8 @@ var _ = Describe("Ufest Core Functionality", func() {
264271
})
265272

266273
It("should retrieve existing chunks", func() {
267-
chunk := manifest.GetChunk(2, false)
274+
chunk, err := manifest.GetChunk(2)
275+
Expect(err).NotTo(HaveOccurred())
268276
Expect(chunk).NotTo(BeNil())
269277
Expect(chunk.Num()).To(Equal(uint16(2)))
270278

@@ -274,19 +282,10 @@ var _ = Describe("Ufest Core Functionality", func() {
274282
})
275283

276284
It("should return nil for non-existent chunks", func() {
277-
chunk := manifest.GetChunk(99, false)
285+
chunk, err := manifest.GetChunk(99)
286+
Expect(err).To(HaveOccurred())
278287
Expect(chunk).To(BeNil())
279288
})
280-
281-
It("should respect locking parameter", func() {
282-
// Test both locked and unlocked access
283-
chunk1 := manifest.GetChunk(1, false) // unlocked
284-
chunk2 := manifest.GetChunk(1, true) // locked
285-
286-
Expect(chunk1).NotTo(BeNil())
287-
Expect(chunk2).NotTo(BeNil())
288-
Expect(chunk1.Num()).To(Equal(chunk2.Num()))
289-
})
290289
})
291290

292291
Describe("NewChunk Method - HRW Distribution", func() {
@@ -483,8 +482,10 @@ var _ = Describe("Ufest Core Functionality", func() {
483482
Expect(loaded.Count()).To(Equal(len(sizes)))
484483
Expect(loaded.Size()).To(Equal(u.Size()))
485484
for i := 1; i <= len(sizes); i++ {
486-
oc := u.GetChunk(i, false)
487-
lc := loaded.GetChunk(i, false)
485+
oc, err := u.GetChunk(i)
486+
Expect(err).NotTo(HaveOccurred())
487+
lc, err := loaded.GetChunk(i)
488+
Expect(err).NotTo(HaveOccurred())
488489
Expect(lc).NotTo(BeNil())
489490
Expect(lc.Num()).To(Equal(oc.Num()))
490491
Expect(lc.Size()).To(Equal(oc.Size()))
@@ -584,29 +585,6 @@ var _ = Describe("Ufest Core Functionality", func() {
584585

585586
})
586587

587-
Describe("Locking Behavior", func() {
588-
It("should provide proper locking mechanisms", func() {
589-
testObjectName := "test-objects/lock-test.bin"
590-
localFQN := mix.MakePathFQN(&localBck, fs.ObjCT, testObjectName)
591-
lom := newBasicLom(localFQN, cos.MiB)
592-
manifest, err := core.NewUfest("test-lock-123", lom, false)
593-
Expect(err).NotTo(HaveOccurred())
594-
595-
// Should be able to use locked operations
596-
chunk, err := manifest.NewChunk(1, lom)
597-
Expect(err).NotTo(HaveOccurred())
598-
chunk.SetCksum(cos.NewCksum(cos.ChecksumOneXxh, "badc0ffee0ddf00d"))
599-
err = manifest.Add(chunk, cos.MiB, 1)
600-
Expect(err).NotTo(HaveOccurred())
601-
602-
manifest.Lock()
603-
retrievedChunk := manifest.GetChunk(1, true /*locked*/)
604-
manifest.Unlock()
605-
606-
Expect(retrievedChunk).NotTo(BeNil())
607-
})
608-
})
609-
610588
Describe("Validation Tests", func() {
611589
var manifest *core.Ufest
612590

‎core/lcopy.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,8 @@ add:
273273
}
274274

275275
// copy object => any local destination
276-
// recommended for copying between different buckets (compare with lom.Copy() above)
276+
// usage: copying between different buckets (compare with lom.Copy() above)
277+
// compare w/ Ufest.Relocate(lom)
277278
func (lom *LOM) Copy2FQN(dstFQN string, buf []byte) (dst *LOM, err error) {
278279
debug.Assert(lom.IsLocked() >= apc.LockRead, lom.Cname(), " source not locked")
279280

@@ -378,7 +379,7 @@ func (lom *LOM) copyChunks(dst *LOM, buf []byte) error {
378379
dst.SetCksum(&wholeCksum.Cksum)
379380

380381
// Store the completed manifest for destination
381-
err = dstUfest.storeCompleted(dst)
382+
err = dstUfest.storeCompleted(dst, false /*override*/)
382383
if err != nil {
383384
return dstUfest._undoCopy(fmt.Errorf("failed to store completed manifest for destination %s: %w", dst.Cname(), err))
384385
}

‎core/lom_test.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -962,8 +962,10 @@ var _ = Describe("LOM", func() {
962962

963963
// Verify all chunks exist and have correct content
964964
for i := 1; i <= srcUfest.Count(); i++ {
965-
srcChunk := srcUfest.GetChunk(i, false)
966-
dstChunk := dstUfest.GetChunk(i, false)
965+
srcChunk, err := srcUfest.GetChunk(i)
966+
Expect(err).NotTo(HaveOccurred())
967+
dstChunk, err := dstUfest.GetChunk(i)
968+
Expect(err).NotTo(HaveOccurred())
967969
Expect(srcChunk).NotTo(BeNil())
968970
Expect(dstChunk).NotTo(BeNil())
969971

‎core/lom_xattr_test.go‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -277,8 +277,10 @@ var _ = Describe("LOM Xattributes", func() {
277277

278278
// Verify individual chunks were copied correctly
279279
for i := 1; i <= srcUfest.Count(); i++ {
280-
srcChunk := srcUfest.GetChunk(i, false)
281-
dstChunk := dstUfest.GetChunk(i, false)
280+
srcChunk, err := srcUfest.GetChunk(i)
281+
Expect(err).NotTo(HaveOccurred())
282+
dstChunk, err := dstUfest.GetChunk(i)
283+
Expect(err).NotTo(HaveOccurred())
282284
Expect(srcChunk).NotTo(BeNil())
283285
Expect(dstChunk).NotTo(BeNil())
284286
Expect(srcChunk.Path()).To(BeARegularFile())
@@ -564,9 +566,6 @@ func createChunkedLOM(fqn string, numChunks int) *core.LOM {
564566
totalSize := int64(numChunks * chunkSize)
565567
lom.SetSize(totalSize)
566568

567-
// Create main object file
568-
createTestFile(fqn, int(totalSize))
569-
570569
// Create Ufest for chunked upload
571570
ufest, err := core.NewUfest("", lom, false)
572571
Expect(err).NotTo(HaveOccurred())
@@ -586,8 +585,6 @@ func createChunkedLOM(fqn string, numChunks int) *core.LOM {
586585
err = lom.CompleteUfest(ufest, false)
587586
Expect(err).NotTo(HaveOccurred())
588587

589-
// Reload to pick up chunked flag
590-
lom.UncacheUnless()
591588
Expect(lom.Load(false, false)).NotTo(HaveOccurred())
592589
Expect(lom.IsChunked()).To(BeTrue())
593590
return lom

‎core/multipart_test.go‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,9 @@ var _ = Describe("MPU-UfestRead", func() {
165165
Expect(err).NotTo(HaveOccurred())
166166
Expect(ufest.Completed()).To(BeTrue())
167167

168+
err = ufest.ValidateChunkLocations(true /*completed*/)
169+
Expect(err).NotTo(HaveOccurred(), "invalid chunk location(s)")
170+
168171
// Create chunk reader
169172
reader, err := ufest.NewReader()
170173
Expect(err).NotTo(HaveOccurred())
@@ -315,6 +318,9 @@ var _ = Describe("MPU-UfestRead", func() {
315318
err = lom.CompleteUfest(ufest, false)
316319
Expect(err).NotTo(HaveOccurred())
317320

321+
err = ufest.ValidateChunkLocations(true /*completed*/)
322+
Expect(err).NotTo(HaveOccurred(), "invalid chunk location(s)")
323+
318324
reader, err := ufest.NewReader()
319325
Expect(err).NotTo(HaveOccurred())
320326
defer func() {
@@ -510,7 +516,8 @@ var _ = Describe("MPU-UfestRead", func() {
510516

511517
for i := range numParts {
512518
expectedPartNum := i + 1
513-
actualChunk := manifest.GetChunk(expectedPartNum, true /*locked*/)
519+
actualChunk, err := manifest.GetChunk(expectedPartNum)
520+
Expect(err).NotTo(HaveOccurred())
514521
Expect(actualChunk).NotTo(BeNil(), "Part %d should exist", expectedPartNum)
515522
Expect(int(actualChunk.Num())).To(Equal(expectedPartNum),
516523
"Part should have correct sequential number")
@@ -519,6 +526,12 @@ var _ = Describe("MPU-UfestRead", func() {
519526

520527
manifest.Unlock()
521528

529+
err = manifest.Check(false /*completed*/)
530+
Expect(err).NotTo(HaveOccurred())
531+
532+
err = manifest.ValidateChunkLocations(false /*completed*/)
533+
Expect(err).NotTo(HaveOccurred(), "invalid chunk location(s)")
534+
522535
By("Step 4: Compute whole-object checksum (production lines 378-395)")
523536
wholeCksum := cos.NewCksumHash(cos.ChecksumMD5) // Local bucket uses MD5
524537
err = manifest.ComputeWholeChecksum(wholeCksum)
@@ -548,6 +561,12 @@ var _ = Describe("MPU-UfestRead", func() {
548561
Expect(manifest.Completed()).To(BeTrue(), "Manifest should be marked completed")
549562
Expect(lom.IsChunked()).To(BeTrue(), "LOM should be marked as chunked")
550563

564+
err = manifest.Check(true /*completed*/)
565+
Expect(err).NotTo(HaveOccurred())
566+
567+
err = manifest.ValidateChunkLocations(true /*completed*/)
568+
Expect(err).NotTo(HaveOccurred(), "invalid chunk location(s)")
569+
551570
// check LOM checksum
552571
lomCksum := lom.Checksum()
553572
Expect(lomCksum).NotTo(BeNil(), "LOM should have checksum set")

0 commit comments

Comments
 (0)