Uploaded image for project: 'Oozie'
  1. Oozie
  2. OOZIE-2542

Option to disable OpenJPA BrokerImpl finalization

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • None
    • 4.3.0
    • None
    • None

    Description

      We noticed that GC is taking a long time in our production. Looking at the live objects, we can see a lot of FinalizingBrokerImpl.

        10:         33881        8402488  org.apache.openjpa.kernel.FinalizingBrokerImpl
        15:         91943        3677720  java.lang.ref.Finalizer
      1134:             1            384  java.lang.ref.Finalizer$FinalizerThread
      1672:             4            128  org.apache.hadoop.yarn.proto.YarnProtos$FinalApplicationStatusProto
      1926:             4             96  org.apache.hadoop.yarn.api.records.FinalApplicationStatus
      

      By default Broker Finalization is enable to safeguard the accidental resource leaks that may occur if a developer fails to explicitly close EntityManagers when finished. Since we are closing all EntityManagers on final block. We should not have any issue.
      More details @ http://openjpa.apache.org/builds/1.2.3/apache-openjpa/docs/ref_guide_optimization.html

      Attachments

        1. OOZIE-2542-V1.patch
          4 kB
          Purshotam Shah
        2. OOZIE-2542-V2.patch
          4 kB
          Purshotam Shah
        3. OOZIE-2542-V3.patch
          4 kB
          Purshotam Shah

        Activity

          gezapeti Gézapeti added a comment -

          Looks like a good idea.
          The logging part always logs the value "non-finalizing" instead of the actual value of brokerImplConfig, otherwise the patch looks good.

          gezapeti Gézapeti added a comment - Looks like a good idea. The logging part always logs the value "non-finalizing" instead of the actual value of brokerImplConfig , otherwise the patch looks good.
          puru Purshotam Shah added a comment -

          Thanks. I was testing this in on my localhost and generated patch a bit early. OOZIE-2542-V2.patch fixed that.

          puru Purshotam Shah added a comment - Thanks. I was testing this in on my localhost and generated patch a bit early. OOZIE-2542 -V2.patch fixed that.
          hadoopqa Hadoop QA added a comment -

          Testing JIRA OOZIE-2542

          Cleaning local git workspace

          ----------------------------

          +1 PATCH_APPLIES
          +1 CLEAN
          -1 RAW_PATCH_ANALYSIS
          . +1 the patch does not introduce any @author tags
          . +1 the patch does not introduce any tabs
          . +1 the patch does not introduce any trailing spaces
          . -1 the patch contains 2 line(s) longer than 132 characters
          . -1 the patch does not add/modify any testcase
          +1 RAT
          . +1 the patch does not seem to introduce new RAT warnings
          +1 JAVADOC
          . +1 the patch does not seem to introduce new Javadoc warnings
          +1 COMPILE
          . +1 HEAD compiles
          . +1 patch compiles
          . +1 the patch does not seem to introduce new javac warnings
          +1 BACKWARDS_COMPATIBILITY
          . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations
          . +1 the patch does not modify JPA files
          +1 TESTS
          . Tests run: 1782
          +1 DISTRO
          . +1 distro tarball builds with the patch

          ----------------------------
          -1 Overall result, please check the reported -1(s)

          The full output of the test-patch run is available at

          . https://builds.apache.org/job/oozie-trunk-precommit-build/2922/

          hadoopqa Hadoop QA added a comment - Testing JIRA OOZIE-2542 Cleaning local git workspace ---------------------------- +1 PATCH_APPLIES +1 CLEAN -1 RAW_PATCH_ANALYSIS . +1 the patch does not introduce any @author tags . +1 the patch does not introduce any tabs . +1 the patch does not introduce any trailing spaces . -1 the patch contains 2 line(s) longer than 132 characters . -1 the patch does not add/modify any testcase +1 RAT . +1 the patch does not seem to introduce new RAT warnings +1 JAVADOC . +1 the patch does not seem to introduce new Javadoc warnings +1 COMPILE . +1 HEAD compiles . +1 patch compiles . +1 the patch does not seem to introduce new javac warnings +1 BACKWARDS_COMPATIBILITY . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations . +1 the patch does not modify JPA files +1 TESTS . Tests run: 1782 +1 DISTRO . +1 distro tarball builds with the patch ---------------------------- -1 Overall result, please check the reported -1(s) The full output of the test-patch run is available at . https://builds.apache.org/job/oozie-trunk-precommit-build/2922/
          hadoopqa Hadoop QA added a comment -

          Testing JIRA OOZIE-2542

          Cleaning local git workspace

          ----------------------------

          +1 PATCH_APPLIES
          +1 CLEAN
          -1 RAW_PATCH_ANALYSIS
          . +1 the patch does not introduce any @author tags
          . +1 the patch does not introduce any tabs
          . +1 the patch does not introduce any trailing spaces
          . -1 the patch contains 2 line(s) longer than 132 characters
          . -1 the patch does not add/modify any testcase
          +1 RAT
          . +1 the patch does not seem to introduce new RAT warnings
          +1 JAVADOC
          . +1 the patch does not seem to introduce new Javadoc warnings
          +1 COMPILE
          . +1 HEAD compiles
          . +1 patch compiles
          . +1 the patch does not seem to introduce new javac warnings
          +1 BACKWARDS_COMPATIBILITY
          . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations
          . +1 the patch does not modify JPA files
          -1 TESTS
          . Tests run: 1782
          . Tests failed: 2
          . Tests errors: 0

          . The patch failed the following testcases:

          . testIDGeneration(org.apache.oozie.service.TestZKUUIDService)
          . testMultipleIDGeneration(org.apache.oozie.service.TestZKUUIDService)

          +1 DISTRO
          . +1 distro tarball builds with the patch

          ----------------------------
          -1 Overall result, please check the reported -1(s)

          The full output of the test-patch run is available at

          . https://builds.apache.org/job/oozie-trunk-precommit-build/2923/

          hadoopqa Hadoop QA added a comment - Testing JIRA OOZIE-2542 Cleaning local git workspace ---------------------------- +1 PATCH_APPLIES +1 CLEAN -1 RAW_PATCH_ANALYSIS . +1 the patch does not introduce any @author tags . +1 the patch does not introduce any tabs . +1 the patch does not introduce any trailing spaces . -1 the patch contains 2 line(s) longer than 132 characters . -1 the patch does not add/modify any testcase +1 RAT . +1 the patch does not seem to introduce new RAT warnings +1 JAVADOC . +1 the patch does not seem to introduce new Javadoc warnings +1 COMPILE . +1 HEAD compiles . +1 patch compiles . +1 the patch does not seem to introduce new javac warnings +1 BACKWARDS_COMPATIBILITY . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations . +1 the patch does not modify JPA files -1 TESTS . Tests run: 1782 . Tests failed: 2 . Tests errors: 0 . The patch failed the following testcases: . testIDGeneration(org.apache.oozie.service.TestZKUUIDService) . testMultipleIDGeneration(org.apache.oozie.service.TestZKUUIDService) +1 DISTRO . +1 distro tarball builds with the patch ---------------------------- -1 Overall result, please check the reported -1(s) The full output of the test-patch run is available at . https://builds.apache.org/job/oozie-trunk-precommit-build/2923/

          the patch contains 2 line(s) longer than 132 characters

          Please fix this

          Please set the default value to "non-finalizing" in oozie-default.xml and change documentation saying setting empty space will unset the value. I don't see any reason to have empty space as default and asking users to set it for this one if this is a safe change. If there are issues after the change with leaked connections which have been missed, then we can decide to either fix leaked connections and if that is not possible disable this by default in a separate jira.

          rohini Rohini Palaniswamy added a comment - the patch contains 2 line(s) longer than 132 characters Please fix this Please set the default value to "non-finalizing" in oozie-default.xml and change documentation saying setting empty space will unset the value. I don't see any reason to have empty space as default and asking users to set it for this one if this is a safe change. If there are issues after the change with leaked connections which have been missed, then we can decide to either fix leaked connections and if that is not possible disable this by default in a separate jira.
          puru Purshotam Shah added a comment -

          This is an advance setting. Most of the user will not hit GC pause hitting because of JPA. I never heard anybody complaining about this.
          So the safest option to go with the default option and if anybody is facing the GC pause issue, they can override.
          The purpose is to provide a setting to disable to finalization if needed.

          puru Purshotam Shah added a comment - This is an advance setting. Most of the user will not hit GC pause hitting because of JPA. I never heard anybody complaining about this. So the safest option to go with the default option and if anybody is facing the GC pause issue, they can override. The purpose is to provide a setting to disable to finalization if needed.

          If this is something of benefit for code execution and is safe to do and will help with performance in general, we should make it default. Unless this is something risky and can have issues, I don't see any reason to have one other extra setting that users have to specifically tune.

          rohini Rohini Palaniswamy added a comment - If this is something of benefit for code execution and is safe to do and will help with performance in general, we should make it default. Unless this is something risky and can have issues, I don't see any reason to have one other extra setting that users have to specifically tune.
          puru Purshotam Shah added a comment -

          Discuss this internally with Rohini and we discussed to keep default as non-finalizing

          puru Purshotam Shah added a comment - Discuss this internally with Rohini and we discussed to keep default as non-finalizing
          hadoopqa Hadoop QA added a comment -

          Testing JIRA OOZIE-2542

          Cleaning local git workspace

          ----------------------------

          +1 PATCH_APPLIES
          +1 CLEAN
          -1 RAW_PATCH_ANALYSIS
          . +1 the patch does not introduce any @author tags
          . +1 the patch does not introduce any tabs
          . +1 the patch does not introduce any trailing spaces
          . +1 the patch does not introduce any line longer than 132
          . -1 the patch does not add/modify any testcase
          +1 RAT
          . +1 the patch does not seem to introduce new RAT warnings
          +1 JAVADOC
          . +1 the patch does not seem to introduce new Javadoc warnings
          +1 COMPILE
          . +1 HEAD compiles
          . +1 patch compiles
          . +1 the patch does not seem to introduce new javac warnings
          +1 BACKWARDS_COMPATIBILITY
          . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations
          . +1 the patch does not modify JPA files
          +1 TESTS
          . Tests run: 1782
          +1 DISTRO
          . +1 distro tarball builds with the patch

          ----------------------------
          -1 Overall result, please check the reported -1(s)

          The full output of the test-patch run is available at

          . https://builds.apache.org/job/oozie-trunk-precommit-build/2936/

          hadoopqa Hadoop QA added a comment - Testing JIRA OOZIE-2542 Cleaning local git workspace ---------------------------- +1 PATCH_APPLIES +1 CLEAN -1 RAW_PATCH_ANALYSIS . +1 the patch does not introduce any @author tags . +1 the patch does not introduce any tabs . +1 the patch does not introduce any trailing spaces . +1 the patch does not introduce any line longer than 132 . -1 the patch does not add/modify any testcase +1 RAT . +1 the patch does not seem to introduce new RAT warnings +1 JAVADOC . +1 the patch does not seem to introduce new Javadoc warnings +1 COMPILE . +1 HEAD compiles . +1 patch compiles . +1 the patch does not seem to introduce new javac warnings +1 BACKWARDS_COMPATIBILITY . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations . +1 the patch does not modify JPA files +1 TESTS . Tests run: 1782 +1 DISTRO . +1 distro tarball builds with the patch ---------------------------- -1 Overall result, please check the reported -1(s) The full output of the test-patch run is available at . https://builds.apache.org/job/oozie-trunk-precommit-build/2936/
          hadoopqa Hadoop QA added a comment -

          Testing JIRA OOZIE-2542

          Cleaning local git workspace

          ----------------------------

          +1 PATCH_APPLIES
          +1 CLEAN
          -1 RAW_PATCH_ANALYSIS
          . +1 the patch does not introduce any @author tags
          . +1 the patch does not introduce any tabs
          . +1 the patch does not introduce any trailing spaces
          . +1 the patch does not introduce any line longer than 132
          . -1 the patch does not add/modify any testcase
          +1 RAT
          . +1 the patch does not seem to introduce new RAT warnings
          +1 JAVADOC
          . +1 the patch does not seem to introduce new Javadoc warnings
          +1 COMPILE
          . +1 HEAD compiles
          . +1 patch compiles
          . +1 the patch does not seem to introduce new javac warnings
          +1 BACKWARDS_COMPATIBILITY
          . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations
          . +1 the patch does not modify JPA files
          -1 TESTS
          . Tests run: 1782
          . Tests failed: 2
          . Tests errors: 0

          . The patch failed the following testcases:

          . testBundleRerunInPausedWithError(org.apache.oozie.command.bundle.TestBundleRerunXCommand)
          . testCatchupJob(org.apache.oozie.command.coord.TestAbandonedCoordChecker)

          +1 DISTRO
          . +1 distro tarball builds with the patch

          ----------------------------
          -1 Overall result, please check the reported -1(s)

          The full output of the test-patch run is available at

          . https://builds.apache.org/job/oozie-trunk-precommit-build/2935/

          hadoopqa Hadoop QA added a comment - Testing JIRA OOZIE-2542 Cleaning local git workspace ---------------------------- +1 PATCH_APPLIES +1 CLEAN -1 RAW_PATCH_ANALYSIS . +1 the patch does not introduce any @author tags . +1 the patch does not introduce any tabs . +1 the patch does not introduce any trailing spaces . +1 the patch does not introduce any line longer than 132 . -1 the patch does not add/modify any testcase +1 RAT . +1 the patch does not seem to introduce new RAT warnings +1 JAVADOC . +1 the patch does not seem to introduce new Javadoc warnings +1 COMPILE . +1 HEAD compiles . +1 patch compiles . +1 the patch does not seem to introduce new javac warnings +1 BACKWARDS_COMPATIBILITY . +1 the patch does not change any JPA Entity/Colum/Basic/Lob/Transient annotations . +1 the patch does not modify JPA files -1 TESTS . Tests run: 1782 . Tests failed: 2 . Tests errors: 0 . The patch failed the following testcases: . testBundleRerunInPausedWithError(org.apache.oozie.command.bundle.TestBundleRerunXCommand) . testCatchupJob(org.apache.oozie.command.coord.TestAbandonedCoordChecker) +1 DISTRO . +1 distro tarball builds with the patch ---------------------------- -1 Overall result, please check the reported -1(s) The full output of the test-patch run is available at . https://builds.apache.org/job/oozie-trunk-precommit-build/2935/

          +1. Please check if these tests are just flaky and pass locally before checking in. I haven't seen these tests fail before or jiras for them as flaky tests.

          rohini Rohini Palaniswamy added a comment - +1. Please check if these tests are just flaky and pass locally before checking in. I haven't seen these tests fail before or jiras for them as flaky tests.
          puru Purshotam Shah added a comment -

          They are flaky and I have seen them failing earlier also.
          Some of the JIRA where they have failed.
          https://issues.apache.org/jira/browse/OOZIE-2246
          https://issues.apache.org/jira/browse/OOZIE-1964

          puru Purshotam Shah added a comment - They are flaky and I have seen them failing earlier also. Some of the JIRA where they have failed. https://issues.apache.org/jira/browse/OOZIE-2246 https://issues.apache.org/jira/browse/OOZIE-1964
          puru Purshotam Shah added a comment -

          Thanks Rohini and Peter for review. Committed to trunk.

          puru Purshotam Shah added a comment - Thanks Rohini and Peter for review. Committed to trunk.
          rkanter Robert Kanter added a comment -

          Closing issue; Oozie 4.3.0 is released.

          rkanter Robert Kanter added a comment - Closing issue; Oozie 4.3.0 is released.

          People

            puru Purshotam Shah
            puru Purshotam Shah
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: