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

HDDS-11230. Make RocksDB cache configurable in Ozone Manager. #7117

Closed
wants to merge 2 commits into from

Conversation

weimingdiit
Copy link
Contributor

What changes were proposed in this pull request?

Make RocksDB cache configurable in Ozone Manager.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-11230

How was this patch tested?

unit tests

@weimingdiit
Copy link
Contributor Author

There are some test errors, I will update the code

@errose28
Copy link
Contributor

Hi @weimingdiit thanks for looking in to this. Maybe it would be better to support RocksDB config files directly instead of needing to expose each config through the Ozone configuration? See the RocksDB docs on this for reference. We could first load the options set by the user in this file, and then override any in Ozone that must be set a certain way, for example creating column families that don't exist.

@weimingdiit
Copy link
Contributor Author

weimingdiit commented Aug 30, 2024

Hi @weimingdiit thanks for looking in to this. Maybe it would be better to support RocksDB config files directly instead of needing to expose each config through the Ozone configuration? See the RocksDB docs on this for reference. We could first load the options set by the user in this file, and then override any in Ozone that must be set a certain way, for example creating column families that don't exist.

@errose28 Thank you very much for your comments,this is a good idea. However, for the current version, could we first make some of the more performance-critical parameters configurable, such as the ones I mentioned?

@weimingdiit
Copy link
Contributor Author

@jojochuang Could you help review this PR? This PR is related to some caching in RocksDB. My current thought is to keep the status quo unchanged in this PR and then create a new issue to make all RocksDB parameters configurable. I'd like to hear your thoughts on this—what do you think?

@weimingdiit
Copy link
Contributor Author

Hi @weimingdiit thanks for looking in to this. Maybe it would be better to support RocksDB config files directly instead of needing to expose each config through the Ozone configuration? See the RocksDB docs on this for reference. We could first load the options set by the user in this file, and then override any in Ozone that must be set a certain way, for example creating column families that don't exist.

@errose28 Thank you very much for your comments,this is a good idea. However, for the current version, could we first make some of the more performance-critical parameters configurable, such as the ones I mentioned?

I thought about it, maybe it would be more convenient for us to manage it through ozone configuration, so that our configuration will have only one file, which is easier to manage.

@errose28
Copy link
Contributor

errose28 commented Sep 3, 2024

I'm ok with adding this field in the Ozone configuration to start if there are no immediate plans to support the full RocksDB INI file. We can file that as a follow up Jira. In the future I do think we should support the INI file instead of propagating every RocksDB config we might need into the Ozone configuration. If we start getting into serious RocksDB tuning then there will be too many options in RocksDB for that approach to be sustainable long term.

@weimingdiit
Copy link
Contributor Author

weimingdiit commented Sep 5, 2024

I'm ok with adding this field in the Ozone configuration to start if there are no immediate plans to support the full RocksDB INI file. We can file that as a follow up Jira. In the future I do think we should support the INI file instead of propagating every RocksDB config we might need into the Ozone configuration. If we start getting into serious RocksDB tuning then there will be too many options in RocksDB for that approach to be sustainable long term.

@errose28 This is a good idea. Maybe I will close this PR, and then make all RocksDB parameters configurable in #7150

@kerneltime kerneltime closed this Sep 5, 2024
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.

3 participants