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

Writecache fixes #1745

Merged
merged 7 commits into from
Sep 2, 2022
Merged

Writecache fixes #1745

merged 7 commits into from
Sep 2, 2022

Conversation

fyrchik
Copy link
Contributor

@fyrchik fyrchik commented Sep 1, 2022

There are 2 problems with the current flush logic:

  1. Objects are evicted from the cache even if PUT to the main storage failed.
  2. There is a race condition, so that where an object could be silently evicted from LRU without removal from the storage.

In this PR we change this logic in 2 aspects:

  1. Removing evicted items is done in the eviction callback.
  2. The removal itself is done in batches to avoid blocking client threads.

Set flush mark in the inside the flush worker because writing to the blobstor
can fail. Because each evicted object must be deleted, it is reasonable
to do this in the evict callback.

The evict callback is protected by LRU mutex and thus potentially interferes
with `Get` and `Iterate` methods. This problem will be addressed in the
future.

Signed-off-by: Evgenii Stratonikov <evgeniy@morphbits.ru>
Remove unused option and additional pointers to db/fstree.

Signed-off-by: Evgenii Stratonikov <evgeniy@morphbits.ru>
@codecov
Copy link

codecov bot commented Sep 1, 2022

Codecov Report

Merging #1745 (36df737) into master (180e5e9) will increase coverage by 0.01%.
The diff coverage is 43.37%.

@@            Coverage Diff             @@
##           master    #1745      +/-   ##
==========================================
+ Coverage   33.16%   33.18%   +0.01%     
==========================================
  Files         351      350       -1     
  Lines       23384    23411      +27     
==========================================
+ Hits         7756     7768      +12     
- Misses      14984    14992       +8     
- Partials      644      651       +7     
Impacted Files Coverage Δ
cmd/neofs-adm/internal/modules/morph/initialize.go 0.00% <0.00%> (ø)
pkg/local_object_storage/shard/delete.go 69.04% <0.00%> (ø)
pkg/local_object_storage/shard/head.go 70.58% <0.00%> (ø)
pkg/local_object_storage/writecache/flush.go 23.01% <0.00%> (+0.35%) ⬆️
pkg/local_object_storage/writecache/options.go 28.57% <ø> (+4.18%) ⬆️
pkg/local_object_storage/writecache/storage.go 28.16% <3.03%> (+3.77%) ⬆️
...ct_storage/blobstor/blobovniczatree/blobovnicza.go 60.75% <62.50%> (-12.93%) ⬇️
...object_storage/blobstor/blobovniczatree/control.go 68.49% <78.26%> (-0.36%) ⬇️
pkg/local_object_storage/shard/get.go 77.96% <100.00%> (ø)
pkg/local_object_storage/writecache/state.go 33.33% <100.00%> (-1.97%) ⬇️
... and 9 more

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

carpawell
carpawell previously approved these changes Sep 1, 2022
…iguration

It is unused since ddaed28 .

Signed-off-by: Evgenii Stratonikov <evgeniy@morphbits.ru>
Signed-off-by: Evgenii Stratonikov <evgeniy@morphbits.ru>
Signed-off-by: Evgenii Stratonikov <evgeniy@morphbits.ru>
We specify the error in the doc-comment, and it is the same for all our
components.

Signed-off-by: Evgenii Stratonikov <evgeniy@morphbits.ru>
Signed-off-by: Evgenii Stratonikov <evgeniy@morphbits.ru>
@fyrchik fyrchik merged commit e6cf0b3 into nspcc-dev:master Sep 2, 2022
@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
Set flush mark in the inside the flush worker because writing to the blobstor
can fail. Because each evicted object must be deleted, it is reasonable
to do this in the evict callback.

The evict callback is protected by LRU mutex and thus potentially interferes
with `Get` and `Iterate` methods. This problem will be addressed in the
future.

Signed-off-by: Evgenii Stratonikov <evgeniy@morphbits.ru>
aprasolova pushed a commit to aprasolova/neofs-node that referenced this pull request Oct 19, 2022
Remove unused option and additional pointers to db/fstree.

Signed-off-by: Evgenii Stratonikov <evgeniy@morphbits.ru>
aprasolova pushed a commit to aprasolova/neofs-node that referenced this pull request Oct 19, 2022
…iguration

It is unused since ddaed28 .

Signed-off-by: Evgenii Stratonikov <evgeniy@morphbits.ru>
aprasolova pushed a commit to aprasolova/neofs-node that referenced this pull request Oct 19, 2022
Signed-off-by: Evgenii Stratonikov <evgeniy@morphbits.ru>
aprasolova pushed a commit to aprasolova/neofs-node that referenced this pull request Oct 19, 2022
Signed-off-by: Evgenii Stratonikov <evgeniy@morphbits.ru>
aprasolova pushed a commit to aprasolova/neofs-node that referenced this pull request Oct 19, 2022
We specify the error in the doc-comment, and it is the same for all our
components.

Signed-off-by: Evgenii Stratonikov <evgeniy@morphbits.ru>
aprasolova pushed a commit to aprasolova/neofs-node that referenced this pull request Oct 19, 2022
Signed-off-by: Evgenii Stratonikov <evgeniy@morphbits.ru>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
neofs-storage Storage node application issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants