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

Unify blobstor interfaces #1584

Merged
merged 17 commits into from
Aug 22, 2022
Merged

Unify blobstor interfaces #1584

merged 17 commits into from
Aug 22, 2022

Conversation

fyrchik
Copy link
Contributor

@fyrchik fyrchik commented Jul 6, 2022

Close #1523.
Preparation for the #1522 .

@codecov
Copy link

codecov bot commented Jul 6, 2022

Codecov Report

Merging #1584 (a4345ea) into master (5139dc9) will decrease coverage by 0.63%.
The diff coverage is 57.50%.

❗ Current head a4345ea differs from pull request most recent head 1a5949d. Consider uploading reports for the commit 1a5949d to get more accurate results

@@            Coverage Diff             @@
##           master    #1584      +/-   ##
==========================================
- Coverage   33.21%   32.58%   -0.64%     
==========================================
  Files         332      337       +5     
  Lines       22752    22651     -101     
==========================================
- Hits         7558     7380     -178     
- Misses      14574    14658      +84     
+ Partials      620      613       -7     
Impacted Files Coverage Δ
pkg/local_object_storage/blobovnicza/id.go 0.00% <0.00%> (ø)
..._object_storage/blobstor/blobovniczatree/errors.go 100.00% <ø> (ø)
..._object_storage/blobstor/blobovniczatree/exists.go 0.00% <0.00%> (ø)
pkg/local_object_storage/blobstor/delete.go 0.00% <0.00%> (ø)
...kg/local_object_storage/blobstor/fstree/control.go 0.00% <0.00%> (ø)
pkg/local_object_storage/blobstor/fstree/option.go 0.00% <0.00%> (ø)
pkg/local_object_storage/blobstor/get_range.go 0.00% <0.00%> (ø)
pkg/local_object_storage/blobstor/info.go 0.00% <0.00%> (ø)
pkg/local_object_storage/blobstor/mode.go 0.00% <ø> (ø)
pkg/local_object_storage/metabase/version.go 88.88% <ø> (ø)
... and 27 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@carpawell carpawell left a comment

Choose a reason for hiding this comment

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

  1. is it worth changing the writecache behavior in that PR?
  2. don't you think we could get problems with general parameters? (e.g. some Storage implementation starts requiring some parameter that another does not need at all)
  3. commits do not start with the issue number

pkg/local_object_storage/metabase/small.go Outdated Show resolved Hide resolved
pkg/local_object_storage/shard/put.go Outdated Show resolved Hide resolved
pkg/local_object_storage/blobstor/delete.go Show resolved Hide resolved
@fyrchik
Copy link
Contributor Author

fyrchik commented Jul 13, 2022

  1. is it worth changing the writecache behavior in that PR?

Yes, it reduces the amount of code that needs to be written. This implementation was tested on the experimental branch, it leads to a more uniform speed.

  1. don't you think we could get problems with general parameters? (e.g. some Storage implementation starts requiring some parameter that another does not need at all)

This is the point -- each object storage should have the same interface. If any implementation starts requiring some additional parameter, we will provide it in the generic structure, it will be unused by the others. However, I cannot imagine this will ever become true.

  1. commits do not start with the issue number

Fixed.

cthulhu-rider
cthulhu-rider previously approved these changes Jul 15, 2022
@fyrchik
Copy link
Contributor Author

fyrchik commented Jul 25, 2022

Rebased on master.

carpawell
carpawell previously approved these changes Jul 25, 2022
cthulhu-rider
cthulhu-rider previously approved these changes Aug 5, 2022
)

// CConfig represents common compression-related configuration.
type CConfig struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why double C?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was meant as Compression Config and embedded struct named Config would be rather misleading. I guess I'll just remove confusing embeddings and use compression.Config as a name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

require.ErrorIs(t, err, expectedErr)
})
t.Skip()
//dir := t.TempDir()
Copy link
Contributor

Choose a reason for hiding this comment

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

Commented because can't be compiled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it became a bit harder to test. I have created a #1686 to get more test coverage: we could test iterate in every component, and then use a dummy one when testing blobstor. Because in blobstor we are really interested only in how errors from subcomponents can be returned to the caller.

fyrchik and others added 17 commits August 19, 2022 17:23
…arate package

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
… operation

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
…ange` operation

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
…te` operation

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
…ts` operation

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
…e` methods

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
Allow to extend blobstor with more storage sub-systems. Currently
objects stored in the FSTree have empty byte slice descriptor and object
from blobovnicza tree have the same id as earlier. Each such change in
the identifier formation should be accompanied with metabase version
increase.

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
…ate` operation

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
1. Remove in-memory cache. It doesn't persist objects and if we want
   more speed, `NoSync` option can be used for the bolt DB.
2. Put to the metabase in a synchronous fashion. This considerably
   simplifies overall logic and plays nicely with the metabase bolt DB
   batch settings.

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
1. Move compression parameters to the `shard` section.
2. Allow to use multiple sub-storage components in the blobstor.

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
Signed-off-by: Evgenii Stratonikov <evgeniy@morphbits.ru>
@carpawell carpawell mentioned this pull request Sep 14, 2022
aprasolova pushed a commit to aprasolova/neofs-node that referenced this pull request Oct 19, 2022
Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
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.

Put fstree and blobovnicza tree configuration on a single level
3 participants