rkhachatryan commented on a change in pull request #16404:
URL: https://github.com/apache/flink/pull/16404#discussion_r665729745
##########
File path:
flink-state-backends/flink-statebackend-changelog/src/main/java/org/apache/flink/state/changelog/ChangelogKeyedStateBackend.java
##########
@@ -533,7 +541,7 @@ public ChangelogState getExistingState(String name,
BackendStateType type)
ChangelogState state;
switch (type) {
case KEY_VALUE:
- state = (ChangelogState) keyValueStatesByName.get(name);
+ state = changelogStates.get(name);
Review comment:
If TTL is/was enabled, then all states will use TTL serializers. The
difference between `changelogStates` and `keyValueStatesByName` is that the
former does NOT wrap values into TTL values.
On recovery, values read from the changelog are TTL values. So using the
latter would cause double-wrapping.
I agree, this needs to be documented.
As for the method, it can also return new non-restored state. So I'm going
to rename it to `getExistingStateForRecovery`. WDYT?
P.S.: we probably need to rethink TTL later, maybe we don't need to write
TTL to changelog. But that means changing the semantics. And then we probably
would change it for all states to be consistent (FLINK-9661).
--
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]
