Fix/v2 split placement to non-container node #2792
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2792 +/- ##
==========================================
- Coverage 22.03% 22.02% -0.01%
==========================================
Files 789 789
Lines 47245 47265 +20
==========================================
Hits 10410 10410
- Misses 35941 35961 +20
Partials 894 894 ☔ View full report in Codecov by Sentry. |
| @@ -20,7 +20,8 @@ type preparedObjectTarget interface { | |||
| } | |||
|
|
|||
| type distributedTarget struct { | |||
| traversal traversal | |||
| traversalParams traversal | |||
There was a problem hiding this comment.
its not params, its a traversal state, so new name is unclear
There was a problem hiding this comment.
i was not sure i can understand what traversal is while there is traverser, so let it be traversalState then
| @@ -156,6 +156,27 @@ func (x Node) PublicKey() []byte { | |||
| return x.key | |||
| } | |||
|
|
|||
| // KeyInContainer passes nodes' public keys from the calculated placement vectors to the | |||
There was a problem hiding this comment.
iterator method (action) is named as property. I suggest LookupKeyInContainer
There was a problem hiding this comment.
I can call it as KeyInContainer(isNotLocalKey()), so some IterateContainerKeys() can be even more appropriate.
There was a problem hiding this comment.
i do not like iteration where i do not need iteration (the same way here), i just wanted bool but there was no key in the distributedTarget, no idea why isLocalKey is useful but did not want to change this part in a little tests fix PR so let it be IterateContainerKeys
| @@ -243,7 +249,11 @@ loop: | |||
| } | |||
|
|
|||
| // perform additional container broadcast if needed | |||
| if t.traversal.submitPrimaryPlacementFinish() { | |||
| if t.traversalParams.submitPrimaryPlacementFinish() { | |||
| // reset traversal progress | |||
There was a problem hiding this comment.
afaik u dont reset processed nodes, so its not a progress reset
There was a problem hiding this comment.
traverser has SubmitSuccess method. it breaks additional broadcasting if do not reset this state
| // for the link validation and may decline it, does not matter what this node | ||
| // thinks about it | ||
| if t.obj.Type() != objectSDK.TypeLink || t.traverser.KeyInContainer(t.isLocalKey) { | ||
| if t.objMeta, err = t.fmt.ValidateContent(t.obj); err != nil { |
There was a problem hiding this comment.
I think some syntactic check can still be made here, although it doesn't seem to be critical.
There was a problem hiding this comment.
you mean to try to parse the link object but not validate? or what "syntactic check"?
There was a problem hiding this comment.
Yes, parse, but don't go fetching/checking things.
| @@ -156,6 +156,27 @@ func (x Node) PublicKey() []byte { | |||
| return x.key | |||
| } | |||
|
|
|||
| // KeyInContainer passes nodes' public keys from the calculated placement vectors to the | |||
There was a problem hiding this comment.
I can call it as KeyInContainer(isNotLocalKey()), so some IterateContainerKeys() can be even more appropriate.
Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
carpawell
force-pushed
the
fix/v2-split-placement-to-non-container-node
branch
from
419bbff
to
59e2f7e
Compare
This may be impossible to validate depending on container privacy settings. Also, if a node does not have to store a link object, its validation is literally useless work: it will repeat all the work one more time on any container inclusion and corresponding link object replication. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
1.22.0 says it should be this way. Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
carpawell
force-pushed
the
fix/v2-split-placement-to-non-container-node
branch
from
59e2f7e
to
695cad1
Compare
No description provided.