NicoK commented on a change in pull request #36: URL: https://github.com/apache/flink-training/pull/36#discussion_r700536930
########## 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: That sounds wrong...did you mean this? ```suggestion *These solutions also leak state whenever there is a long ride and the END event is missing.* ``` -- 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]
