dmvk commented on a change in pull request #11533:
URL: https://github.com/apache/beam/pull/11533#discussion_r415812145



##########
File path: 
runners/flink/src/main/java/org/apache/beam/runners/flink/translation/wrappers/streaming/state/FlinkStateInternals.java
##########
@@ -76,8 +76,8 @@
   private final KeyedStateBackend<ByteBuffer> flinkStateBackend;
   private Coder<K> keyCoder;
 
-  // Combined watermark holds for all keys of this partition
-  private final Map<String, Instant> watermarkHolds = new HashMap<>();
+  // Watermark holds for all keys/windows of this partition
+  private final PriorityQueue<Long> watermarkHolds = new PriorityQueue<>();

Review comment:
       👍 that makes sense
   
   It would be nice if we could get rid of `pq.remove(...)` calls as these are 
O(n) and number of keys may be fairly large. How about using `TreeMap<Long, 
Integer>` instead, where value would be number of references to that particular 
offset?
   
   This should have O(log n) characteristic and may hopefully deduplicate some 
of the entries. 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to