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: export persist_period_sec option and background_threads #448
feat: export persist_period_sec option and background_threads #448
Conversation
Thanks for the contributing. Overall looks good. But could you add some tests ? |
@aleksuss Yes! I'll add now. thanks for the quick feedback |
2181a20
to
20773cf
Compare
6f0a88a
to
e2518a1
Compare
…cksdb#447 - [x] add test disable all threads on get_with_cache_and_bulkload_test - [x] add test for stats_persist_period_sec
e2518a1
to
182922f
Compare
Summary: fixed - rust-rocksdb/rust-rocksdb#447 - rust-rocksdb/rust-rocksdb#448 Pull Request resolved: #7168 Reviewed By: cheng-chang Differential Revision: D22736013 Pulled By: ajkr fbshipit-source-id: fdd784aa75d26a367b9108b05ffdd94a2ae117d3
@developerfred expose
Without |
@ailisp I will open a new pull request to export the |
@ailisp I exported how |
It might be good idea to follow their naming (lines before your change) void rocksdb_env_set_high_priority_background_threads(rocksdb_env_t* env, int n) {
env->rep->SetBackgroundThreads(n, Env::HIGH);
} Also please also export |
@ailisp done 👍🏾 - |
Summary: - rust-rocksdb/rust-rocksdb#448 Pull Request resolved: #7191 Reviewed By: riversand963 Differential Revision: D22809066 Pulled By: ajkr fbshipit-source-id: 036939f9a28cacc3f677c318d1aed97fe5f4f85e
5e5a34b
to
e66b37b
Compare
@developerfred My bad in test, your PRs fully works, it just takes some time to wait rocksdb destroy other threads (wait some seconds and lldb attach there's only one thread). I'm very sorry of that. So no need to export Env::Default (ust-rocksdb/rust-rocksdb#448) |
@ailisp ok! No problem! |
@developerfred We've successfully use your fork on our repo, Thank you! looking forward to next rust-rocksdb release, to upgrade your changes in c++ rocksdb and land this PR |
@stanislav-tkach Facebook has released rocksdb containing @developerfred 's change and it's been bumped in rust-rocksdb, let's merge this PR? (@developerfred Would you mind resolve conflict? I cannot push to your branch, if you don't have time I can fork from your fork, resolve conflict and make a new PR, then close this one) |
I solve this now, thanks for information and ping. |
@developerfred apply please suggestions by |
ok @aleksuss |
e1afc19
to
aaa49ce
Compare
df09aa1
to
f37377a
Compare
Thank you @developerfred and @aleksuss ! |
Summary: fixed - rust-rocksdb/rust-rocksdb#447 - rust-rocksdb/rust-rocksdb#448 Pull Request resolved: facebook#7168 Reviewed By: cheng-chang Differential Revision: D22736013 Pulled By: ajkr fbshipit-source-id: fdd784aa75d26a367b9108b05ffdd94a2ae117d3
Summary: - rust-rocksdb/rust-rocksdb#448 Pull Request resolved: facebook#7191 Reviewed By: riversand963 Differential Revision: D22809066 Pulled By: ajkr fbshipit-source-id: 036939f9a28cacc3f677c318d1aed97fe5f4f85e
…-rocksdb#448)" This reverts commit ccf6ae3.
exported functions:
set_low_priority_background_threads
set_bottom_priority_background_threads
env_default
feat: export Env::default() facebook/rocksdb#7207set_stats_persist_period_sec
close #447
Export PR on RocksDB
facebook/rocksdb#7168