rkhachatryan commented on pull request #16606: URL: https://github.com/apache/flink/pull/16606#issuecomment-918072760
Thanks a lot for the write-up Yuan! To clarify my concerns regarding option 2: States (e.g. `ChangelogValueState`) [connect](https://github.com/apache/flink/blob/7901f26d3470389c7fcfc720e2bab32cc226f1e6/flink-state-backends/flink-statebackend-changelog/src/main/java/org/apache/flink/state/changelog/ChangelogValueState.java#L62) `StateChangelogWriter` with the underlying backend: each state change must be reflected atomically in both. When creating a snapshot, mirroring this atomicity is crucial for correctness. That's why I think it's better to keep these two parts as close to each other as possible (and states are part of the `ChangelogKeyedStateBackend`, while materializer is more independent). In option 3, circular dependency is redundant if `triggerMaterialization` method is removed (that's what I did in the original commit: instead of triggering the materialization, test can [waits](https://github.com/rkhachatryan/flink/commit/61f37810ecccca53791547a052d4e895b8e4f513#diff-8654aaea028ca18fe809d05ce46a4a17cf3a4b18f72ec588f02611d47956eb19R5140) materiazation to happen). So it seems like this approach doesn't have major concerns. -- 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]
