Fix and improve EC part range request buffer management - #4176
Merged
Merged
Conversation
Regression from 6916f13. EC GET handler manages request buffers for part ranges from buffer pool. If buffer is no longer needed in the context of the current request, it is returned to the pool. Previously, the return criterion was false if the entire range was requested with length=0 and 0 bytes were received without a fatal error. This could lead to a data race with various consequences. Problem was inconsistent, so reproduction was pretty challenging: it was necessary to turn off the node storing one EC part at the right time. It could happen that the buffer would be filled with another range of another EC part, which in turn would lead to incorrect final data. Example from CLI: ``` Error: rpc error: copy payload: payload size overflow ``` Also, following panic was caught at least once: ``` panic: too short buffer [recovered, repanicked] goroutine 120984 [running]: sync.(*WaitGroup).Go.func1.1() sync/waitgroup.go:251 +0x48 panic({0x10609e0c0?, 0x106341150?}) runtime/panic.go:860 +0x12c github.com/nspcc-dev/neofs-sdk-go/proto/encoding.MarshalToBytes[...]({0x3501635e62bb?, 0x3d, 0x45}, 0x6390b2e8, {0x350163a345a0?, 0x41, 0x300000010655eb90}) github.com/nspcc-dev/neofs-sdk-go@v1.0.0-rc.22/proto/encoding/encoding.go:205 +0xd8 github.com/nspcc-dev/neofs-sdk-go/proto/refs.WriteSignature[...]({0x3501635e6298, 0x60, 0x68}, {0x3501637788d0, 0x41?, 0x35016390b378?}, {0x350163a345a0?, 0x41, 0x41}, 0x0) github.com/nspcc-dev/neofs-sdk-go@v1.0.0-rc.22/proto/refs/encoding.go:250 +0xa0 github.com/nspcc-dev/neofs-sdk-go/proto/refs.WriteSignatureField[...]({0x3501635e6296, 0x62, 0x6a}, 0x5, {0x3501637788d0, 0x21, 0x21}, {0x350163a345a0, 0x41, 0x41}, ...) github.com/nspcc-dev/neofs-sdk-go@v1.0.0-rc.22/proto/refs/encoding.go:243 +0xf0 github.com/nspcc-dev/neofs-sdk-go/proto/session.writeRequestVerificationHeader[...]({0x3501635e6296, 0x62, 0x6a}, {0x0, 0x0?, 0x0?}, {0x0?, 0x0?, 0x0?}, 0x0?, ...) github.com/nspcc-dev/neofs-sdk-go@v1.0.0-rc.22/proto/session/encoding.go:564 +0x1cc github.com/nspcc-dev/neofs-sdk-go/proto/session.WriteSingleSignatureRequestVerificationHeader[...](...) github.com/nspcc-dev/neofs-sdk-go@v1.0.0-rc.22/proto/session/encoding.go:556 github.com/nspcc-dev/neofs-sdk-go/proto/session.WriteSingleSignatureRequestVerificationHeaderToRequest[...]({0x3501635e6294, 0x64, 0x6c}, {0x3501637788d0, 0x21, 0x21}, 0x0, {0x350163a345a0, 0x41, 0x41}) github.com/nspcc-dev/neofs-sdk-go@v1.0.0-rc.22/proto/session/encoding.go:548 +0x194 github.com/nspcc-dev/neofs-node/pkg/services/object.(*Server).writeRequestSignatures(0x350163595e00, {0x3501635e6200, 0xf8, 0x100}, 0x94, {0x3501635e6202, 0x52, 0xfe}, {0x3501635e6256, 0x3e, ...}, ...) github.com/nspcc-dev/neofs-node/pkg/services/object/proto.go:321 +0x174 github.com/nspcc-dev/neofs-node/pkg/services/object.(*Server).makeLocalRequest(0x350163595e00, 0x1, {0x155554?, 0x0?}, 0x52, 0x350163835780, {0x3501638357e8, 0x4, 0x4}) github.com/nspcc-dev/neofs-node/pkg/services/object/proto.go:395 +0x21c github.com/nspcc-dev/neofs-node/pkg/services/object.(*Server).makeGetECPartRequest(0x350163595e00, 0x1, {0x0?, 0x63a208d8?}, {0x6c, 0xd, 0x78, 0xfb, 0xb7, 0xe4, ...}, ...) github.com/nspcc-dev/neofs-node/pkg/services/object/get.go:953 +0x170 github.com/nspcc-dev/neofs-node/pkg/services/object.(*getECTransport).makeGetECPartRangeRequest(0x350163c58140, 0x1, {0x0?, 0x63a20900?}, {0x0, 0x2}, 0x0, 0x155554) github.com/nspcc-dev/neofs-node/pkg/services/object/get.go:995 +0x2ac ``` Or, more likely, the buffer would become corrupted and the following error would occur: ``` warn object/get.go:749 GET object API failure (receive message) {"node": "localhost:8081", "error": "rpc error: code = Internal desc = grpc: failed to unmarshal the received message: proto: cannot parse invalid wire-format data"} ``` This fixes behavior for described scenario only. It should be noted that room for improvement was noted, both in terms of buffer return and a more safe code structure. But that's for the future. Fixes #4166. Signed-off-by: Leonard Liubich <leonard@morphbits.io>
cthulhu-rider
force-pushed
the
bugfix/ec-get
branch
from
September 11, 2026 15:28
70a5e93 to
26ed4c1
Compare
So it could be potentially reused. Signed-off-by: Leonard Liubich <leonard@morphbits.io>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4176 +/- ##
==========================================
+ Coverage 30.99% 31.01% +0.02%
==========================================
Files 673 673
Lines 40646 40682 +36
==========================================
+ Hits 12597 12618 +21
- Misses 28049 28064 +15 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
If at least one byte of particular EC part range was received, the same request will no longer used. Signed-off-by: Leonard Liubich <leonard@morphbits.io>
Previously, called method was tied to its callers' behavior. This complicated the logic and could easily break. Signed-off-by: Leonard Liubich <leonard@morphbits.io>
In the previous logic, SN could miss the lack of EC part payload range from a remote SN. Also, it could interrupt the payload reception too early when response chunk was empty. All because of the zero-length comparison for full range queries. Signed-off-by: Leonard Liubich <leonard@morphbits.io>
cthulhu-rider
changed the title
cthulhu-rider
marked this pull request as ready for review
September 11, 2026 16:13
cthulhu-rider
requested review from
End-rey,
carpawell and
roman-khimov
as code owners
September 11, 2026 16:13
roman-khimov
approved these changes
Sep 12, 2026
The check is performed but after more data is already sent to the client. In particular, in #4166 this led to a client (CLI) error, although it should have been caught by the server: ``` Error: rpc error: copy payload: payload size overflow ``` Signed-off-by: Leonard Liubich <leonard@morphbits.io>
cthulhu-rider
force-pushed
the
bugfix/ec-get
branch
from
September 14, 2026 08:17
6204e54 to
51aa3c5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.