EC expiration by cthulhu-rider · Pull Request #3718 · nspcc-dev/neofs-node

cthulhu-rider · 2025-12-05T12:03:57Z

codecov · 2025-12-05T12:14:52Z

Codecov Report

❌ Patch coverage is 76.40449% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 26.93%. Comparing base (b71983d) to head (18a0c51).
⚠️ Report is 29 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/local_object_storage/metabase/delete.go	78.37%	11 Missing and 5 partials ⚠️
pkg/local_object_storage/shard/delete.go	66.66%	4 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3718      +/-   ##
==========================================
+ Coverage   26.89%   26.93%   +0.03%     
==========================================
  Files         658      658              
  Lines       41825    41879      +54     
==========================================
+ Hits        11249    11279      +30     
- Misses      29542    29559      +17     
- Partials     1034     1041       +7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

pkg/local_object_storage/shard/delete.go

roman-khimov · 2025-12-08T12:36:37Z

pkg/local_object_storage/metabase/delete.go

 type DeleteRes struct {
+	// Objects that were hardly linked to objects passed to [DB.Delete] and that
+	// were deleted along with them.
+	AdditionalObjects map[oid.Address][]oid.ID


What bugs me is that we don't need maps here or below we're OK with sets of addresses with their size. So what we can do is just to have DeletedObjects []struct{addr oid.Address, size uint64} which would also cover for AvailableRemoved (not sure about RawRemoved). Then this result is easier to digest in Delete() caller, just a single pass on deleted things, not requiring any iter.Seq dances.

there can be cases when input address list is not extended, and all its items are removed. In this case, returning shallow copy in res would be more efficient. At the same time, having []struct{addr oid.Address, size uint64} would require to make a copy anyway

then EC brings one more list of addresses (agree map is not required here, can be of []oid.Address type). So why should we concat EC list with the input one when we have iterators (or two loops if u wish)?

With current code you allocate for Sizes anyway and it usually has all of the addresses from the input.

do u mean

[]struct{addr oid.Address, size uint64}

to be allocated by the caller?

No, it can be managed in metabase, it's a part of DeleteRes anyway.

if same addrs passed by the caller are handled, isn't allocation of Sizes []uint64 only is lighter than []struct{addr oid.Address, size uint64} w/ copying addrs elements to it?

It is. But you already have a map[oid.Address]uint64 there and that makes it a completely different story.

I can keep it []uint64. But structing it with addresses makes no sense

On its own it's not, but combined with AdditionalObjects it sure is. We want to allocate as little as possible both in terms of overall size and in terms of allocated objects (which adds both CPU and space overhead). A single slice is a single slice, it's easy for allocator to handle, a map is already more complex thing, a map of slices is even worse, it can require a lot of microallocations. If that would be required by the way we use this construction --- ok, no problem, but it's not.

pkg/local_object_storage/metabase/delete.go

pkg/local_object_storage/shard/delete.go

pkg/local_object_storage/metabase/delete.go

pkg/local_object_storage/metabase/delete_test.go

pkg/local_object_storage/metabase/delete.go

carpawell · 2025-12-11T11:11:10Z

pkg/local_object_storage/metabase/delete.go

 	err = db.boltDB.Update(func(tx *bbolt.Tx) error {
 		// We need to clear slice because tx can try to execute multiple times.
-		rawRemoved, availableRemoved, err = db.deleteGroup(tx, addrs, sizes)
+		rawRemoved, availableRemoved, removed, err = db.deleteGroup(tx, addrs)


too many vars with confusing names to me. add suffixes like "count", "objects", etc?

pkg/local_object_storage/metabase/delete.go

carpawell · 2025-12-11T11:56:08Z

pkg/local_object_storage/metabase/delete.go

+		}
+
+		if ecPref == nil {
+			ecPref = slices.Concat([]byte{metaPrefixIDAttr}, partID, []byte(iec.AttributePrefix)) // any of EC attributes


can you, please, recall me, if it is possible to PUT some object with this attribute, mentioning some different object as a parent? will it be accidentally dropped too with this code?

Each EC part has parent header inside, so consistency can be checked.

i meant if it is possible to misuse this code with ~~our regular splits~~ custom objects that have conflicting attributes

roman-khimov

I'll squash the first two commits.

Expiration mechanism was incompatible with EC. When a root object expires, either it or its size-split parts are submitted for deletion. Therefore, EC parts were not included. At the same time, it is important to remove physical objects within GC procedure. This corrects the behavior with minimal intrusion into original logic. Specifically, metabase `Delete()` method now automatically picks up EC parts, deletes and returns them for removal from write-cache and BLOB storage. Fixes #3712. Refs #3502, #3500, #526. Signed-off-by: Leonard Lyubich <leonard@morphbits.io>

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>

cthulhu-rider force-pushed the ec-expiration branch 2 times, most recently from ba8a798 to 2b289c2 Compare December 5, 2025 12:08

cthulhu-rider requested a review from evgeniiz321 December 5, 2025 12:08

cthulhu-rider force-pushed the ec-expiration branch from 2b289c2 to 67a0e24 Compare December 8, 2025 10:43

cthulhu-rider marked this pull request as ready for review December 8, 2025 10:45

cthulhu-rider requested review from End-rey, carpawell and roman-khimov as code owners December 8, 2025 10:45

roman-khimov reviewed Dec 8, 2025

View reviewed changes

cthulhu-rider force-pushed the ec-expiration branch from 67a0e24 to 34e4125 Compare December 9, 2025 15:59

roman-khimov reviewed Dec 9, 2025

View reviewed changes

pkg/local_object_storage/shard/delete.go Outdated Show resolved Hide resolved

pkg/local_object_storage/metabase/delete.go Outdated Show resolved Hide resolved

pkg/local_object_storage/metabase/delete.go Outdated Show resolved Hide resolved

cthulhu-rider force-pushed the ec-expiration branch 3 times, most recently from 79de47e to f3254df Compare December 10, 2025 10:11

cthulhu-rider requested a review from roman-khimov December 10, 2025 10:15

roman-khimov reviewed Dec 11, 2025

View reviewed changes

pkg/local_object_storage/metabase/delete.go Outdated Show resolved Hide resolved

pkg/local_object_storage/metabase/delete_test.go Show resolved Hide resolved

pkg/local_object_storage/metabase/delete.go Outdated Show resolved Hide resolved

cthulhu-rider force-pushed the ec-expiration branch from f3254df to 163cc35 Compare December 11, 2025 09:00

carpawell reviewed Dec 11, 2025

View reviewed changes

roman-khimov approved these changes Dec 12, 2025

View reviewed changes

carpawell approved these changes Dec 12, 2025

View reviewed changes

cthulhu-rider added 2 commits December 12, 2025 12:08

storage/metabase: Merge two for loops

18a0c51

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>

roman-khimov force-pushed the ec-expiration branch from 163cc35 to 18a0c51 Compare December 12, 2025 09:08

roman-khimov merged commit fc95807 into master Dec 12, 2025
22 checks passed

roman-khimov deleted the ec-expiration branch December 12, 2025 09:36

Conversation

cthulhu-rider commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roman-khimov Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carpawell Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roman-khimov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cthulhu-rider commented Dec 5, 2025 •

edited

Loading

codecov bot commented Dec 5, 2025 •

edited

Loading

roman-khimov Dec 8, 2025 •

edited

Loading

carpawell Dec 12, 2025 •

edited

Loading