StefanRRichter commented on a change in pull request #7586:
[FLINK-10912][rocksdb] Configurable RocksDBStateBackend options
URL: https://github.com/apache/flink/pull/7586#discussion_r258871077
##########
File path:
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBStateBackend.java
##########
@@ -331,9 +334,18 @@ private RocksDBStateBackend(RocksDBStateBackend original,
Configuration config,
// configure metric options
this.defaultMetricOptions =
RocksDBNativeMetricOptions.fromConfig(config);
- // copy remaining settings
- this.predefinedOptions = original.predefinedOptions;
- this.optionsFactory = original.optionsFactory;
+ // configure RocksDB predefined options
+ this.predefinedOptions = original.predefinedOptions == null ?
+
PredefinedOptions.valueOf(config.getString(RocksDBOptions.PREDEFINED_OPTIONS))
: original.predefinedOptions;
+
+ // configure RocksDB options factory
+ try {
+ this.optionsFactory = original.optionsFactory == null ?
+
loadOptionsFactory(config.getString(RocksDBOptions.OPTIONS_FACTORY), config,
classLoader) :
+ original.optionsFactory;
Review comment:
Wouldn't it make more sense to also appy the check of
`ConfigurableOptionsFactory` and calling configure in case it was set by the
user and not loaded dynamically? Because users might also set a factory in code
hthat they expect to be configured from the conf.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
