Revive dead object via control service #2968

End-rey · 2024-10-17T12:26:31Z

Closes #1450.

codecov · 2024-10-17T12:29:47Z

Codecov Report

Attention: Patch coverage is 9.67239% with 579 lines in your changes missing coverage. Please review.

Project coverage is 22.89%. Comparing base (c4bdae1) to head (26563d4).
Report is 8 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/services/control/service.pb.go	4.02%	262 Missing ⚠️
pkg/services/control/service_neofs.pb.go	0.00%	87 Missing ⚠️
pkg/services/control/service_grpc.pb.go	0.00%	62 Missing ⚠️
cmd/neofs-cli/modules/control/object_revive.go	0.00%	52 Missing ⚠️
pkg/services/control/server/object_revive.go	0.00%	33 Missing ⚠️
pkg/local_object_storage/metabase/revive.go	61.44%	23 Missing and 9 partials ⚠️
pkg/local_object_storage/engine/revive.go	0.00%	17 Missing ⚠️
pkg/services/control/rpc.go	0.00%	13 Missing ⚠️
pkg/local_object_storage/shard/revive.go	0.00%	10 Missing ⚠️
pkg/services/control/convert.go	0.00%	9 Missing ⚠️
... and 1 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2968      +/-   ##
==========================================
- Coverage   23.47%   22.89%   -0.59%     
==========================================
  Files         780      785       +5     
  Lines       46655    58747   +12092     
==========================================
+ Hits        10953    13449    +2496     
- Misses      34833    44419    +9586     
- Partials      869      879      +10

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

pkg/local_object_storage/shard/mode/mode.go

pkg/local_object_storage/engine/revive.go

pkg/local_object_storage/metabase/revive.go

Signed-off-by: Andrey Butusov <[email protected]>

roman-khimov · 2024-10-22T19:34:01Z

pkg/services/control/server/object_revive.go

+	}
+
+	var addr oid.Address
+	err = addr.DecodeString(request.GetBody().GetObjectAddress())


Why a string representation here? I'd not expect this in a protobuf protocol, you can avoid decoding.

I don't really understand. Do I need to transmit via bytes? I did the same as the status.

Whoa. Really unexpected. Well, it's a good reference, but I'd say it's a bug of status implementation. We can either divert here and make it better (it simplifies things) or follow established practice. Not critical then.

i thought i was expired by some another example for status but i do not find any string implementation (it is strange, cause i am for raw bytes too) so let it be raw bytes then cause we also have it in control API:

neofs-node/pkg/services/control/service.proto

Lines 117 to 119 in 5c5c42c

// List of object addresses to be removed.

// in NeoFS API binary format.

repeated bytes address_list = 1;

status can be fixed separately

Can you please explain where I should use bytes and where string?
For example, here:

neofs-node/pkg/services/control/service.proto

Lines 454 to 456 in 1c78683

string shard_id = 1;

// Object's revival status in a free form (human readable).

string status = 2;

I see that shard_id can also be bytes, as shown here:

neofs-node/pkg/services/control/types.proto

Line 123 in f858a81

bytes shard_ID = 1 [json_name = "shardID"];

i think shard ID is ok to be string cause there is no fixed public rule for bytes -> string conversion (for addresses we know it), but for things from our public API it is ok to be bytes and cli can convert it inside according to SDK
@roman-khimov

Shard ID can be a string, it's deeply internal to the node, no one knows how to convert it properly into something human-oriented.

pkg/local_object_storage/engine/revive.go

Includes API definition extending, RPC implementation, tests of metabase func. The command requests server's storage engine to revive object by address. It's purge all removal marks from all metabases and returns revival statuses. Signed-off-by: Andrey Butusov <[email protected]>

Support command that revive object by purging all removal marks from all metabases. ``` $ neofs-cli control object status --endpoint s04.neofs.devenv:8081 -w services/storage/wallet04.json --object 4yYJV2AyHaJ3fpVimEihAj6NTkwmanbwE7YatJUWBPyM/CYUeRyjfiNXhBMW9XZmdSBJNVLXYBH3gSNQASWTGZX8t Enter password > Shard ID: 2wEzFvWsao9yBiDWKjSBfF metabase: AVAILABLE peapod: path: "/storage/peapod1.db" $ neofs-cli object delete -r s04.neofs.devenv:8080 -w services/storage/wallet04 .json --cid 4yYJV2AyHaJ3fpVimEihAj6NTkwmanbwE7YatJUWBPyM --oid CYUeRyjfiNXhBMW9XZmdSBJNVLXYBH3gSNQASWTGZX8t Enter password > Object CYUeRyjfiNXhBMW9XZmdSBJNVLXYBH3gSNQASWTGZX8t removed successfully. ID: C2LrMExCF3FTfQaFYYZfoDGLpsUVLUy156pUpWh72YeN CID: 4yYJV2AyHaJ3fpVimEihAj6NTkwmanbwE7YatJUWBPyM $ neofs-cli control object status --endpoint s04.neofs.devenv:8081 -w services/storage/wallet04.json --object 4yYJV2AyHaJ3fpVimEihAj6NTkwmanbwE7YatJUWBPyM/CYUeRyjfiNXhBMW9XZmdSBJNVLXYBH3gSNQASWTGZX8t Enter password > Shard ID: 2wEzFvWsao9yBiDWKjSBfF metabase: AVAILABLE,IN GRAVEYARD peapod: path: "/storage/peapod1.db" $ neofs-cli control object revive --endpoint s04.neofs.devenv:8081 -w services/storage/wallet04.json --object 4yYJV2AyHaJ3fpVimEihAj6NTkwmanbwE7YatJUWBPyM/CYUeRyjfiNXhBMW9XZmdSBJNVLXYBH3gSNQASWTGZX8t Enter password > Shard ID: Vuy2Q8QaPZSuUxDycPxSBC Revival status: don't revive, err: logical error: object neither in the graveyard nor was marked with GC mark Shard ID: 2wEzFvWsao9yBiDWKjSBfF Revival status: successful revival from graveyard, tomb: 4yYJV2AyHaJ3fpVimEihAj6NTkwmanbwE7YatJUWBPyM/C2LrMExCF3FTfQaFYYZfoDGLpsUVLUy156pUpWh72YeN $ neofs-cli control object status --endpoint s04.neofs.devenv:8081 -w services/storage/wallet04.json --object 4yYJV2AyHaJ3fpVimEihAj6NTkwmanbwE7YatJUWBPyM/CYUeRyjfiNXhBMW9XZmdSBJNVLXYBH3gSNQASWTGZX8t Enter password > Shard ID: 2wEzFvWsao9yBiDWKjSBfF metabase: AVAILABLE peapod: path: "/storage/peapod1.db" ``` Closes #1450. Signed-off-by: Andrey Butusov <[email protected]>

roman-khimov · 2024-10-24T19:54:34Z

pkg/services/control/server/object_revive.go

+	}
+
+	var addr oid.Address
+	err = addr.DecodeString(string(request.GetBody().GetObjectAddress()))


Wait, but the idea was exactly to avoid DecodeString.

But I don't really understand how to avoid it. I did it like drop object, that @carpawell provided as example:

neofs-node/pkg/services/control/service.proto

Lines 123 to 125 in 5567e5f

// List of object addresses to be removed.

// in NeoFS API binary format.

repeated bytes address_list = 1;

And I can't find a method to convert bytes to an address.

Looks like our control API is broken all over the place wrt this problem, so we can just follow the way it currently works and solve the problem for all requests in #2980.

End-rey requested review from roman-khimov, carpawell and cthulhu-rider as code owners October 17, 2024 12:26

End-rey force-pushed the 1450-allow-to-revive-dead-object-via-control-service branch from 7399fa6 to 2694957 Compare October 17, 2024 12:27

carpawell reviewed Oct 17, 2024

View reviewed changes

End-rey self-assigned this Oct 18, 2024

*: fix incorrect comment var name

d743d42

Signed-off-by: Andrey Butusov <[email protected]>

End-rey force-pushed the 1450-allow-to-revive-dead-object-via-control-service branch from 2694957 to 1e3f1ae Compare October 18, 2024 11:58

End-rey requested a review from carpawell October 21, 2024 13:27

carpawell approved these changes Oct 21, 2024

View reviewed changes

roman-khimov reviewed Oct 22, 2024

View reviewed changes

End-rey force-pushed the 1450-allow-to-revive-dead-object-via-control-service branch from 1e3f1ae to 4ded500 Compare October 23, 2024 11:57

End-rey added 2 commits October 23, 2024 17:26

End-rey force-pushed the 1450-allow-to-revive-dead-object-via-control-service branch from 4ded500 to 26563d4 Compare October 23, 2024 14:26

End-rey requested a review from roman-khimov October 24, 2024 07:43

roman-khimov reviewed Oct 24, 2024

View reviewed changes

roman-khimov mentioned this pull request Oct 25, 2024

Control API passes addresses in stringified format #2980

Open

roman-khimov approved these changes Oct 25, 2024

View reviewed changes

roman-khimov merged commit 1b2a229 into master Oct 25, 2024
18 of 21 checks passed

roman-khimov deleted the 1450-allow-to-revive-dead-object-via-control-service branch October 25, 2024 12:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revive dead object via control service #2968

Revive dead object via control service #2968

End-rey commented Oct 17, 2024

codecov bot commented Oct 17, 2024 •

edited

Loading

roman-khimov Oct 22, 2024

End-rey Oct 23, 2024

roman-khimov Oct 23, 2024

carpawell Oct 23, 2024

End-rey Oct 23, 2024

carpawell Oct 23, 2024 •

edited

Loading

roman-khimov Oct 23, 2024

roman-khimov Oct 24, 2024

End-rey Oct 25, 2024

roman-khimov Oct 25, 2024

	// List of object addresses to be removed.
	// in NeoFS API binary format.
	repeated bytes address_list = 1;

	string shard_id = 1;
	// Object's revival status in a free form (human readable).
	string status = 2;

Revive dead object via control service #2968

Revive dead object via control service #2968

Conversation

End-rey commented Oct 17, 2024

codecov bot commented Oct 17, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carpawell Oct 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Oct 17, 2024 •

edited

Loading

carpawell Oct 23, 2024 •

edited

Loading