StefanRRichter commented on a change in pull request #7586: 
[FLINK-10912][rocksdb] Configurable RocksDBStateBackend options
URL: https://github.com/apache/flink/pull/7586#discussion_r257695837
 
 

 ##########
 File path: 
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBStateBackend.java
 ##########
 @@ -317,9 +318,13 @@ 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
+               this.optionsFactory = original.optionsFactory == null ?
+                       ConfigurableOptionsFactory.fromConfig(config) : 
original.optionsFactory;
 
 Review comment:
   Here we could check if the user specified a different classname to choose a 
different `OptionsFactory` (other than `ConfigurableOptionsFactory`) through 
the config and not through code. Still, code should override config in this 
case.

----------------------------------------------------------------
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

Reply via email to