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

Make Conduit's rocksdb_max_open_files parameter configurable #2086

Merged
merged 1 commit into from
Sep 2, 2022

Conversation

cvwright
Copy link
Contributor

@cvwright cvwright commented Sep 2, 2022

This PR exposes the Conduit configuration variable rocksdb_max_open_files to the playbook and sets it to a slightly higher default value.

The parameter exists to prevent RocksDB from opening too many files and exceeding the system's maximum number of open files.

By default, Conduit sets the parameter to 20 if no value is given in the config file. Here I'm setting it to 64 instead, so that we avoid creating a bottleneck on larger systems. Some RocksDB documentation suggests allowing for at least one open file per CPU.

@spantaleev spantaleev merged commit f1e294f into spantaleev:master Sep 2, 2022
@spantaleev
Copy link
Owner

Thanks!

Not sure if this is helpful though:

If not specified, Conduit defaults to a relatively low value of 20

It makes it sound like not specifying that variable (or unsetting/clearing) will somehow make it default to 20.

Will it though? Or will rocksdb_max_open_files = cause a configuration syntax error?

@cvwright
Copy link
Contributor Author

cvwright commented Sep 2, 2022

If rocksdb_max_open_files is not in conduit.toml, then Conduit will set it to 20.

If the file says rocksdb_max_open_files = without giving a value, then I expect that's a syntax error.

@spantaleev
Copy link
Owner

I suspect so too, which is why I was thinking that telling people this:

If not specified, Conduit defaults to a relatively low value of 20

right next to the variable definition, could potentially make them want to unset it (in an effort to get the actual default value hardcoded in Conduit), and thus get a syntax error.

This comment being there doesn't seem of much value to anyone.

@cvwright
Copy link
Contributor Author

cvwright commented Sep 3, 2022

right next to the variable definition, could potentially make them want to unset it (in an effort to get the actual default value hardcoded in Conduit), and thus get a syntax error.

This comment being there doesn't seem of much value to anyone.

Ah right, now I see what you mean. Yes, I agree. Please feel free to delete that comment.

Or I can submit another PR to clean up the wording, e.g. "By default, Conduit uses a relatively low value of 20".

spantaleev added a commit that referenced this pull request Sep 5, 2022
@cvwright cvwright deleted the conduit-max-files branch January 11, 2024 20:50
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.

None yet

2 participants