Repository navigation
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4210 +/- ##
==========================================
- Coverage 31.45% 31.28% -0.17%
==========================================
Files 677 677
Lines 41572 41599 +27
==========================================
- Hits 13075 13016 -59
- Misses 28497 28583 +86 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Checked with nspcc-dev/neofs-testcases#1467, but it required adding the last commit, since it drops the whole rule. It effectively means that any EC check is now forced to check any other rule, multiplying every EC check complexity by the number of EC rules. |
4febb94 to
6752ff8
Compare
| panic("trying to cross restore EC parts with single EC rule: missing parts number: %d, skip parts number: %d") | ||
| } | ||
| var ( | ||
| shortage = int(rule.ParityPartNum) - (len(missingIdx) + len(skipIdx)) |
There was a problem hiding this comment.
this part was made based on my wrong assumptions about the code above, dropped
There was a problem hiding this comment.
oh no, i now recall why it is done this way, returned it back, but fixed, no negative counters
There was a problem hiding this comment.
but, interestingly, none of the cases in the python test found it. i need to improve local coverage
| // Server may store the part. We consider it unavailable, but we don't attempt to recreate it. | ||
| // Once SN finishes maintenance, the part will likely become available. | ||
| if len(missingIdx)+len(skipIdx) >= int(rule.ParityPartNum) { | ||
| if len(missingIdx)+len(skipIdx) >= int(rule.ParityPartNum) && len(ecRules) == 1 { |
There was a problem hiding this comment.
Why >= btw? Let's say we have 8/3 split and 3 parts missing, 8 are sufficient.
There was a problem hiding this comment.
it confused me too, but it is likely done this way to prevent redundant append below and to make fast error return. i dont like it either
| shortage = int(rule.ParityPartNum) - (len(missingIdx) + len(skipIdx)) | ||
| dataPartsToRestore = make([]int, 0, rule.DataPartNum) | ||
| ) | ||
| for partIdx := range rule.DataPartNum { |
There was a problem hiding this comment.
But why data parts only? Parity can be missing as well.
There was a problem hiding this comment.
we are here to reconstruct data from other rules. other rules are only required to have the same data parts, parity parts are encoded according to different parity numbers, it is pointless to me to try to fetch other parity parts but the data parts are always there and always the same. code below will always recreate missing parity parts the same way it did before this PR, the only assumption from my side is that there is no need to fetch parity parts and try to play with them, trying to find out if they are suitable for this rule
There was a problem hiding this comment.
it is kinda tricky in such cases if we are only fetching data parts, cause there not enough parts to restore the full chain as of 1 < 2 == true but that's nonsense, cause we have exactly the original data we want; it is just not enough to "recreate" (but making a new encoding is still possible) parity parts
| if partLastByte <= off { | ||
| continue | ||
| } | ||
| for nodeIdx := range iec.NodeSequenceForPart(int(partIdx), totalParts, len(nodeLists)) { |
There was a problem hiding this comment.
Can't you just request an object range from healthy nodes? Without thinking much about other rule parts?
There was a problem hiding this comment.
can be done i think, but it sounds strange to me: we are already deep there and know exactly what parts we have to fetch with what ranges, but instead we are trying other nodes (or even ourselves again) to do this job again and believe in it
If there are not enough EC parts for object recreation but more EC rules are found, SN fetches minimum required missing data parts from these rules and uses them for this rule restoration. Refs #3848. Signed-off-by: Pavel Karpy <carpawell@nspcc.io>
If all objects for EC rule X are lost, SN can restore them using the non-X rules (if any). It is done by forcing all EC rules rechecks for every EC object found on any SN node. Closes #3848. Signed-off-by: Pavel Karpy <carpawell@nspcc.io>
6752ff8 to
4541aab
Compare
This branch has not been deployed
2 participants
No description provided.