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

storage: Use timeout for boltdb database opening #3148

Merged
merged 1 commit into from
Oct 9, 2023

Conversation

fyfyrchik
Copy link
Contributor

@fyfyrchik fyfyrchik commented Oct 6, 2023

Problem

To dump the DB, the service must be stopped. If this is not the case dump command just hangs without any output.
This may be unexpected from the ops POV (hello, @532910).

Solution

Introduce timeout for bbolt db open. I have decided not to put it in the configuration, because 5 seconds is more than enough, given that flock retry is 50 milliseconds
https://github.com/etcd-io/bbolt/blob/db45d0d6b1f6d173baad00019fdc7c58ad0ab002/db.go#L19
https://github.com/etcd-io/bbolt/blob/db45d0d6b1f6d173baad00019fdc7c58ad0ab002/db.go#L222

Do you think this is worth having in the configuration (like I dump and then stop the service and expect dump to work for such a usecase)?

Copy link
Member

@roman-khimov roman-khimov left a comment

Choose a reason for hiding this comment

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

OK otherwise.

pkg/core/storage/boltdb_store.go Outdated Show resolved Hide resolved
@roman-khimov roman-khimov added the enhancement Improving existing functionality label Oct 6, 2023
@roman-khimov roman-khimov added this to the v0.102.1 milestone Oct 6, 2023
To dump the DB, the service must be stopped.
If this is not the case `dump` command just hangs without any output,
which _may_ be unexpected from the ops POV.
Introduce a 1 second timeous, which is more than enough, given
that bbolt retries doing flock() every 50ms.

Signed-off-by: Evgenii Stratonikov <fyfyrchik@runbox.com>
@codecov
Copy link

codecov bot commented Oct 9, 2023

Codecov Report

Merging #3148 (f1c6b9f) into master (91c928e) will decrease coverage by 0.01%.
Report is 4 commits behind head on master.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3148      +/-   ##
==========================================
- Coverage   84.84%   84.84%   -0.01%     
==========================================
  Files         330      330              
  Lines       44332    44337       +5     
==========================================
+ Hits        37615    37617       +2     
- Misses       5204     5206       +2     
- Partials     1513     1514       +1     
Files Coverage Δ
pkg/core/storage/boltdb_store.go 78.76% <100.00%> (+0.38%) ⬆️

... and 4 files with indirect coverage changes

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

@roman-khimov roman-khimov merged commit cef7009 into nspcc-dev:master Oct 9, 2023
13 of 18 checks passed
@fyfyrchik fyfyrchik deleted the fix-dump branch October 9, 2023 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improving existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants