Accumulo
  1. Accumulo
  2. ACCUMULO-2128

Provide resource cleanup via static utility rather than Instance.close

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.4.5, 1.5.1, 1.6.0
    • Component/s: client
    • Labels:
      None

      Description

      After discussion about the current state of our code base and the need to properly clean up global resources, consensus is that we need to provide a work around for now that doesn't rely on API changes.

      Later, when we refactor the client api we'll include proper lifecycle management, for now we just need a global utility for unloading.

      This ticket needs to revert all commits from the Instance close work: ACCUMULO-1379, ACCUMULO-1697, ACCUMULO-1858, ACCUMULO-2027, ACCUMULO-1889, and ACCUMULO-2105

      Also ACCUMULO-1923 can be closed as wontfix with a link here.

      Then we need a version of the solution outlined in ACCUMULO-2113 that does not rely on reflection, if possible.

        Issue Links

          Activity

          Hide
          Keith Turner added a comment -

          I created a non-reflection version of the utility. Its in the following branch on github which is based off 1.4.5-SNAPSHOT. Jared Winick I need to add your waitForZooKeeperClientThreads() method in the CleanUp class I created. I would like to get this in place before I proceed with pushing to 1.4 and merging forward. Can you create a patch that adds that method to o.a.a.core.util.CleanUp in the following branch and attach it here? That way the code will be attributed to you and we can use it in Accumulo.

          https://github.com/keith-turner/accumulo/tree/ACCUMULO-2113

          Show
          Keith Turner added a comment - I created a non-reflection version of the utility. Its in the following branch on github which is based off 1.4.5-SNAPSHOT. Jared Winick I need to add your waitForZooKeeperClientThreads() method in the CleanUp class I created. I would like to get this in place before I proceed with pushing to 1.4 and merging forward. Can you create a patch that adds that method to o.a.a.core.util.CleanUp in the following branch and attach it here? That way the code will be attributed to you and we can use it in Accumulo. https://github.com/keith-turner/accumulo/tree/ACCUMULO-2113
          Hide
          Jared Winick added a comment -

          Patch to include waitForZooKeeperClientThreads method. Obviously feel free to modify as needed.

          Show
          Jared Winick added a comment - Patch to include waitForZooKeeperClientThreads method. Obviously feel free to modify as needed.
          Hide
          Sean Busbey added a comment -

          forgot to include ACCUMULO-2010 in the revert list.

          Show
          Sean Busbey added a comment - forgot to include ACCUMULO-2010 in the revert list.
          Hide
          Keith Turner added a comment -

          I have worked out a plan for doing this work. My overall plan is to revert Instance.close() in all branches. After thats done I will add the static utility and merge it forward. I was going to revert in 1.4.5, add the utility in 1.4.5, and then merge all of that forward but I think that could make resolving merge conflicts more complicated. This would make it harder to review conflicts related to the utility.

          The following commits occurred related to Instance.close() (merge related commits not included in list). The commits are in the order applied.

          commits in 1.6.0-SNAPSHOT
          
          379881e
          975e8c0
          674fa95
          0d0bc46 
          ada4180
          79d686f
          3f6c66e
          7da1164
          
          commits in 1.5.1-SNAPSHOT
          
          975e8c0
          0d0bc46
          ada4180
          79d686f
          
          commits in 1.4.5-SNAPSHOT
          
          975e8c0
          0d0bc46
          ada4180
          79d686f
          

          I am thinking of doing the following to revert and then add this utility. I am breaking up things for 1.6.0-SNAPSHOT in the hopes that all commits for 1.6.0 will leave things in a compiling state. Also hoping that breaking up the 1.6.0 revert/merge steps will make it more resilient to concurrent commits by other developers.

          1. revert 379881e 674fa95 in 1.6.0-SNAPSHOT
          2. revert 975e8c0 0d0bc46 ada4180 79d686f in 1.4.5-SNAPSHOT
          3. merge 1.4.5-SNAPSHOT to 1.5.1-SNAPSHOT
          4. merge 1.5.1-SNAPSHOT to 1.6.0-SNAPSHOT
          5. revert 3f6c66e 7da1164 in 1.6.0-SNAPSHOT
          6. add utility to 1.4.5-SNAPSHOT
          7. merge 1.4.5-SNAPSHOT to 1.5.1-SNAPSHOT
          8. merge 1.5.1-SNAPSHOT to 1.6.0-SNAPSHOT
          9. merge 1.6.0-SNAPSHOT to master
          Show
          Keith Turner added a comment - I have worked out a plan for doing this work. My overall plan is to revert Instance.close() in all branches. After thats done I will add the static utility and merge it forward. I was going to revert in 1.4.5, add the utility in 1.4.5, and then merge all of that forward but I think that could make resolving merge conflicts more complicated. This would make it harder to review conflicts related to the utility. The following commits occurred related to Instance.close() (merge related commits not included in list). The commits are in the order applied. commits in 1.6.0-SNAPSHOT 379881e 975e8c0 674fa95 0d0bc46 ada4180 79d686f 3f6c66e 7da1164 commits in 1.5.1-SNAPSHOT 975e8c0 0d0bc46 ada4180 79d686f commits in 1.4.5-SNAPSHOT 975e8c0 0d0bc46 ada4180 79d686f I am thinking of doing the following to revert and then add this utility. I am breaking up things for 1.6.0-SNAPSHOT in the hopes that all commits for 1.6.0 will leave things in a compiling state. Also hoping that breaking up the 1.6.0 revert/merge steps will make it more resilient to concurrent commits by other developers. revert 379881e 674fa95 in 1.6.0-SNAPSHOT revert 975e8c0 0d0bc46 ada4180 79d686f in 1.4.5-SNAPSHOT merge 1.4.5-SNAPSHOT to 1.5.1-SNAPSHOT merge 1.5.1-SNAPSHOT to 1.6.0-SNAPSHOT revert 3f6c66e 7da1164 in 1.6.0-SNAPSHOT add utility to 1.4.5-SNAPSHOT merge 1.4.5-SNAPSHOT to 1.5.1-SNAPSHOT merge 1.5.1-SNAPSHOT to 1.6.0-SNAPSHOT merge 1.6.0-SNAPSHOT to master
          Hide
          Josh Elser added a comment -

          I didn't verify myself that you listed all of the necessary commits that need reverting (I trust you there), but the above plan (with those commits specifically) looks right to me.

          Show
          Josh Elser added a comment - I didn't verify myself that you listed all of the necessary commits that need reverting (I trust you there), but the above plan (with those commits specifically) looks right to me.
          Hide
          Sean Busbey added a comment -

          I was going to say that all three branches should include 335f693 from ACCUMULO-2010, but now I notice that that patch covered more than just things introduced as a part of the Instance.close stuff.

          So now I'm not sure. Better to just handle an unclean revert somewhere else?

          Show
          Sean Busbey added a comment - I was going to say that all three branches should include 335f693 from ACCUMULO-2010 , but now I notice that that patch covered more than just things introduced as a part of the Instance.close stuff. So now I'm not sure. Better to just handle an unclean revert somewhere else?
          Hide
          ASF subversion and git services added a comment -

          Commit 016f3bb10c43f6461c5d41025b0e07b50f1638a2 in branch refs/heads/1.6.0-SNAPSHOT from [~keith_turner]
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=016f3bb ]

          ACCUMULO-2128 Revert "ACCUMULO-1889 found a few more ZooKeeperInstances that are not closed"

          This reverts commit 674fa95cacaa9353142071a66006e0ffb65cae94.

          Conflicts:
          core/src/main/java/org/apache/accumulo/core/client/mapreduce/AbstractInputFormat.java

          Show
          ASF subversion and git services added a comment - Commit 016f3bb10c43f6461c5d41025b0e07b50f1638a2 in branch refs/heads/1.6.0-SNAPSHOT from [~keith_turner] [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=016f3bb ] ACCUMULO-2128 Revert " ACCUMULO-1889 found a few more ZooKeeperInstances that are not closed" This reverts commit 674fa95cacaa9353142071a66006e0ffb65cae94. Conflicts: core/src/main/java/org/apache/accumulo/core/client/mapreduce/AbstractInputFormat.java
          Hide
          Keith Turner added a comment -

          335f693 is a partial revert of some of the changes made by other commits I am trying to revert. I don't want to revert 335f693 because that would bring back stuff we are trying to get rid of. 379881e is the same situation. When I tried to revert 379881e it brought back some calls to instance.close() that were removed. This was the first thing I tried and I threw it out after reviewing the changes. 335f693 is a bigger change, I will review it to see if anything needs to be done.

          Show
          Keith Turner added a comment - 335f693 is a partial revert of some of the changes made by other commits I am trying to revert. I don't want to revert 335f693 because that would bring back stuff we are trying to get rid of. 379881e is the same situation. When I tried to revert 379881e it brought back some calls to instance.close() that were removed. This was the first thing I tried and I threw it out after reviewing the changes. 335f693 is a bigger change, I will review it to see if anything needs to be done.
          Hide
          ASF subversion and git services added a comment -

          Commit 715825b3299fd742d8570971a7f271178b932812 in branch refs/heads/1.4.5-SNAPSHOT from [~keith_turner]
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=715825b ]

          ACCUMULO-2128 added utility to cleanup accumulo static resources

          Signed-off-by: Keith Turner <kturner@apache.org>

          Show
          ASF subversion and git services added a comment - Commit 715825b3299fd742d8570971a7f271178b932812 in branch refs/heads/1.4.5-SNAPSHOT from [~keith_turner] [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=715825b ] ACCUMULO-2128 added utility to cleanup accumulo static resources Signed-off-by: Keith Turner <kturner@apache.org>
          Hide
          ASF subversion and git services added a comment -

          Commit c94a73f478c91a24e35583d41bb39102461c54fa in branch refs/heads/1.4.5-SNAPSHOT from Jared Winick
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=c94a73f ]

          ACCUMULO-2128 added waitForZooKeeperClientThreads method

          Signed-off-by: Keith Turner <kturner@apache.org>

          Show
          ASF subversion and git services added a comment - Commit c94a73f478c91a24e35583d41bb39102461c54fa in branch refs/heads/1.4.5-SNAPSHOT from Jared Winick [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=c94a73f ] ACCUMULO-2128 added waitForZooKeeperClientThreads method Signed-off-by: Keith Turner <kturner@apache.org>
          Hide
          ASF subversion and git services added a comment -

          Commit 8f9fe41751415ab66ddbce6d6dec058999afc1d3 in branch refs/heads/1.4.5-SNAPSHOT from [~keith_turner]
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=8f9fe41 ]

          ACCUMULO-2128 added test for static clean up utility

          Show
          ASF subversion and git services added a comment - Commit 8f9fe41751415ab66ddbce6d6dec058999afc1d3 in branch refs/heads/1.4.5-SNAPSHOT from [~keith_turner] [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=8f9fe41 ] ACCUMULO-2128 added test for static clean up utility
          Hide
          ASF subversion and git services added a comment -

          Commit 715825b3299fd742d8570971a7f271178b932812 in branch refs/heads/1.5.1-SNAPSHOT from [~keith_turner]
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=715825b ]

          ACCUMULO-2128 added utility to cleanup accumulo static resources

          Signed-off-by: Keith Turner <kturner@apache.org>

          Show
          ASF subversion and git services added a comment - Commit 715825b3299fd742d8570971a7f271178b932812 in branch refs/heads/1.5.1-SNAPSHOT from [~keith_turner] [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=715825b ] ACCUMULO-2128 added utility to cleanup accumulo static resources Signed-off-by: Keith Turner <kturner@apache.org>
          Hide
          ASF subversion and git services added a comment -

          Commit c94a73f478c91a24e35583d41bb39102461c54fa in branch refs/heads/1.5.1-SNAPSHOT from Jared Winick
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=c94a73f ]

          ACCUMULO-2128 added waitForZooKeeperClientThreads method

          Signed-off-by: Keith Turner <kturner@apache.org>

          Show
          ASF subversion and git services added a comment - Commit c94a73f478c91a24e35583d41bb39102461c54fa in branch refs/heads/1.5.1-SNAPSHOT from Jared Winick [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=c94a73f ] ACCUMULO-2128 added waitForZooKeeperClientThreads method Signed-off-by: Keith Turner <kturner@apache.org>
          Hide
          ASF subversion and git services added a comment -

          Commit 8f9fe41751415ab66ddbce6d6dec058999afc1d3 in branch refs/heads/1.5.1-SNAPSHOT from [~keith_turner]
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=8f9fe41 ]

          ACCUMULO-2128 added test for static clean up utility

          Show
          ASF subversion and git services added a comment - Commit 8f9fe41751415ab66ddbce6d6dec058999afc1d3 in branch refs/heads/1.5.1-SNAPSHOT from [~keith_turner] [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=8f9fe41 ] ACCUMULO-2128 added test for static clean up utility
          Hide
          ASF subversion and git services added a comment -

          Commit 715825b3299fd742d8570971a7f271178b932812 in branch refs/heads/1.6.0-SNAPSHOT from [~keith_turner]
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=715825b ]

          ACCUMULO-2128 added utility to cleanup accumulo static resources

          Signed-off-by: Keith Turner <kturner@apache.org>

          Show
          ASF subversion and git services added a comment - Commit 715825b3299fd742d8570971a7f271178b932812 in branch refs/heads/1.6.0-SNAPSHOT from [~keith_turner] [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=715825b ] ACCUMULO-2128 added utility to cleanup accumulo static resources Signed-off-by: Keith Turner <kturner@apache.org>
          Hide
          ASF subversion and git services added a comment -

          Commit c94a73f478c91a24e35583d41bb39102461c54fa in branch refs/heads/1.6.0-SNAPSHOT from Jared Winick
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=c94a73f ]

          ACCUMULO-2128 added waitForZooKeeperClientThreads method

          Signed-off-by: Keith Turner <kturner@apache.org>

          Show
          ASF subversion and git services added a comment - Commit c94a73f478c91a24e35583d41bb39102461c54fa in branch refs/heads/1.6.0-SNAPSHOT from Jared Winick [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=c94a73f ] ACCUMULO-2128 added waitForZooKeeperClientThreads method Signed-off-by: Keith Turner <kturner@apache.org>
          Hide
          ASF subversion and git services added a comment -

          Commit 8f9fe41751415ab66ddbce6d6dec058999afc1d3 in branch refs/heads/1.6.0-SNAPSHOT from [~keith_turner]
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=8f9fe41 ]

          ACCUMULO-2128 added test for static clean up utility

          Show
          ASF subversion and git services added a comment - Commit 8f9fe41751415ab66ddbce6d6dec058999afc1d3 in branch refs/heads/1.6.0-SNAPSHOT from [~keith_turner] [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=8f9fe41 ] ACCUMULO-2128 added test for static clean up utility
          Hide
          ASF subversion and git services added a comment -

          Commit c94a73f478c91a24e35583d41bb39102461c54fa in branch refs/heads/master from Jared Winick
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=c94a73f ]

          ACCUMULO-2128 added waitForZooKeeperClientThreads method

          Signed-off-by: Keith Turner <kturner@apache.org>

          Show
          ASF subversion and git services added a comment - Commit c94a73f478c91a24e35583d41bb39102461c54fa in branch refs/heads/master from Jared Winick [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=c94a73f ] ACCUMULO-2128 added waitForZooKeeperClientThreads method Signed-off-by: Keith Turner <kturner@apache.org>
          Hide
          ASF subversion and git services added a comment -

          Commit 8f9fe41751415ab66ddbce6d6dec058999afc1d3 in branch refs/heads/master from [~keith_turner]
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=8f9fe41 ]

          ACCUMULO-2128 added test for static clean up utility

          Show
          ASF subversion and git services added a comment - Commit 8f9fe41751415ab66ddbce6d6dec058999afc1d3 in branch refs/heads/master from [~keith_turner] [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=8f9fe41 ] ACCUMULO-2128 added test for static clean up utility
          Hide
          ASF subversion and git services added a comment -

          Commit 21b1b1108b6094d9706d1a01125d8e36041b484c in branch refs/heads/1.5.1-SNAPSHOT from Christopher Tubbs
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=21b1b11 ]

          ACCUMULO-2128 Remove warnings introduced by merge

          Show
          ASF subversion and git services added a comment - Commit 21b1b1108b6094d9706d1a01125d8e36041b484c in branch refs/heads/1.5.1-SNAPSHOT from Christopher Tubbs [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=21b1b11 ] ACCUMULO-2128 Remove warnings introduced by merge
          Hide
          ASF subversion and git services added a comment -

          Commit 21b1b1108b6094d9706d1a01125d8e36041b484c in branch refs/heads/1.6.0-SNAPSHOT from Christopher Tubbs
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=21b1b11 ]

          ACCUMULO-2128 Remove warnings introduced by merge

          Show
          ASF subversion and git services added a comment - Commit 21b1b1108b6094d9706d1a01125d8e36041b484c in branch refs/heads/1.6.0-SNAPSHOT from Christopher Tubbs [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=21b1b11 ] ACCUMULO-2128 Remove warnings introduced by merge
          Hide
          ASF subversion and git services added a comment -

          Commit 21b1b1108b6094d9706d1a01125d8e36041b484c in branch refs/heads/master from Christopher Tubbs
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=21b1b11 ]

          ACCUMULO-2128 Remove warnings introduced by merge

          Show
          ASF subversion and git services added a comment - Commit 21b1b1108b6094d9706d1a01125d8e36041b484c in branch refs/heads/master from Christopher Tubbs [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=21b1b11 ] ACCUMULO-2128 Remove warnings introduced by merge

            People

            • Assignee:
              Keith Turner
              Reporter:
              Sean Busbey
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development