[#253] (Draft) feat(RocksDBKvBackend): make rocksdb.Options, WriteOptions, ReadOptions configurable#2607
[#253] (Draft) feat(RocksDBKvBackend): make rocksdb.Options, WriteOptions, ReadOptions configurable#2607unknowntpo wants to merge 13 commits intoapache:mainfrom
Conversation
|
@yuqi1129 This is the draft PR for making |
| private void initializeOptionSetters() { | ||
| // Each option name maps to a lambda that applies the setting to the appropriate | ||
| // option object | ||
| optionSetters.put("gravitino.entity.store.kv.rocksdb.options.maxBackgroundJobs", |
There was a problem hiding this comment.
I believe you can shorten it to options.maxBackgroundJobs or maxBackgroundJobs.
| if (configMap.containsKey(optionKey)) { | ||
| this.optionSetters.get(optionKey).accept(this, configMap.get(optionKey)); | ||
| } else { | ||
| this.optionSetters.get(optionKey).accept(this, null); |
There was a problem hiding this comment.
No need to set null value. Please remove this line directly.
There was a problem hiding this comment.
okay, I'll fix it.
| * Apply user-defined options to option if this options is configurable. | ||
| * TODO: List all configurable options. | ||
| */ | ||
| public void setOptions(Config config) { |
There was a problem hiding this comment.
How do you distinguish between a write option and a read option?
There was a problem hiding this comment.
My bad, this method is responsible for applying config to rocksdb.Options, rocksdb.WriteOptions, rocksdb.ReadOptions.
So I the the method name could be changed to applyUserConfig ?
And if we need to add more options to rocksdb.Options, rocksdb.WriteOptions, rocksdb.ReadOptions.
just add more entries in initializeOptionSetters.
e.g.
optionSetters.put("gravitino.entity.store.kv.rocksdb.options.maxBackgroundJobs",
(holder, value) -> {
holder.options.setMaxBackgroundJobs(Integer.parseInt(value));
});
optionSetters.put("gravitino.entity.store.kv.rocksdb.writeOptions.noSlowDown",
(holder, value) -> {
holder.writeOptions.setNoSlowDown(Boolean.parseBoolean(value));
});
optionSetters.put("gravitino.entity.store.kv.rocksdb.readOptions.autoPrefixMode",
(holder, value) -> {
holder.readOptions.setAutoPrefixMode(Boolean.parseBoolean(value));
});
unknowntpo
force-pushed
the
feat-rocksdb-options
branch
from
ab2c3fd to
4a0e733
Compare
unknowntpo
force-pushed
the
feat-rocksdb-options
branch
from
1385549 to
e34a194
Compare
unknowntpo
force-pushed
the
feat-rocksdb-options
branch
2 times, most recently
from
e827ead to
79e6c52
Compare
unknowntpo
force-pushed
the
feat-rocksdb-options
branch
from
79e6c52 to
aae8ce0
Compare
|
@yuqi1129 I've added all configs from |
Okay, I will review it soon, thanks |
3 participants
What changes were proposed in this pull request?
This PR makes
rocksdb.Options,WriteOptions,ReadOptionsconfigurableWhy are the changes needed?
see #253 for more information.
Does this PR introduce any user-facing change?
(Please list the user-facing changes introduced by your change, including
User can set
Options,WriteOptions,ReadOptionsin the gravatino configuration file.e.g.
How was this patch tested?
Test methods in
RocksDBOptionsclass with mocks.References: