Uploaded image for project: 'Flink'
  1. Flink
  2. FLINK-5955

Merging a list of buffered records will have problem when ObjectReuse is turned on

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.3.0
    • Component/s: Table API & SQL
    • Labels:
      None

      Description

      Turn on ObjectReuse in MultipleProgramsTestBase:
      TestEnvironment clusterEnv = new TestEnvironment(cluster, 4, true);

      Then the tests "testEventTimeSessionGroupWindow", "testEventTimeSessionGroupWindow", and "testEventTimeTumblingGroupWindowOverTime" will fail.

      The reason is that we have buffered iterated records for group-merge. I think we should change the Agg merge to pair-merge, and later add group-merge when needed (in the future we should add rules to select either pair-merge or group-merge, but for now all built-in aggregates should work fine with pair-merge).

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user shaoxuan-wang opened a pull request:

          https://github.com/apache/flink/pull/3464

          FLINK-5955 [table] Merging a list of buffered records will have problem when ObjectReuse is turned on

          This PR changes the dataSet AGG merge to pair-merge.

          If we buffer the iterated records for group-merge, we will get wrong error when ObjectReuse is turned on. Alternatively, we could deep-copy every record and buffer them for group-merge. But I think that is expense in terms of memory and also CPU. We could later add group-merge when needed (in the future we should add rules to select either pair-merge or group-merge, but for now all built-in aggregates should work fine with pair-merge).

          Thanks for contributing to Apache Flink. Before you open your pull request, please take the following check list into consideration.
          If your changes take all of the items into account, feel free to open your pull request. For more information and/or questions please refer to the [How To Contribute guide](http://flink.apache.org/how-to-contribute.html).
          In addition to going through the list, please provide a meaningful description of your changes.

          • [X] General
          • The pull request references the related JIRA issue ("[FLINK-XXX] Jira title text")
          • The pull request addresses only one issue
          • Each commit in the PR has a meaningful commit message (including the JIRA id)
          • [ ] Documentation
          • Documentation has been added for new functionality
          • Old documentation affected by the pull request has been updated
          • JavaDoc for public methods has been added
          • [X] Tests & Build
          • Functionality added by the pull request is covered by tests
          • `mvn clean verify` has been executed successfully locally or a Travis build has passed

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/shaoxuan-wang/flink F5955-submit

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/flink/pull/3464.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #3464


          commit e6cdab7cd309f16d028894943f177f4321889630
          Author: shaoxuan-wang <wshaoxuan@gmail.com>
          Date: 2017-03-03T05:50:29Z

          FLINK-5955 [table] Merging a list of buffered records will have problem when ObjectReuse is turned on


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user shaoxuan-wang opened a pull request: https://github.com/apache/flink/pull/3464 FLINK-5955 [table] Merging a list of buffered records will have problem when ObjectReuse is turned on This PR changes the dataSet AGG merge to pair-merge. If we buffer the iterated records for group-merge, we will get wrong error when ObjectReuse is turned on. Alternatively, we could deep-copy every record and buffer them for group-merge. But I think that is expense in terms of memory and also CPU. We could later add group-merge when needed (in the future we should add rules to select either pair-merge or group-merge, but for now all built-in aggregates should work fine with pair-merge). Thanks for contributing to Apache Flink. Before you open your pull request, please take the following check list into consideration. If your changes take all of the items into account, feel free to open your pull request. For more information and/or questions please refer to the [How To Contribute guide] ( http://flink.apache.org/how-to-contribute.html ). In addition to going through the list, please provide a meaningful description of your changes. [X] General The pull request references the related JIRA issue (" [FLINK-XXX] Jira title text") The pull request addresses only one issue Each commit in the PR has a meaningful commit message (including the JIRA id) [ ] Documentation Documentation has been added for new functionality Old documentation affected by the pull request has been updated JavaDoc for public methods has been added [X] Tests & Build Functionality added by the pull request is covered by tests `mvn clean verify` has been executed successfully locally or a Travis build has passed You can merge this pull request into a Git repository by running: $ git pull https://github.com/shaoxuan-wang/flink F5955-submit Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/3464.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3464 commit e6cdab7cd309f16d028894943f177f4321889630 Author: shaoxuan-wang <wshaoxuan@gmail.com> Date: 2017-03-03T05:50:29Z FLINK-5955 [table] Merging a list of buffered records will have problem when ObjectReuse is turned on
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user fhueske commented on the issue:

          https://github.com/apache/flink/pull/3464

          Thanks for the fix @shaoxuan-wang! Merging

          Show
          githubbot ASF GitHub Bot added a comment - Github user fhueske commented on the issue: https://github.com/apache/flink/pull/3464 Thanks for the fix @shaoxuan-wang! Merging
          Hide
          fhueske Fabian Hueske added a comment -

          Fixed with 14fab4c412048f769209855d876221817e73ba25

          Show
          fhueske Fabian Hueske added a comment - Fixed with 14fab4c412048f769209855d876221817e73ba25
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user fhueske commented on the issue:

          https://github.com/apache/flink/pull/3464

          Hi @shaoxuan-wang, I forgot to close this PR when merging. Can you close it please?
          Thanks

          Show
          githubbot ASF GitHub Bot added a comment - Github user fhueske commented on the issue: https://github.com/apache/flink/pull/3464 Hi @shaoxuan-wang, I forgot to close this PR when merging. Can you close it please? Thanks
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user shaoxuan-wang closed the pull request at:

          https://github.com/apache/flink/pull/3464

          Show
          githubbot ASF GitHub Bot added a comment - Github user shaoxuan-wang closed the pull request at: https://github.com/apache/flink/pull/3464
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user shaoxuan-wang commented on the issue:

          https://github.com/apache/flink/pull/3464

          Sure @fhueske, this PR is closed

          Show
          githubbot ASF GitHub Bot added a comment - Github user shaoxuan-wang commented on the issue: https://github.com/apache/flink/pull/3464 Sure @fhueske, this PR is closed

            People

            • Assignee:
              ShaoxuanWang Shaoxuan Wang
              Reporter:
              ShaoxuanWang Shaoxuan Wang
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development