Skip to content

PUT server code improvements #3506

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

Draft
wants to merge 27 commits into
base: master
Choose a base branch
from
Draft

PUT server code improvements #3506

wants to merge 27 commits into from

Conversation

cthulhu-rider
Copy link
Contributor

continues #3457. I'd like to have this before #3504

Use one instruction to calculate the flag. Also, do it per-object and do
not store in the struct field. In addition, evaluate only for non-local
requests.

Signed-off-by: Leonard Lyubich <[email protected]>
Will be useful for EC policies in #3420.

Signed-off-by: Leonard Lyubich <[email protected]>
So it can be reused.

Signed-off-by: Leonard Lyubich <[email protected]>
This extends PUT server logic to comply with EC policies - independently
or together with REP ones. This logic will remain inactive until it
becomes possible to create containers with EC policies.

This works for small objects only: encoding and distribution will be the
same for big objects, but SNs will deny them without proper adaptation.

Closes #3419. Refs #3420.

Signed-off-by: Leonard Lyubich <[email protected]>
On the one hand, unconditional locking seems safer. From the other one,
such code could create a false impression that the set of signatures may
change after being sent to the contract. However:
 1. this cannot happen;
 2. even if it did, it would be wrong behavior.

Since lock is acquired for writing only now, `sync.RWMutex` is replaced
with faster `sync.Mutex`.

Signed-off-by: Leonard Lyubich <[email protected]>
The parameter is one and required.

Signed-off-by: Leonard Lyubich <[email protected]>
So that it is clear what must be specified and what is not.

Signed-off-by: Leonard Lyubich <[email protected]>
Method did not check pointer for nil. Anyway, it is never nil.

Signed-off-by: Leonard Lyubich <[email protected]>
It's clearer this way.

Signed-off-by: Leonard Lyubich <[email protected]>
There is only one result.

Signed-off-by: Leonard Lyubich <[email protected]>
Previously, it was checked by internal processor and had disadvantages
described below.

PUT handler detected a flow violation too late. For example, on
repeated heading message receipt, the server done resource-intensive
checks of the request signatures and body. Although flow check is
extremely lightweight.

The internal processor is also used for internal SN needs. It always
uses it correctly, therefore checks are unnecessary.

Now stream control is done instantly upon receipt of the next message.

Signed-off-by: Leonard Lyubich <[email protected]>
`internal.Target` is an implementation detail that should not be
included in server responses. Although currently server does not attach
them to the response status, after #2744 it would have shown up.

Signed-off-by: Leonard Lyubich <[email protected]>
In addition to having fewer methods, it is now impossible to pass chunk
or close before the header. This reduces the likelihood of dev mistakes.

Signed-off-by: Leonard Lyubich <[email protected]>
The field was used in single method, so was no sense in being a field.

Signed-off-by: Leonard Lyubich <[email protected]>
More accepted approach in Go.

Signed-off-by: Leonard Lyubich <[email protected]>
It became a redundant wrapper around `Service` type.

Signed-off-by: Leonard Lyubich <[email protected]>
The original intention was to remove the nil check and chaining within
the setters themselves. However, replacing them removes even more code.

Signed-off-by: Leonard Lyubich <[email protected]>
Use `Service` field instead. Although not all fields may be needed, this
is a simpler approach anyway.

Signed-off-by: Leonard Lyubich <[email protected]>
To make it clearer what it locks.

Signed-off-by: Leonard Lyubich <[email protected]>
There are a lot of fields, so at least the state variables are separated.

Signed-off-by: Leonard Lyubich <[email protected]>
No longer need to have private `PutInitOptions` fields with this anymore.

Signed-off-by: Leonard Lyubich <[email protected]>
Copy link

codecov bot commented Aug 6, 2025

Codecov Report

❌ Patch coverage is 61.04167% with 187 lines in your changes missing coverage. Please review.
✅ Project coverage is 23.79%. Comparing base (5d20065) to head (00af94f).
⚠️ Report is 10 commits behind head on master.

Files with missing lines Patch % Lines
pkg/services/object/put/distributed.go 53.70% 43 Missing and 7 partials ⚠️
pkg/services/object/put/streamer.go 54.08% 31 Missing and 14 partials ⚠️
pkg/services/object/put/ec.go 53.40% 30 Missing and 11 partials ⚠️
pkg/services/object/server.go 0.00% 30 Missing ⚠️
internal/ec/ec.go 62.50% 8 Missing and 4 partials ⚠️
cmd/neofs-node/object.go 0.00% 5 Missing ⚠️
pkg/services/object/delete/util.go 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3506      +/-   ##
==========================================
+ Coverage   23.46%   23.79%   +0.32%     
==========================================
  Files         669      674       +5     
  Lines       50243    50448     +205     
==========================================
+ Hits        11788    12002     +214     
+ Misses      37535    37511      -24     
- Partials      920      935      +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant