[GitHub] [beam] je-ik commented on a change in pull request #15994: [BEAM-13263] Support OnWindowExpiration in (non-portable) Flink runner

[GitBox <gi...@apache.org>]

je-ik commented on a change in pull request #15994:
URL: https://github.com/apache/beam/pull/15994#discussion_r767826049



##########
File path: runners/flink/src/main/java/org/apache/beam/runners/flink/translation/wrappers/streaming/state/FlinkStateInternals.java
##########
@@ -215,6 +216,22 @@ public void clearGlobalState() {
     }
   }
 
+  public List<ByteBuffer> getGlobalWindowStateKeys() {

Review comment:
       I would warn against picking random type of state and declare it invariant, that it must be present for all keys. Even if it works today, it might be subject to change and hard-to-debug errors. Although watermark hold looks like a pretty good candidate, I would pick it only as a last resort option.
   
   The call `flinkStateBackend.applyToAllKeys` looks like it would deserve a better name, if it is applied to all "keys and states". :) The optimization could be done there, maybe the state backend knows it has sorted keys, and can therefore do the deduplication cheaper.
   
   Until then I would suggest explicit deduplication, that should be O(n log n), which should be fine.
   
   




-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org