NicoK commented on a change in pull request #36: URL: https://github.com/apache/flink-training/pull/36#discussion_r700778445
########## File path: long-ride-alerts/DISCUSSION.md ########## @@ -21,25 +21,30 @@ under the License. (Discussion of [Lab: `KeyedProcessFunction` and Timers (Long Ride Alerts)](./)) -Flaws in the reference solutions: +### Flaws in the solutions -* The reference solutions leak state in the case where a START event is missing. -* In the case where the END event eventually arrives, but after the timer -has fired and has cleared the matching START event, then a duplicate alert is generated. +*The reference solutions can leak state when an event is missing.* -A good way to write unit tests for a `KeyedProcessFunction` to check for state retention, etc., is to -use the test harnesses described in the -[documentation on testing](https://ci.apache.org/projects/flink/flink-docs-stable/dev/stream/testing.html#unit-testing-stateful-or-timely-udfs--custom-operators). - -These issues could be addressed by keeping some state longer, and then either +This could be addressed by either using [state TTL](https://ci.apache.org/projects/flink/flink-docs-stable/dev/stream/state/state.html#state-time-to-live-ttl), or another timer, to eventually clear any lingering state. -But regardless of how long we retain the state, we must eventually clear it, and thereafter we would +*These solutions also leak state whenever there is a long ride, unless the END event is missing.* Review comment: The sentence is still misleading as it suggests that you always leak state for every long ride but this is only true if the start event is missing or if the end event is late. Long rides with both events are cleaned up in `processElement`. The text should thus go through these (or reflect them somehow): - start event missing -> end event sits in state (leak!) - end event missing -> timer fires and clears state (ok) - end event late -> timer fires, clears state, end event arrives and is stored in state (leak!) -- 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]
