HBase
  1. HBase
  2. HBASE-4803

Split log worker should terminate properly when waiting for znode

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Implemented
    • Affects Version/s: None
    • Fix Version/s: 0.95.0
    • Component/s: None
    • Labels:
      None

      Description

      This is an attempt to fix the fact that SplitLogWorker threads were not being terminated properly in some multi-master unit tests.

      1. HBASE-4803.patch
        2 kB
        Nicolas Spiegelberg

        Activity

        Hide
        stack added a comment -

        Marking closed.

        Show
        stack added a comment - Marking closed.
        Hide
        stack added a comment -

        This patch is no longer relevant. The code it effects has been changed under it in particular by:

        Author: Lars Hofhansl <larsh@apache.org>  2012-03-22 10:49:32
        Committer: Lars Hofhansl <larsh@apache.org>  2012-03-22 10:49:32
        Parent: cff5945609e95b06d6e832007da5972dac432702 (HBASE-5328 Small changes to Master to make it more testable)
        Child:  0741c69a9ef833c7c3b4aa87c1355558c10df6a1 (HBASE-5542 Unify HRegion.mutateRowsWithLocks() and HRegion.processRow() (Scott Chen) part 2)
        Branches: 6060_santander, 6060_smallpatch.txt, 6299, 6698, checksums, ram, ram2, remotes/origin/instant_schema_alter, remotes/origin/trunk, testing, x
        Follows: 
        Precedes: 
        
            HBASE-5542 Unify HRegion.mutateRowsWithLocks() and HRegion.processRow() (Scott Chen)
            
            git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1303915 13f79535-47bb-0310-9956-ffa450edef68
        
        

        The above commit also seems to catch most of the sentiment behind this patch; testing exitWorker status in loop tests to see if we should continue or not.

        Resolving as Implemented.

        Show
        stack added a comment - This patch is no longer relevant. The code it effects has been changed under it in particular by: Author: Lars Hofhansl <larsh@apache.org> 2012-03-22 10:49:32 Committer: Lars Hofhansl <larsh@apache.org> 2012-03-22 10:49:32 Parent: cff5945609e95b06d6e832007da5972dac432702 (HBASE-5328 Small changes to Master to make it more testable) Child: 0741c69a9ef833c7c3b4aa87c1355558c10df6a1 (HBASE-5542 Unify HRegion.mutateRowsWithLocks() and HRegion.processRow() (Scott Chen) part 2) Branches: 6060_santander, 6060_smallpatch.txt, 6299, 6698, checksums, ram, ram2, remotes/origin/instant_schema_alter, remotes/origin/trunk, testing, x Follows: Precedes: HBASE-5542 Unify HRegion.mutateRowsWithLocks() and HRegion.processRow() (Scott Chen) git-svn-id: https: //svn.apache.org/repos/asf/hbase/trunk@1303915 13f79535-47bb-0310-9956-ffa450edef68 The above commit also seems to catch most of the sentiment behind this patch; testing exitWorker status in loop tests to see if we should continue or not. Resolving as Implemented.
        Hide
        Lars Hofhansl added a comment -

        Pushing to 0.96, but feel free to pull back.

        Show
        Lars Hofhansl added a comment - Pushing to 0.96, but feel free to pull back.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12503961/HBASE-4803.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        -1 javadoc. The javadoc tool appears to have generated -163 warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        -1 findbugs. The patch appears to introduce 51 new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests:
        org.apache.hadoop.hbase.master.TestSplitLogManager

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/270//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/270//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/270//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12503961/HBASE-4803.patch against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 javadoc. The javadoc tool appears to have generated -163 warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 51 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.master.TestSplitLogManager Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/270//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/270//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/270//console This message is automatically generated.
        Hide
        Nicolas Spiegelberg added a comment -

        @Ted. you are correct. Prakash did the original patch, that was a modification I added (both conditions would print to LOG.debug before) and I mixed up the logic. should fix.

        Show
        Nicolas Spiegelberg added a comment - @Ted. you are correct. Prakash did the original patch, that was a modification I added (both conditions would print to LOG.debug before) and I mixed up the logic. should fix.
        Hide
        Ted Yu added a comment -
        +          if (!exitWorker) {
        +            LOG.debug(s);
        +          } else {
        +            LOG.error(s + " (exitWorker is not set)");
        

        For the else block, exitWorker is set. The above seems to reverse the current code where exitWorker is expected to be true.
        Please explain.

        Show
        Ted Yu added a comment - + if (!exitWorker) { + LOG.debug(s); + } else { + LOG.error(s + " (exitWorker is not set)" ); For the else block, exitWorker is set. The above seems to reverse the current code where exitWorker is expected to be true. Please explain.
        Hide
        Nicolas Spiegelberg added a comment -

        Part of 89-fb to 92 port. See r1188420

        Show
        Nicolas Spiegelberg added a comment - Part of 89-fb to 92 port. See r1188420

          People

          • Assignee:
            Prakash Khemani
            Reporter:
            Nicolas Spiegelberg
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development