1996fanrui commented on code in PR #26831:
URL: https://github.com/apache/flink/pull/26831#discussion_r2230455833
##########
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/state/rocksdb/AbstractRocksDBState.java:
##########
@@ -176,7 +176,10 @@ <T> byte[] serializeValueNullSensitive(T value,
TypeSerializer<T> serializer)
throws IOException {
dataOutputView.clear();
dataOutputView.writeBoolean(value == null);
- return serializeValueInternal(value, serializer);
+ if (value != null) {
+ serializer.serialize(value, dataOutputView);
+ }
+ return dataOutputView.getCopyOfBuffer();
Review Comment:
Currently, hashmap, RocksDB and ForSt are compatible with the case where the
user value of the map state is null (Including write, read, checkpoint and
restore), but it has an asymmetry that causing a NPE issue. Here is core
change, and ForSt is same. （Get more details from PR description.）
Unfortunately, change log state backend is not compatible with the case
where the user value of the map state is null [1]. It could be improved to be
compatible with this case, but it will break the serialization format and
introduce some migration issues. I disabled `testMapStateWithNullValue` for
change log state backend to avoid the CI is broken in this PR.
Hey @Zakelly , I hope to hear your opinion on how to move forward with the
change log state backend?
If it is easier to solve, I would prefer to fix all problems of rocksdb,
ForSt and change log together in the current PR. But if it is not, it might
make more sense to fix the problems with rocksdb and ForSt serializers first.
WDYT?
[1]
https://github.com/apache/flink/blob/31785e076c86d0a44e3f4a17f44a04908a2d3eb4/flink-state-backends/flink-statebackend-changelog/src/main/java/org/apache/flink/state/changelog/ChangelogMapState.java#L224
--
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]
