StefanRRichter commented on code in PR #22771:
URL: https://github.com/apache/flink/pull/22771#discussion_r1228267599
##########
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/RocksDBWriteBatchWrapper.java:
##########
@@ -79,7 +82,16 @@ public RocksDBWriteBatchWrapper(
Preconditions.checkArgument(batchSize >= 0, "Max batch size have to be
no negative.");
this.db = rocksDB;
- this.options = options;
+ if (options != null) {
+ this.options = options;
+ // ensure we don't close the object we don't own
+ this.ownsWriteOptions = false;
+ } else {
+ // Use default write options with disabled WAL
+ this.options = new WriteOptions().setDisableWAL(true);
+ // ensure we close the object we own
+ this.ownsWriteOptions = true;
+ }
Review Comment:
I was starting to implement this in the way you suggested, but the backend
WriteOptions are currently managed by `RocksDBResourceContainer` and calls to
the getter of WriteOptions always create new object and the ownership is
claimed by the container. If we don't want to create multiple objects that
stick around until the backend is disposed, we would have to create the options
before creating the backend and passing it in everywhere. Then the ownership
would be pretty hard to determine along the way. I think this is actually the
safest option for the change and the closest to the current code.
--
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]
