Skip to content
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

pool: Drop opening no-op sessions #623

Merged
merged 3 commits into from
Sep 17, 2024
Merged

pool: Drop opening no-op sessions #623

merged 3 commits into from
Sep 17, 2024

Conversation

cthulhu-rider
Copy link
Contributor

No description provided.

Signed-off-by: Leonard Lyubich <[email protected]>
`Table` contains no session token and signature.

Signed-off-by: Leonard Lyubich <[email protected]>
@cthulhu-rider cthulhu-rider changed the title pool: Session-related changes pool: Drop opening no-op sessions Sep 16, 2024
@cthulhu-rider cthulhu-rider marked this pull request as ready for review September 16, 2024 14:43

func (x objectSearchOnlyClientWrapper) getClient() (sdkClientInterface, error) { return x.c, nil }

func TestPool_ObjectSearchInit(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why duplicating prologue in these? Likely we can have less code and still separate subtests for different operations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer independent tests for methods

Copy link
Contributor

@smallhive smallhive left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a feature of the pool that allows autosession. We removing it here, do we need to handle sessions ourselves again?

@roman-khimov
Copy link
Member

Sessions are still there where they're needed. It's just that most requests can be handled fine without sessions.

Currently, there are only two NeoFS operations supporting sessions:
object creation (PUT) and removal (DELETE). The latter is essentially
creation of the tombstone object. Sessions are used in cases when
client requests trusted storage node to form an object and sign it via
so-called private session key. Public part of this key is signed by
original client in the attached session token.

This makes no sense for any other op because server does nothing on
behalf of the client, it just executes his request.

Previously, pool of NeoFS clients could open session for any object op
incl. reading ones. As said, this broke nothing, but was completely
redundant.

This drops session opening for any op except PUT/DELETE to increase
pool's efficiency and relieve network pressure.

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

codecov bot commented Sep 17, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.

Project coverage is 54.13%. Comparing base (ab1e38b) to head (12a4bf2).
Report is 17 commits behind head on master.

Files with missing lines Patch % Lines
pool/object.go 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #623      +/-   ##
==========================================
+ Coverage   53.93%   54.13%   +0.19%     
==========================================
  Files         164      164              
  Lines       19222    19193      -29     
==========================================
+ Hits        10368    10390      +22     
+ Misses       8415     8358      -57     
- Partials      439      445       +6     

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

@cthulhu-rider cthulhu-rider merged commit c0f79fb into master Sep 17, 2024
11 checks passed
@cthulhu-rider cthulhu-rider deleted the proxy-token branch September 17, 2024 09:46
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.

3 participants