You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@beam.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2020/11/03 13:58:00 UTC

[jira] [Work logged] (BEAM-11143) AfterWatermark can leak state for the late trigger when merging windows

     [ https://issues.apache.org/jira/browse/BEAM-11143?focusedWorklogId=506895&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-506895 ]

ASF GitHub Bot logged work on BEAM-11143:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 03/Nov/20 13:57
            Start Date: 03/Nov/20 13:57
    Worklog Time Spent: 10m 
      Work Description: kennknowles commented on pull request #13206:
URL: https://github.com/apache/beam/pull/13206#issuecomment-720678222


   IIRC there are some DirectRunner tests around leaking state. It doesn't quite fit with "ValidatesRunner" because `runners-core` is a utility library that is an optional implementation detail, not an externally observable behavior. A runner isn't required to use this form of state at all, or Java, etc. I know of at least one runner that does its triggering outside of Java, but indeed most of them _will_ use this indefinitely.
   
   With regard to runners-core and `ReduceFnRunner`, I agree that all the contexts and namespaces can be quite confusing. The code is quite obscure, especially considering how elementary the actual functionality. Once or twice I've tried to untangle it but failed each time. A major refactor or rewrite might actually be easier than incremental simplification. I'd want to get a clearer test suite that is truly the spec for triggers before doing that. Basically the spirit of `ReduceFnRunnerTest` but expressed as external `TestStream`-based end-to-end behavioral tests instead of heavily mocked out (hence unreliable) unit tests. This won't improve the ability to introspect intermediate state but it will make the code simpler.
   
   Meanwhile, I do think it is reasonable to add accessors to the various internal contexts and have `ReduceFnRunner` assert things about its own behavior. As long as it stays "in bounds" and only asserts things that it really controls and that cannot vary across different uses of `ReduceFnRunner`.


----------------------------------------------------------------
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:
users@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 506895)
    Time Spent: 0.5h  (was: 20m)

> AfterWatermark can leak state for the late trigger when merging windows
> -----------------------------------------------------------------------
>
>                 Key: BEAM-11143
>                 URL: https://issues.apache.org/jira/browse/BEAM-11143
>             Project: Beam
>          Issue Type: Bug
>          Components: runner-core
>            Reporter: Sam Whittle
>            Assignee: Sam Whittle
>            Priority: P2
>             Fix For: 2.26.0
>
>          Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> AfterWatermarkStateMachine attempts to clear the late trigger state when merging and the early trigger is not closed in some windows.  However it appears to clear the state corresponding to the new window, not all of the states for the input windows.  This leaks the state which can lead to unbounded pipeline state buildup.
> https://github.com/apache/beam/blob/68d6c8e6243b1d8f392840273f886276e2a8baff/runners/core-java/src/main/java/org/apache/beam/runners/core/triggers/AfterWatermarkStateMachine.java#L133



--
This message was sent by Atlassian Jira
(v8.3.4#803005)