Optimize buffering of forwarded SEARCH requests - #4116
Conversation
4347a87 to
1695f41
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #4116 +/- ##
==========================================
- Coverage 27.19% 27.13% -0.06%
==========================================
Files 677 677
Lines 46206 46312 +106
==========================================
+ Hits 12564 12567 +3
- Misses 32442 32546 +104
+ Partials 1200 1199 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1695f41 to
5d595c6
Compare
roman-khimov
left a comment
roman-khimov
left a comment
There was a problem hiding this comment.
We need to share this code with SDK, it's unmaintainable otherwise.
|
|
||
| func (s *Server) writeLocalSearchObjectsRequest(buf []byte, bodyLen int, body *protoobject.SearchV2Request_Body, metaHdrLen int) (int, error) { | ||
| // TODO: can be calculated once and reused | ||
| originSig, err := neofsecdsa.Signer(s.signer).Sign(nil) |
this is planned to be done in #4005 |
|
Let's solve #4005 first then. |
i don't think it's worth blocking optimizations with refactorings |
|
It's too easy to lose control over the codebase this way, we need to structure our changes appropriately as early as possible. |
agree. But at the same time the more usecases are created, the easier it will be to form libs i'll try my best to synthesize SDK packages asap |
5d595c6 to
ff70559
Compare
| } | ||
| if n != len(buf) { | ||
| return nil, fmt.Errorf("got wrong request length: expected %d, got %d", n, len(buf)) | ||
| return nil, newWrongRequestLengthError(len(buf), n) |
There was a problem hiding this comment.
one more usecase appears
There was a problem hiding this comment.
what this error means for a user btw? isnt it a panic?
There was a problem hiding this comment.
internal server error. Can be a panic. Should never happen anyway
ff70559 to
381aa09
Compare
| 1 + maxGetResponseChunkVarintLen // 1 for iprotobuf.TagBytes2 | ||
| getResponseChunkBufferLen = maxChunkOffsetInGetResponse + maxGetResponseChunkLen + maxResponseVerificationHeaderLen | ||
|
|
||
| modernRequestVerificationSignatureCount = 2 |
There was a problem hiding this comment.
make it api dependent? we will have nspcc-dev/neofs-api#406 soon, it will then be "the most modern"
There was a problem hiding this comment.
it is API dependent. To support new API feature, future change is inevitable
| MarshalStable([]byte) | ||
| } | ||
|
|
||
| func writeStablyMarshalledField(buf []byte, tag byte, ln int, fld stablyMarshalledFIeld) int { |
There was a problem hiding this comment.
can ln and fld merged into a single interface? likely every our struct will handle it ok
There was a problem hiding this comment.
length of embedded messages is always needed twice: to calc whole message len and to write field prefix. So, i dont like the idea to calc it twice
| } | ||
| if n != len(buf) { | ||
| return nil, fmt.Errorf("got wrong request length: expected %d, got %d", n, len(buf)) | ||
| return nil, newWrongRequestLengthError(len(buf), n) |
There was a problem hiding this comment.
what this error means for a user btw? isnt it a panic?
Server-side part of nspcc-dev/neofs-sdk-go@c4dda73. Additionally de-duplicates the request encoding for sending to multiple nodes. Signed-off-by: Leonard Liubich <leonard@morphbits.io>
381aa09 to
4c3813e
Compare
roman-khimov
left a comment
roman-khimov
left a comment
There was a problem hiding this comment.
Needs to be shared with SDK code.
3 participants
No description provided.