Myasuka commented on code in PR #22458:
URL: https://github.com/apache/flink/pull/22458#discussion_r1180397914
##########
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBResourceContainer.java:
##########
@@ -114,14 +117,18 @@ public RocksDBResourceContainer(
this.handlesToClose = new ArrayList<>();
}
- /** Gets the RocksDB {@link DBOptions} to be used for RocksDB instances. */
public DBOptions getDbOptions() {
+ return getDbOptions(null);
+ }
+
+ /** Gets the RocksDB {@link DBOptions} to be used for RocksDB instances. */
+ public DBOptions getDbOptions(File instanceRocksDBPath) {
Review Comment:
To be honest, I don't like the introduced method of `getDbOptions(File
instanceRocksDBPath)`, which is a bit weird.
Since
[RocksDBResourceContainer](https://github.com/apache/flink/blob/3664609c7622ccae80e36e85099a1b79b5935fe9/flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/EmbeddedRocksDBStateBackend.java#L474-L476)
is created after the
[instanceBasePath](https://github.com/apache/flink/blob/3664609c7622ccae80e36e85099a1b79b5935fe9/flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/EmbeddedRocksDBStateBackend.java#L455-L463)
in the same class, why not making the `instanceBasePath` as part of
`RocksDBResourceContainer`, just like the passed `ReadableConfig`? If so, we
don't need to introduce another method here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
