curcur commented on pull request #15200: URL: https://github.com/apache/flink/pull/15200#issuecomment-855628016
Thanks @rkhachatryan for addressing the comments! Some remaining points to discuss offline 1. why "MERGE_NS" is named "stateMerged" instead of "mergeNamespaces" 2. Where to loggingIterator/loggingIterable/loggingMapEntry should be put within the interface of "StateChangeLogger" 3. ChangelogAggregatingState.add()/ChangelogReducingState.add() using update instead of add to implement, might have differences on ChangelogAggregatingState. The reason is to log incremental changes instead of updated states. Details see in-line comment. 4. more explicit options. MAP_PUT LIST_APPEND MAP_REMOVE .... 5. About Iterator Discussion 1). ChangelogKeyGroupedPriorityQueue.iterator(); is it changeable? 2). When an iterator can be changed. remove and update entry? (Most of my concerns have already been addressed in last round of review, just double confirm) 6. How is update different from updateInternal ChangelogListState.update/ChangelogListState.updateInternal From reading their comments, it seems they are different somehow, but the changelogger seems to be same. 7. Specific question on map iterator. See in-line comments for details. Some backend supports iterator change/remove; but some does not, but this is not the main point, I am asking in the case where iterator change/remove are supported In the case where the iterator supports changes Let's say I get an iterator list of keys, and I get an iterator list of values, I remove some keys in the key iterator, and some values in the value iterator Is it guaranteed that such remove is always paired? (Key, Value) -- 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: us...@infra.apache.org
