Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix operations with storage groups #2491

Merged
merged 7 commits into from
Aug 15, 2023
Merged

Conversation

cthulhu-rider
Copy link
Contributor

@cthulhu-rider cthulhu-rider commented Aug 10, 2023

fix regression came #2470
no changelog needed

Previously, `storagegroup.CollectMembers` function could return error
with text `data must be 64-bytes long`. According to the message it is
not clear which data is incorrect. Now context has been added to the
error so that it is clear at what stage it occurs.

Refs #2488.

Signed-off-by: Leonard Lyubich <[email protected]>
Previously, `storagegroup.CollectMembers` function could return error
with text `data must be 64-bytes long` if any of the split members were
without homomorphic checksum or with the invalid one. Including the
bypass of the splitting chain, and the error occurred already after at
stage `tz.Concat`. The correct format of the homomorphic checksum is
required to form the group, so it is required to do an in-place check.

Check each element of `IterateSplitLeaves` iterator and break traversal
on any checksum problem.

Refs #2488.

Signed-off-by: Leonard Lyubich <[email protected]>
Previously, `storagegroup.CollectMembers` function ignored absence of
the object ID in the member's child header. This could lead to incorrect
storage group formation, and also hid problem objects in the system (ID
is a required field for any object).

Immediately return error from `CollectMembers` if any child of any
member is without ID.

Refs #2488.

Signed-off-by: Leonard Lyubich <[email protected]>
@codecov
Copy link

codecov bot commented Aug 10, 2023

Codecov Report

Merging #2491 (ebebd14) into master (1684eee) will increase coverage by 0.08%.
Report is 10 commits behind head on master.
The diff coverage is 40.57%.

❗ Current head ebebd14 differs from pull request most recent head 28cf719. Consider uploading reports for the commit 28cf719 to get more accurate results

@@            Coverage Diff             @@
##           master    #2491      +/-   ##
==========================================
+ Coverage   29.26%   29.34%   +0.08%     
==========================================
  Files         399      401       +2     
  Lines       30390    30396       +6     
==========================================
+ Hits         8893     8921      +28     
+ Misses      20761    20739      -22     
  Partials      736      736              
Files Changed Coverage Δ
cmd/neofs-adm/internal/modules/config/config.go 30.98% <0.00%> (ø)
cmd/neofs-adm/internal/modules/morph/balance.go 0.00% <0.00%> (ø)
...md/neofs-adm/internal/modules/morph/dump_hashes.go 0.00% <0.00%> (ø)
cmd/neofs-adm/internal/modules/morph/epoch.go 0.00% <0.00%> (ø)
cmd/neofs-adm/internal/modules/morph/initialize.go 0.00% <0.00%> (ø)
...d/neofs-adm/internal/modules/morph/local_client.go 0.00% <0.00%> (ø)
cmd/neofs-node/container.go 0.00% <0.00%> (ø)
pkg/local_object_storage/blobstor/fstree/fstree.go 64.87% <0.00%> (ø)
pkg/services/object_manager/placement/netmap.go 0.00% <0.00%> (ø)
pkg/services/container/morph/executor.go 37.03% <25.00%> (ø)
... and 7 more

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Previously, storage nodes didn't check homomorphic checksum correctness
in incoming objects. When homomorphic hashing is required in the
container, nodes accepted objects without homomorphic hash header. This
could lead to subsequent problems with storage group creation because it
expects checksums in the members' headers.

Make storage nodes to check that homomorphic checksum of the new object
payload is set and has correct format according to TillichZemor algo. If
homomorphic hashing is disabled, the header is unchecked. Exact TZ hash
value is unchecked for now due to performance issues.

Refs #2488.

Signed-off-by: Leonard Lyubich <[email protected]>
Previously, storage nodes didn't calculate and set homomorphic payload
checksum for the sliced objects even when it was required. This could
affect Data Audit subsystem working with homomorphic hashes.

Bypass hashing option from `slicingTarget` to the underlying `Slicer`.

Fixes #2488.

Signed-off-by: Leonard Lyubich <[email protected]>
cmd/object-scanner/main.go Outdated Show resolved Hide resolved
cmd/object-scanner/main.go Outdated Show resolved Hide resolved
cmd/object-scanner/main.go Outdated Show resolved Hide resolved
Utility iterates over all objects and asserts if homomorphic checksum
format is compliant with the container policy.

Signed-off-by: Leonard Lyubich <[email protected]>
…objects"

This reverts commit 70704d3. The
utility may be useful in the future, for example, as part of NeoFS Lens
app. Thus, it's retained in the history.

Signed-off-by: Leonard Lyubich <[email protected]>
@roman-khimov roman-khimov merged commit f77c825 into master Aug 15, 2023
7 of 8 checks passed
@roman-khimov roman-khimov deleted the bugfix/storagegroup-ops branch August 15, 2023 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants