Drop lock bucket by roman-khimov · Pull Request #3672 · nspcc-dev/neofs-node

roman-khimov · 2025-11-10T20:50:51Z

No description provided.

codecov · 2025-11-11T06:20:25Z

Codecov Report

❌ Patch coverage is 70.51282% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 26.91%. Comparing base (79e4ff0) to head (7656ba6).
⚠️ Report is 23 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/core/object/fmt.go	7.69%	12 Missing ⚠️
pkg/local_object_storage/engine/put.go	76.66%	6 Missing and 1 partial ⚠️
cmd/neofs-cli/modules/object/lock.go	0.00%	2 Missing ⚠️
pkg/local_object_storage/metabase/version.go	80.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3672      +/-   ##
==========================================
- Coverage   27.32%   26.91%   -0.41%     
==========================================
  Files         657      658       +1     
  Lines       41827    41735      -92     
==========================================
- Hits        11429    11235     -194     
- Misses      29335    29463     +128     
+ Partials     1063     1037      -26

☔ 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/engine/put.go

carpawell

Once you drop Lock(..., locked []oid.ID) API from the engine, you must prohibit deprecated LOCK objects with payload on node's API layer. Either i do not see it, or it is not done.

carpawell · 2025-11-14T09:13:54Z

pkg/local_object_storage/metabase/lock.go

 func objectLocked(tx *bbolt.Tx, currEpoch uint64, metaCursor *bbolt.Cursor, idCnr cid.ID, idObj oid.ID) bool {
-	if associatedWithTypedObject(currEpoch, metaCursor, idObj, object.TypeLock) {
-		return true
+	locked, lockID := associatedWithTypedObject(currEpoch, metaCursor, idObj, object.TypeLock)


Lock can be deleted manually

why? https://github.com/nspcc-dev/neofs-api/blob/fc8584b766d6b544337fc055dca34c29b0155d31/object/service.proto#L92-L93

Control service.

In fact we have a test that does exactly that, that's how it was discovered initially.

Control service.

i think it should do it hard, without any expiration/TS/GCMark status, just drop everything. not sure i understand when LOCK can be graveyarded

I agree. But the way it currently works is via a mark. This code can be removed in future. Specifically I think that's the second part of #3476, tombstone processing will be changed significantly as well and that's where the concept of graveyard can be reconsidered.

ok, if you are saying it will be changed

pkg/local_object_storage/engine/put.go

roman-khimov · 2025-11-14T10:00:23Z

must prohibit deprecated LOCK objects with payload on node's API layer

I think one of the last commits does that, but I can easily miss some code that's related to it.

pkg/local_object_storage/engine/put.go

cthulhu-rider · 2025-11-18T16:37:48Z

pkg/local_object_storage/engine/put.go

 	if err != nil {
 		e.log.Warn("object put: pool task submitting", zap.Stringer("shard", id), zap.Error(err))
-		overloaded = errors.Is(err, ants.ErrPoolOverload)
 		close(exitCh)


may be dropped now

roman-khimov · 2025-11-19T14:58:31Z

Looks like we still have a lot of code producing old LOCKs:
https://rest.fs.neo.org/Aku38wpn2xEqfEPxmwyAa7N3VnAkMV8PEVkpLqwKM1z3/4333-1763562486/index.html#suites/295ed2e000f8fa6f3ade59cc13b98615/537c2b8fe584decf/
https://rest.fs.neo.org/Aku38wpn2xEqfEPxmwyAa7N3VnAkMV8PEVkpLqwKM1z3/4333-1763549834/index.html

This needs to be fixed before merging the PR.

carpawell · 2025-11-28T13:49:43Z

pkg/local_object_storage/metabase/lock.go

 func objectLocked(tx *bbolt.Tx, currEpoch uint64, metaCursor *bbolt.Cursor, idCnr cid.ID, idObj oid.ID) bool {
-	if associatedWithTypedObject(currEpoch, metaCursor, idObj, object.TypeLock) {
-		return true
+	locked, lockID := associatedWithTypedObject(currEpoch, metaCursor, idObj, object.TypeLock)


ok, if you are saying it will be changed

carpawell · 2025-11-28T13:53:36Z

pkg/core/object/fmt.go

+				return errors.New("system object has payload")
+			}
+			if obj.AssociatedObject().IsZero() {
+				return errors.New("system object has zero associated object")


LOCK type can be specified (unexpected bugs with types; logs in debug process)

If there is an engine-level request to drop some object it should be deleted everywhere because all shards can potentially store it. Signed-off-by: Roman Khimov <roman@nspcc.ru>

Much like we have tombstones and locks broadcasted to all nodes they all should be broadcasted to all shards to ensure proper meta handling at the shard level. It's important for subsequent lock processing rework as well since a lock for LINK object can be accepted by a shard not knowing about this LINK: --- FAIL: TestStorageEngine_Put_Lock (3.11s) --- FAIL: TestStorageEngine_Put_Lock/shards=5 (2.33s) --- FAIL: TestStorageEngine_Put_Lock/shards=5/non-regular_target (0.67s) put_test.go:110: Error Trace: /home/rik/dev/neofs-node/pkg/local_object_storage/engine/put_test.go:110 Error: Expected error with "status: code = 2051 message = locking non-regular object is forbidden" in chain but got nil. Test: TestStorageEngine_Put_Lock/shards=5/non-regular_target Signed-off-by: Roman Khimov <roman@nspcc.ru>

Lock can be deleted manually and in this case it's no longer active even if it's not yet physically removed. Signed-off-by: Roman Khimov <roman@nspcc.ru>

1. If any shard returns something like "object is locked" it's final, there is no point in continuing iterations. 2. Wrap and return the last error from the method, don't lose it. 3. Simplify putToShard signature, single error is sufficient here. Signed-off-by: Roman Khimov <roman@nspcc.ru>

It's no longer needed since API 2.18, we have all the data in meta index. This also removes Lock() method itself, a simple Put() of a LOCK object is sufficient now. A part of #3476. Signed-off-by: Roman Khimov <roman@nspcc.ru>

CLI creates this object according to some rules known to CLI only, this should be communicated. Signed-off-by: Roman Khimov <roman@nspcc.ru>

Signed-off-by: Roman Khimov <roman@nspcc.ru>

pkg/local_object_storage/engine/lock_test.go:176:2 staticcheck S1021: should merge variable declaration with assignment on next line Not related to recent changes. Signed-off-by: Roman Khimov <roman@nspcc.ru>

roman-khimov · 2025-12-02T13:40:18Z

Rebased, updated. It was unprepared object handling, see https://github.com/nspcc-dev/neofs-node/compare/f8eed22402d0f45bc25afb8db0a94b0f16755305..7656ba6b6cef9640a900e76bca0d04306a8f79e5. Tests still running in https://github.com/nspcc-dev/neofs-node/actions/runs/19860579457/job/56909340778

roman-khimov · 2025-12-02T19:09:44Z

Green (problems are not related to locks).
https://rest.fs.neo.org/Aku38wpn2xEqfEPxmwyAa7N3VnAkMV8PEVkpLqwKM1z3/4441-1764687654/index.html
https://rest.fs.neo.org/Aku38wpn2xEqfEPxmwyAa7N3VnAkMV8PEVkpLqwKM1z3/4441-1764687878/index.html

roman-khimov added this to the v0.50.0 milestone Nov 10, 2025

roman-khimov requested review from End-rey, carpawell and cthulhu-rider as code owners November 10, 2025 20:50

roman-khimov force-pushed the drop-lock-bucket branch 2 times, most recently from 44855fa to 6b9f67c Compare November 11, 2025 06:13

End-rey approved these changes Nov 13, 2025

View reviewed changes

pkg/local_object_storage/engine/put.go Outdated Show resolved Hide resolved

roman-khimov force-pushed the drop-lock-bucket branch from 6b9f67c to e7c3bae Compare November 13, 2025 15:32

carpawell reviewed Nov 14, 2025

View reviewed changes

roman-khimov force-pushed the drop-lock-bucket branch from e7c3bae to 1372faf Compare November 14, 2025 12:08

cthulhu-rider reviewed Nov 18, 2025

View reviewed changes

pkg/local_object_storage/engine/put.go Outdated Show resolved Hide resolved

roman-khimov force-pushed the drop-lock-bucket branch from 1372faf to 20adac3 Compare November 18, 2025 16:15

roman-khimov requested review from carpawell and cthulhu-rider November 18, 2025 16:16

cthulhu-rider approved these changes Nov 18, 2025

View reviewed changes

roman-khimov force-pushed the drop-lock-bucket branch from 20adac3 to 20fb2e0 Compare November 18, 2025 19:56

roman-khimov added the blocked Can't be done because of something label Nov 20, 2025

roman-khimov modified the milestones: v0.50.0, v0.51.0 Nov 21, 2025

carpawell approved these changes Nov 28, 2025

View reviewed changes

roman-khimov added 5 commits December 2, 2025 09:39

engine: drop object from all shards at the engine level

658f80c

If there is an engine-level request to drop some object it should be deleted everywhere because all shards can potentially store it. Signed-off-by: Roman Khimov <roman@nspcc.ru>

metabase: check graveyard for locks when taking them from meta

09799b7

Lock can be deleted manually and in this case it's no longer active even if it's not yet physically removed. Signed-off-by: Roman Khimov <roman@nspcc.ru>

metabase: do not use any special locker index

ad705d3

It's no longer needed since API 2.18, we have all the data in meta index. This also removes Lock() method itself, a simple Put() of a LOCK object is sufficient now. A part of #3476. Signed-off-by: Roman Khimov <roman@nspcc.ru>

roman-khimov force-pushed the drop-lock-bucket branch from 20fb2e0 to f8eed22 Compare December 2, 2025 06:41

cli: set object version when creating a LOCK

b6823c1

CLI creates this object according to some rules known to CLI only, this should be communicated. Signed-off-by: Roman Khimov <roman@nspcc.ru>

roman-khimov added 2 commits December 2, 2025 16:15

core: reject <2.18 API LOCKs

a88a651

Signed-off-by: Roman Khimov <roman@nspcc.ru>

engine: merge declaration and initialization in test

7656ba6

pkg/local_object_storage/engine/lock_test.go:176:2 staticcheck S1021: should merge variable declaration with assignment on next line Not related to recent changes. Signed-off-by: Roman Khimov <roman@nspcc.ru>

roman-khimov force-pushed the drop-lock-bucket branch from f8eed22 to 7656ba6 Compare December 2, 2025 13:15

roman-khimov merged commit ae46bc4 into master Dec 2, 2025
21 of 22 checks passed

roman-khimov deleted the drop-lock-bucket branch December 2, 2025 19:10

Copilot AI mentioned this pull request Dec 10, 2025

Drop support for old tombstones #3730

Closed

roman-khimov mentioned this pull request Dec 25, 2025

Helper object for newbie nodes/shards #2015

Closed

roman-khimov mentioned this pull request Mar 10, 2026

Forbid to PUT pre-2.18 objects #3869

Closed

Conversation

roman-khimov commented Nov 10, 2025

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Uh oh!

carpawell left a comment

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!

Uh oh!

Uh oh!

roman-khimov commented Nov 14, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roman-khimov commented Nov 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roman-khimov commented Dec 2, 2025

Uh oh!

roman-khimov commented Dec 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Nov 11, 2025 •

edited

Loading