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

feat: skip block cache deallocation to make shutdown fast #2683

Merged
merged 7 commits into from
Dec 5, 2024

Conversation

wanghenshui
Copy link
Contributor

@wanghenshui wanghenshui commented Dec 1, 2024

when blockcache size is too big, skip dtor could save shutdown time

ref https://groups.google.com/g/rocksdb/c/xQ_o9jWoFqg

@torwig
Copy link
Contributor

torwig commented Dec 1, 2024

@wanghenshui You can run ./x.py format before committing changes to format the code.

@wanghenshui wanghenshui force-pushed the skip_cache_dtor branch 2 times, most recently from 040b73a to 32314c4 Compare December 1, 2024 17:02
@wanghenshui wanghenshui changed the title [feat] skip rocksdb's cache dtor feat: skip rocksdb's cache dtor to make shutdown fast Dec 1, 2024
git-hulk
git-hulk previously approved these changes Dec 2, 2024
Copy link
Member

@PragmaTwice PragmaTwice left a comment

Choose a reason for hiding this comment

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

Also we need to add a go test case for it.

Copy link
Member

@mapleFU mapleFU left a comment

Choose a reason for hiding this comment

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

PrepareRestoreDB() also call CloseDB, should it prevent from unmanage memory?

@wanghenshui
Copy link
Contributor Author

PrepareRestoreDB() also call CloseDB, should it prevent from unmanage memory?

yes. i 'll make a single function from CloseDB, not inside it

@PragmaTwice PragmaTwice changed the title feat: skip rocksdb's cache dtor to make shutdown fast feat: skip block cache deallocation to make shutdown fast Dec 4, 2024
PragmaTwice
PragmaTwice previously approved these changes Dec 4, 2024
git-hulk
git-hulk previously approved these changes Dec 4, 2024
Copy link
Member

@mapleFU mapleFU left a comment

Choose a reason for hiding this comment

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

General LGTM but maybe in the future we can have better method for this, I think this flag can be set only when cli/main.cc. But this is also ok to me

Copy link

sonarqubecloud bot commented Dec 5, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@git-hulk git-hulk merged commit d3bca42 into apache:unstable Dec 5, 2024
32 of 33 checks passed
@wanghenshui wanghenshui deleted the skip_cache_dtor branch December 5, 2024 08:10
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.

5 participants