Skip to content

Unify blobstor interfaces#1584

Merged
fyrchik merged 17 commits intonspcc-dev:masterfrom
fyrchik:config-refactor
Aug 22, 2022
Merged

Unify blobstor interfaces#1584
fyrchik merged 17 commits intonspcc-dev:masterfrom
fyrchik:config-refactor

Conversation

@fyrchik
Copy link
Contributor

@fyrchik fyrchik commented Jul 6, 2022 •
edited
Loading

Close #1523.
Preparation for the #1522 .

@codecov
Copy link

codecov bot commented Jul 6, 2022 •
edited
Loading

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

carpawell
carpawell reviewed Jul 12, 2022
View reviewed changes
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

@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
View reviewed changes
@fyrchik
Copy link
Contributor Author

fyrchik commented Jul 25, 2022

Rebased on master.

carpawell
carpawell previously approved these changes Jul 25, 2022
View reviewed changes
cthulhu-rider
cthulhu-rider previously approved these changes Aug 5, 2022
View reviewed changes
pkg/local_object_storage/blobstor/compression/compress.go Outdated
)

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

@cthulhu-rider cthulhu-rider Aug 5, 2022

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

@fyrchik fyrchik Aug 9, 2022

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

@fyrchik fyrchik Aug 19, 2022

Choose a reason for hiding this comment

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

Fixed

pkg/local_object_storage/blobstor/iterate_test.go
require.ErrorIs(t, err, expectedErr)
})
t.Skip()
//dir := t.TempDir()
Copy link
Contributor

@cthulhu-rider cthulhu-rider Aug 5, 2022

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

@fyrchik fyrchik Aug 19, 2022

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.

This was referenced Aug 15, 2022
Closed
Closed
fyrchik and others added 17 commits August 19, 2022 17:23
@fyrchik
[nspcc-dev#1523] local_object_storage: Move blobovnicza tree to a sep…
d136fde 
…arate package

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
@fyrchik
[nspcc-dev#1523] local_object_storage: Unify parameters for the Get…
7e4059e 
… operation

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
@fyrchik
[nspcc-dev#1523] local_object_storage: Unify parameters for the `GetR…
8e81f8f 
…ange` operation

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
@fyrchik
[nspcc-dev#1523] local_object_storage: Unify parameters for the `Dele…
c01f990 
…te` operation

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
@fyrchik
[nspcc-dev#1523] local_object_storage: Unify parameters for the `Exis…
e28c221 
…ts` operation

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
@fyrchik
[nspcc-dev#1523] blobstor: Export single Get, GetRange and `Delet…
4c01821 
…e` methods

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
@fyrchik
[nspcc-dev#1523] blobstor: Unify parameters for Put operation
778cdce 
Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
@fyrchik
[nspcc-dev#1523] shard: Store generic storage ID in metabase
f9bc77c 
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>
@fyrchik
[nspcc-dev#1523] local_object_storage: Unify parameters for the `Iter…
6baaa9b 
…ate` operation

Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
@fyrchik
[nspcc-dev#1523] writecache: Simplify logic
6543f4b 
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>
@fyrchik
[nspcc-dev#1523] writecache: Remove cloneBytes
1610ed0 
Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
@fyrchik
[nspcc-dev#1523] blobstor: Unify fstree and blobovnicza interfaces
dcb07cd 
Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
@fyrchik
[nspcc-dev#1523] blobstor: Unify request dispatch logic
3939a85 
Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
@fyrchik
[nspcc-dev#1523] blobstor: Remove unused types
0a81a20 
Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
@fyrchik
[nspcc-dev#1523] neofs-node: Refactor configuration
0315572 
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>
@fyrchik
[nspcc-dev#1584] blobovniczatree: move public methods to different files
49eed1d 
Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
@fyrchik
[nspcc-dev#1523] blobstor: Rename compression.CConfig to Config
1a5949d 
Signed-off-by: Evgenii Stratonikov <evgeniy@morphbits.ru>
@fyrchik fyrchik merged commit e8c6dce into nspcc-dev:master Aug 22, 2022
@fyrchik fyrchik mentioned this pull request Aug 22, 2022
Closed
@fyrchik fyrchik mentioned this pull request Aug 24, 2022
Closed
@carpawell carpawell mentioned this pull request Sep 14, 2022
Merged
aprasolova pushed a commit to aprasolova/neofs-node that referenced this pull request Oct 19, 2022
@fyrchik
[nspcc-dev#1584] blobovniczatree: move public methods to different files
ab1dd12 
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

Reviewers

@carpawell carpawell carpawell approved these changes

@cthulhu-rider cthulhu-rider Awaiting requested review from cthulhu-rider cthulhu-rider is a code owner

Assignees

@fyrchik fyrchik

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Put fstree and blobovnicza tree configuration on a single level

3 participants

@fyrchik @carpawell @cthulhu-rider