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

slicer: Do not write split ID into parent header #474

Merged

Conversation

cthulhu-rider
Copy link
Contributor

Split ID relates to split-chain only, original root object should not carry this information, otherwise, it will be perceived by the system itself as part of a sliced chain that does not exist.

Split ID relates to split-chain only, original root object should not
carry this information, otherwise, it will be perceived by the system
itself as part of a sliced chain that does not exist.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@smallhive
Copy link
Contributor

I consider you should discuss this with @carpawell, because we fixed/added the splitID to first header here #448, to fix some bug inside node

@carpawell
Copy link
Member

@smallhive, the first child obj and the root obj are not the same so that PR is correct.

@cthulhu-rider, do we have a test for "every child has a SplitID"? Slicer code is quite complex such a pair of tests would help (if not yet): (1) every part has a correct and full Split header and (2) a root object has no split info ((2) is already done in that PR).

BTW, is our API description full enough to understand that we should not add splitID to the root object?

@cthulhu-rider
Copy link
Contributor Author

@carpawell

do we have a test for "every child has a SplitID"?

yep, i modified place which checks case 1.

for _, h := range writer.headers {
splitID := h.SplitID()
require.NotNil(t, splitID)
require.Equal(t, writer.splitID.ToV2(), splitID.ToV2())

API description full enough to understand

full enough? guess no. Strictly speaking, root object is not a part of split-chain cuz it's an autonomous object. When we restore original object, we don't expect to see any split headers, huh? I think so.

if you ask me why this (root split ID absence) is not a recommendation, but a requirement, I will not give an exact answer. There is an open issue nspcc-dev/neofs-node#2451

@roman-khimov roman-khimov merged commit 1300860 into nspcc-dev:master Jul 25, 2023
5 checks passed
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.

4 participants