HBase
  1. HBase
  2. HBASE-5954

Allow proper fsync support for HBase

    Details

    • Type: Improvement Improvement
    • Status: Patch Available
    • Priority: Critical Critical
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: 2.0.0
    • Component/s: HFile, wal
    • Labels:
      None

      Description

      At least get recommendation into 0.96 doc and some numbers running w/ this hdfs feature enabled.

      1. hbase-hdfs-744.txt
        7 kB
        Lars Hofhansl
      2. 5954-WIP-v2-trunk.txt
        19 kB
        Lars Hofhansl
      3. 5954-WIP-trunk.txt
        17 kB
        Lars Hofhansl
      4. 5954-v6-trunk.txt
        30 kB
        Lars Hofhansl
      5. 5954-v6-trunk.txt
        30 kB
        Lars Hofhansl
      6. 5954-v6-trunk.txt
        30 kB
        stack
      7. 5954-v6-trunk.txt
        30 kB
        stack
      8. 5954-v6-trunk.txt
        30 kB
        stack
      9. 5954-v5-trunk.txt
        27 kB
        Lars Hofhansl
      10. 5954-v4-trunk.txt
        19 kB
        Lars Hofhansl
      11. 5954-v3-trunk.txt
        19 kB
        Lars Hofhansl
      12. 5954-v3-trunk.txt
        19 kB
        Lars Hofhansl
      13. 5954-trunk-hdfs-trunk-v6.txt
        18 kB
        Lars Hofhansl
      14. 5954-trunk-hdfs-trunk-v5.txt
        18 kB
        Lars Hofhansl
      15. 5954-trunk-hdfs-trunk-v4.txt
        18 kB
        Lars Hofhansl
      16. 5954-trunk-hdfs-trunk-v3.txt
        17 kB
        Lars Hofhansl
      17. 5954-trunk-hdfs-trunk-v2.txt
        17 kB
        Lars Hofhansl
      18. 5954-trunk-hdfs-trunk.txt
        14 kB
        Lars Hofhansl

        Issue Links

          Activity

          Hide
          Lars Hofhansl added a comment -

          stack and I looked at this patch together and we could not convince ourselves that it is correct (namely that we'll never declared an fsync done when not all entries are fsync'ed).

          What makes that really hard is the disruptor pattern with sync threads working on individual patterns. That is a really nice design for performance. Although it makes it really hard to fit in this change.

          We'll look again and see if we can make it work.

          Show
          Lars Hofhansl added a comment - stack and I looked at this patch together and we could not convince ourselves that it is correct (namely that we'll never declared an fsync done when not all entries are fsync'ed). What makes that really hard is the disruptor pattern with sync threads working on individual patterns. That is a really nice design for performance. Although it makes it really hard to fit in this change. We'll look again and see if we can make it work.
          Hide
          stack added a comment -

          We could. Lets do that if can't make the API as is work.

          Show
          stack added a comment - We could. Lets do that if can't make the API as is work.
          Hide
          Lars Hofhansl added a comment -

          We can dramatically simplify this if we make this a global only switch, with no mixed operation. In that case the API we invented is a bit useless, but so be it. What do you think? It would make this patch a nobrainer.

          I think my comment: https://issues.apache.org/jira/browse/HBASE-5954?focusedCommentId=14296413&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14296413 got lost in the noise.

          Show
          Lars Hofhansl added a comment - We can dramatically simplify this if we make this a global only switch, with no mixed operation. In that case the API we invented is a bit useless, but so be it. What do you think? It would make this patch a nobrainer. I think my comment: https://issues.apache.org/jira/browse/HBASE-5954?focusedCommentId=14296413&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14296413 got lost in the noise.
          Hide
          stack added a comment -

          Oh, BTW, this is a critical improvement.

          Show
          stack added a comment - Oh, BTW, this is a critical improvement.
          Hide
          stack added a comment -

          I think that is a good idea Lars Hofhansl I wouldn't be surprised if you 'got' it but this was tricky stuff keeping one type of state going and now you'd interleave another. I'll come by your shop. Just say when suits.

          Show
          stack added a comment - I think that is a good idea Lars Hofhansl I wouldn't be surprised if you 'got' it but this was tricky stuff keeping one type of state going and now you'd interleave another. I'll come by your shop. Just say when suits.
          Hide
          Lars Hofhansl added a comment -

          I am having a bit of hard time convincing myself that this is a safe change. The tests do not make me feel better about it either.

          stack you seem to feel the same, wanna sit together in front of a screen next week and go over it.

          Show
          Lars Hofhansl added a comment - I am having a bit of hard time convincing myself that this is a safe change. The tests do not make me feel better about it either. stack you seem to feel the same, wanna sit together in front of a screen next week and go over it.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12695775/5954-v6-trunk.txt
          against master branch at commit 86b8b8da0082bea9e8b9c43df917acd67a680cd1.
          ATTACHMENT ID: 12695775

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

          +1 tests included. The patch appears to include 15 new or modified tests.

          -1 javac. The applied patch generated 112 javac compiler warnings (more than the master's current 111 warnings).

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

          -1 checkstyle. The applied patch generated 1938 checkstyle errors (more than the master's current 1936 errors).

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

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

          +1 lineLengths. The patch does not introduce lines longer than 100

          +1 site. The mvn site goal succeeds with this patch.

          -1 core tests. The patch failed these unit tests:

          -1 core zombie tests. There are 1 zombie test(s): at org.apache.hadoop.hbase.TestAcidGuarantees.testScanAtomicity(TestAcidGuarantees.java:354)

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/checkstyle-aggregate.html

          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//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/12695775/5954-v6-trunk.txt against master branch at commit 86b8b8da0082bea9e8b9c43df917acd67a680cd1. ATTACHMENT ID: 12695775 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 15 new or modified tests. -1 javac . The applied patch generated 112 javac compiler warnings (more than the master's current 111 warnings). +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. -1 checkstyle . The applied patch generated 1938 checkstyle errors (more than the master's current 1936 errors). +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: -1 core zombie tests . There are 1 zombie test(s): at org.apache.hadoop.hbase.TestAcidGuarantees.testScanAtomicity(TestAcidGuarantees.java:354) Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/12663//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12695772/5954-v6-trunk.txt
          against master branch at commit 86b8b8da0082bea9e8b9c43df917acd67a680cd1.
          ATTACHMENT ID: 12695772

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

          +1 tests included. The patch appears to include 15 new or modified tests.

          -1 javac. The applied patch generated 112 javac compiler warnings (more than the master's current 111 warnings).

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

          -1 checkstyle. The applied patch generated 1938 checkstyle errors (more than the master's current 1936 errors).

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

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

          +1 lineLengths. The patch does not introduce lines longer than 100

          +1 site. The mvn site goal succeeds with this patch.

          -1 core tests. The patch failed these unit tests:

          -1 core zombie tests. There are 1 zombie test(s): at org.apache.hama.bsp.TestKeyValueTextInputFormat.testInput(TestKeyValueTextInputFormat.java:180)

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/checkstyle-aggregate.html

          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//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/12695772/5954-v6-trunk.txt against master branch at commit 86b8b8da0082bea9e8b9c43df917acd67a680cd1. ATTACHMENT ID: 12695772 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 15 new or modified tests. -1 javac . The applied patch generated 112 javac compiler warnings (more than the master's current 111 warnings). +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. -1 checkstyle . The applied patch generated 1938 checkstyle errors (more than the master's current 1936 errors). +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: -1 core zombie tests . There are 1 zombie test(s): at org.apache.hama.bsp.TestKeyValueTextInputFormat.testInput(TestKeyValueTextInputFormat.java:180) Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/12662//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12695754/5954-v6-trunk.txt
          against master branch at commit 825871431ec48036fd3e3cd9625c451b50cbe308.
          ATTACHMENT ID: 12695754

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

          +1 tests included. The patch appears to include 15 new or modified tests.

          -1 javac. The applied patch generated 112 javac compiler warnings (more than the master's current 111 warnings).

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

          -1 checkstyle. The applied patch generated 1938 checkstyle errors (more than the master's current 1936 errors).

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          -1 release audit. The applied patch generated 1 release audit warnings (more than the master's current 0 warnings).

          +1 lineLengths. The patch does not introduce lines longer than 100

          +1 site. The mvn site goal succeeds with this patch.

          +1 core tests. The patch passed unit tests in .

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//testReport/
          Release audit warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/patchReleaseAuditWarnings.txt
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/checkstyle-aggregate.html

          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//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/12695754/5954-v6-trunk.txt against master branch at commit 825871431ec48036fd3e3cd9625c451b50cbe308. ATTACHMENT ID: 12695754 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 15 new or modified tests. -1 javac . The applied patch generated 112 javac compiler warnings (more than the master's current 111 warnings). +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. -1 checkstyle . The applied patch generated 1938 checkstyle errors (more than the master's current 1936 errors). +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. -1 release audit . The applied patch generated 1 release audit warnings (more than the master's current 0 warnings). +1 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn site goal succeeds with this patch. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/patchReleaseAuditWarnings.txt Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/12661//console This message is automatically generated.
          Hide
          stack added a comment -

          Says this failed...

          TestEndToEndSplitTransaction.blockUntilRegionSplit:460 IO Failed to get daught...

          Show
          stack added a comment - Says this failed... TestEndToEndSplitTransaction.blockUntilRegionSplit:460 IO Failed to get daught...
          Hide
          stack added a comment -

          Retry

          Are the checkstyle and javac items yours?

          Show
          stack added a comment - Retry Are the checkstyle and javac items yours?
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12695726/5954-v6-trunk.txt
          against master branch at commit 825871431ec48036fd3e3cd9625c451b50cbe308.
          ATTACHMENT ID: 12695726

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

          +1 tests included. The patch appears to include 15 new or modified tests.

          -1 javac. The applied patch generated 112 javac compiler warnings (more than the master's current 111 warnings).

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

          -1 checkstyle. The applied patch generated 1938 checkstyle errors (more than the master's current 1936 errors).

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          -1 release audit. The applied patch generated 1 release audit warnings (more than the master's current 0 warnings).

          +1 lineLengths. The patch does not introduce lines longer than 100

          +1 site. The mvn site goal succeeds with this patch.

          -1 core tests. The patch failed these unit tests:

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//testReport/
          Release audit warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/patchReleaseAuditWarnings.txt
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/checkstyle-aggregate.html

          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//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/12695726/5954-v6-trunk.txt against master branch at commit 825871431ec48036fd3e3cd9625c451b50cbe308. ATTACHMENT ID: 12695726 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 15 new or modified tests. -1 javac . The applied patch generated 112 javac compiler warnings (more than the master's current 111 warnings). +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. -1 checkstyle . The applied patch generated 1938 checkstyle errors (more than the master's current 1936 errors). +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. -1 release audit . The applied patch generated 1 release audit warnings (more than the master's current 0 warnings). +1 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/patchReleaseAuditWarnings.txt Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/12659//console This message is automatically generated.
          Hide
          Lars Hofhansl added a comment -

          Passed locally a few time with patch applied. Hung once without patch applied. Same memory profile with and without the patch. Same amount of time spent with and without patch. Unrelated I think. I'll get some more runs in.

          Show
          Lars Hofhansl added a comment - Passed locally a few time with patch applied. Hung once without patch applied. Same memory profile with and without the patch. Same amount of time spent with and without patch. Unrelated I think. I'll get some more runs in.
          Hide
          Lars Hofhansl added a comment -

          This time TestAcidGuarantees did not finish.

          Show
          Lars Hofhansl added a comment - This time TestAcidGuarantees did not finish.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12695691/5954-v6-trunk.txt
          against master branch at commit b08802a3e8e522f84519415b83455870b49bf8da.
          ATTACHMENT ID: 12695691

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

          +1 tests included. The patch appears to include 15 new or modified tests.

          -1 javac. The applied patch generated 112 javac compiler warnings (more than the master's current 111 warnings).

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

          -1 checkstyle. The applied patch generated 1941 checkstyle errors (more than the master's current 1939 errors).

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

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

          +1 lineLengths. The patch does not introduce lines longer than 100

          +1 site. The mvn site goal succeeds with this patch.

          -1 core tests. The patch failed these unit tests:

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/checkstyle-aggregate.html

          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//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/12695691/5954-v6-trunk.txt against master branch at commit b08802a3e8e522f84519415b83455870b49bf8da. ATTACHMENT ID: 12695691 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 15 new or modified tests. -1 javac . The applied patch generated 112 javac compiler warnings (more than the master's current 111 warnings). +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. -1 checkstyle . The applied patch generated 1941 checkstyle errors (more than the master's current 1939 errors). +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 +1 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/12655//console This message is automatically generated.
          Hide
          Lars Hofhansl added a comment -

          v6 has a very basic unittest, verifying that the metrics are updated correctly

          Show
          Lars Hofhansl added a comment - v6 has a very basic unittest, verifying that the metrics are updated correctly
          Hide
          stack added a comment -

          I took a look at that patch. LGTM. This stuff is hard to review. It is tricky. Proof is in the pudding. For sure the failure is unrelated? Its in the WAL? I'm +1 on commit. Suggest try hadoopqa a few more times before commit.

          Show
          stack added a comment - I took a look at that patch. LGTM. This stuff is hard to review. It is tricky. Proof is in the pudding. For sure the failure is unrelated? Its in the WAL? I'm +1 on commit. Suggest try hadoopqa a few more times before commit.
          Hide
          stack added a comment -

          What checkstyle, javadoc, and extra compiler warnings?

          Show
          stack added a comment - What checkstyle, javadoc, and extra compiler warnings?
          Hide
          Lars Hofhansl added a comment -

          TestWALReplay passes locally every time I run it (and takes the same amount of time with or without the patch). So that looks to be unrelated.

          Show
          Lars Hofhansl added a comment - TestWALReplay passes locally every time I run it (and takes the same amount of time with or without the patch). So that looks to be unrelated.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12695466/5954-v5-trunk.txt
          against master branch at commit b08802a3e8e522f84519415b83455870b49bf8da.
          ATTACHMENT ID: 12695466

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

          +1 tests included. The patch appears to include 12 new or modified tests.

          -1 javac. The applied patch generated 112 javac compiler warnings (more than the master's current 111 warnings).

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

          -1 checkstyle. The applied patch generated 1941 checkstyle errors (more than the master's current 1939 errors).

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

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

          -1 lineLengths. The patch introduces the following lines longer than 100:
          + put.setDurability(opts.writeToWAL ? opts.fsyncWAL ? Durability.FSYNC_WAL : Durability.SYNC_WAL : Durability.SKIP_WAL);
          + put.setDurability(opts.writeToWAL ? opts.fsyncWAL ? Durability.FSYNC_WAL : Durability.SYNC_WAL : Durability.SKIP_WAL);
          + public void postSync(final long timeInNanos, final int handlerSyncs, final int handlerFsyncs) {

          +1 site. The mvn site goal succeeds with this patch.

          -1 core tests. The patch failed these unit tests:

          -1 core zombie tests. There are 1 zombie test(s): at org.apache.hadoop.hbase.regionserver.wal.TestWALReplay.testReplayEditsAfterRegionMovedWithMultiCF(TestWALReplay.java:245)

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/checkstyle-aggregate.html

          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//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/12695466/5954-v5-trunk.txt against master branch at commit b08802a3e8e522f84519415b83455870b49bf8da. ATTACHMENT ID: 12695466 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 12 new or modified tests. -1 javac . The applied patch generated 112 javac compiler warnings (more than the master's current 111 warnings). +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. -1 checkstyle . The applied patch generated 1941 checkstyle errors (more than the master's current 1939 errors). +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces the following lines longer than 100: + put.setDurability(opts.writeToWAL ? opts.fsyncWAL ? Durability.FSYNC_WAL : Durability.SYNC_WAL : Durability.SKIP_WAL); + put.setDurability(opts.writeToWAL ? opts.fsyncWAL ? Durability.FSYNC_WAL : Durability.SYNC_WAL : Durability.SKIP_WAL); + public void postSync(final long timeInNanos, final int handlerSyncs, final int handlerFsyncs) { +1 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: -1 core zombie tests . There are 1 zombie test(s): at org.apache.hadoop.hbase.regionserver.wal.TestWALReplay.testReplayEditsAfterRegionMovedWithMultiCF(TestWALReplay.java:245) Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/12647//console This message is automatically generated.
          Hide
          Lars Hofhansl added a comment -

          v5 has the metrics updated. Keep tracks of number of sync and fsync (number of edits that is)

          Tomorrow I'll try to add a unittest based on that.

          Show
          Lars Hofhansl added a comment - v5 has the metrics updated. Keep tracks of number of sync and fsync (number of edits that is) Tomorrow I'll try to add a unittest based on that.
          Hide
          Lars Hofhansl added a comment -

          First part for gathering metrics. Still need to update MetricsWalXXX will do that tomorrow. Parking this WIP patch here.

          Show
          Lars Hofhansl added a comment - First part for gathering metrics. Still need to update MetricsWalXXX will do that tomorrow. Parking this WIP patch here.
          Hide
          Ted Yu added a comment -

          Good work, Lars

          Show
          Ted Yu added a comment - Good work, Lars
          Hide
          Lars Hofhansl added a comment -

          Much better. Thinking about metrics now. At least a count of hflushes and hsyncs and how many edit edit we had for each in total.

          Show
          Lars Hofhansl added a comment - Much better. Thinking about metrics now. At least a count of hflushes and hsyncs and how many edit edit we had for each in total.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12695395/5954-v4-trunk.txt
          against master branch at commit a5613efa7b22a31811cd326e9ad231911afdbde2.
          ATTACHMENT ID: 12695395

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

          +1 tests included. The patch appears to include 6 new or modified tests.

          -1 javac. The applied patch generated 112 javac compiler warnings (more than the master's current 111 warnings).

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

          -1 checkstyle. The applied patch generated 1942 checkstyle errors (more than the master's current 1941 errors).

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

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

          -1 lineLengths. The patch introduces the following lines longer than 100:
          + private int releaseSyncFutures(final long currentSequence, final long currentFSequence, final Throwable t) {
          + put.setDurability(opts.writeToWAL ? opts.fsyncWAL ? Durability.FSYNC_WAL : Durability.SYNC_WAL : Durability.SKIP_WAL);
          + put.setDurability(opts.writeToWAL ? opts.fsyncWAL ? Durability.FSYNC_WAL : Durability.SYNC_WAL : Durability.SKIP_WAL);

          +1 site. The mvn site goal succeeds with this patch.

          +1 core tests. The patch passed unit tests in .

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/checkstyle-aggregate.html

          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//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/12695395/5954-v4-trunk.txt against master branch at commit a5613efa7b22a31811cd326e9ad231911afdbde2. ATTACHMENT ID: 12695395 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 6 new or modified tests. -1 javac . The applied patch generated 112 javac compiler warnings (more than the master's current 111 warnings). +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. -1 checkstyle . The applied patch generated 1942 checkstyle errors (more than the master's current 1941 errors). +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces the following lines longer than 100: + private int releaseSyncFutures(final long currentSequence, final long currentFSequence, final Throwable t) { + put.setDurability(opts.writeToWAL ? opts.fsyncWAL ? Durability.FSYNC_WAL : Durability.SYNC_WAL : Durability.SKIP_WAL); + put.setDurability(opts.writeToWAL ? opts.fsyncWAL ? Durability.FSYNC_WAL : Durability.SYNC_WAL : Durability.SKIP_WAL); +1 site . The mvn site goal succeeds with this patch. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/12639//console This message is automatically generated.
          Hide
          Lars Hofhansl added a comment -

          Found it SyncRunner.releaseSyncFutures would not terminate with the patch.
          The idea is that if we synced to sequence number A and fsynced to sequence number B, it is safe to release all syncFuture before A or B, but only fsyncFutures before B.

          Show
          Lars Hofhansl added a comment - Found it SyncRunner.releaseSyncFutures would not terminate with the patch. The idea is that if we synced to sequence number A and fsynced to sequence number B, it is safe to release all syncFuture before A or B, but only fsyncFutures before B.
          Hide
          Sean Busbey added a comment -

          that's usually a memory problem.

          Show
          Sean Busbey added a comment - that's usually a memory problem.
          Hide
          Lars Hofhansl added a comment -

          Hmm... "Cannot create native thread"? I fail to see how this is related.

          Show
          Lars Hofhansl added a comment - Hmm... "Cannot create native thread"? I fail to see how this is related.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12695313/5954-v3-trunk.txt
          against master branch at commit 6bfa8ea977e991c372c0bef7faa49d6142befebc.
          ATTACHMENT ID: 12695313

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

          +1 tests included. The patch appears to include 6 new or modified tests.

          -1 javac. The applied patch generated 112 javac compiler warnings (more than the master's current 111 warnings).

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

          -1 checkstyle. The applied patch generated 1940 checkstyle errors (more than the master's current 1939 errors).

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

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

          -1 lineLengths. The patch introduces the following lines longer than 100:
          + private int releaseSyncFutures(final long currentSequence, final long currentFSequence, final Throwable t) {
          + put.setDurability(opts.writeToWAL ? opts.fsyncWAL ? Durability.FSYNC_WAL : Durability.SYNC_WAL : Durability.SKIP_WAL);
          + put.setDurability(opts.writeToWAL ? opts.fsyncWAL ? Durability.FSYNC_WAL : Durability.SYNC_WAL : Durability.SKIP_WAL);

          +1 site. The mvn site goal succeeds with this patch.

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.util.TestMergeTable
          org.apache.hadoop.hbase.util.TestMiniClusterLoadSequential
          org.apache.hadoop.hbase.TestZooKeeper
          org.apache.hadoop.hbase.util.TestRegionSplitter
          org.apache.hadoop.hbase.util.TestMiniClusterLoadParallel
          org.apache.hadoop.hbase.util.TestMiniClusterLoadEncoded

          -1 core zombie tests. There are 6 zombie test(s):

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/checkstyle-aggregate.html

          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//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/12695313/5954-v3-trunk.txt against master branch at commit 6bfa8ea977e991c372c0bef7faa49d6142befebc. ATTACHMENT ID: 12695313 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 6 new or modified tests. -1 javac . The applied patch generated 112 javac compiler warnings (more than the master's current 111 warnings). +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. -1 checkstyle . The applied patch generated 1940 checkstyle errors (more than the master's current 1939 errors). +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces the following lines longer than 100: + private int releaseSyncFutures(final long currentSequence, final long currentFSequence, final Throwable t) { + put.setDurability(opts.writeToWAL ? opts.fsyncWAL ? Durability.FSYNC_WAL : Durability.SYNC_WAL : Durability.SKIP_WAL); + put.setDurability(opts.writeToWAL ? opts.fsyncWAL ? Durability.FSYNC_WAL : Durability.SYNC_WAL : Durability.SKIP_WAL); +1 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.util.TestMergeTable org.apache.hadoop.hbase.util.TestMiniClusterLoadSequential org.apache.hadoop.hbase.TestZooKeeper org.apache.hadoop.hbase.util.TestRegionSplitter org.apache.hadoop.hbase.util.TestMiniClusterLoadParallel org.apache.hadoop.hbase.util.TestMiniClusterLoadEncoded -1 core zombie tests . There are 6 zombie test(s): Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/12634//console This message is automatically generated.
          Hide
          Lars Hofhansl added a comment -

          Re-attaching to trigger another run.

          Show
          Lars Hofhansl added a comment - Re-attaching to trigger another run.
          Hide
          Lars Hofhansl added a comment -

          These actually looks environmental. Trying again.

          Show
          Lars Hofhansl added a comment - These actually looks environmental. Trying again.
          Hide
          Lars Hofhansl added a comment -

          Now, that does not look too swell. Looking.

          Show
          Lars Hofhansl added a comment - Now, that does not look too swell. Looking.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12695192/5954-v3-trunk.txt
          against master branch at commit 15a4738470ac02e8354e92998f0899072d8f4716.
          ATTACHMENT ID: 12695192

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

          +1 tests included. The patch appears to include 6 new or modified tests.

          -1 javac. The applied patch generated 112 javac compiler warnings (more than the master's current 111 warnings).

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

          -1 checkstyle. The applied patch generated 1937 checkstyle errors (more than the master's current 1936 errors).

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

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

          -1 lineLengths. The patch introduces the following lines longer than 100:
          + private int releaseSyncFutures(final long currentSequence, final long currentFSequence, final Throwable t) {
          + put.setDurability(opts.writeToWAL ? opts.fsyncWAL ? Durability.FSYNC_WAL : Durability.SYNC_WAL : Durability.SKIP_WAL);
          + put.setDurability(opts.writeToWAL ? opts.fsyncWAL ? Durability.FSYNC_WAL : Durability.SYNC_WAL : Durability.SKIP_WAL);

          +1 site. The mvn site goal succeeds with this patch.

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.regionserver.TestSCVFWithMiniCluster
          org.apache.hadoop.hbase.mapreduce.TestTimeRangeMapRed
          org.apache.hadoop.hbase.replication.TestReplicationDisableInactivePeer
          org.apache.hadoop.hbase.coprocessor.TestRegionObserverBypass
          org.apache.hadoop.hbase.replication.TestReplicationSmallTests
          org.apache.hadoop.hbase.wal.TestDefaultWALProviderWithHLogKey
          org.apache.hadoop.hbase.replication.TestReplicationKillMasterRSCompressed
          org.apache.hadoop.hbase.replication.TestReplicationEndpoint
          org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat
          org.apache.hadoop.hbase.replication.TestReplicationKillMasterRS
          org.apache.hadoop.hbase.mapreduce.TestCellCounter
          org.apache.hadoop.hbase.mapreduce.TestMultiTableInputFormat
          org.apache.hadoop.hbase.mapreduce.TestMultithreadedTableMapper
          org.apache.hadoop.hbase.replication.TestReplicationKillSlaveRS

          -1 core zombie tests. There are 13 zombie test(s): at org.apache.hadoop.hbase.mapreduce.TestImportTsv.testMROnTableWithCustomMapper(TestImportTsv.java:153)
          at org.apache.hadoop.hbase.mapreduce.TestLoadIncrementalHFiles.testRegionCrossingHFileSplit(TestLoadIncrementalHFiles.java:193)
          at org.apache.hadoop.hbase.mapreduce.TestLoadIncrementalHFiles.testRegionCrossingHFileSplitRowColBloom(TestLoadIncrementalHFiles.java:189)
          at org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat2.testMRIncrementalLoad(TestHFileOutputFormat2.java:374)
          at org.apache.hadoop.hbase.mapreduce.TableSnapshotInputFormatTestBase.testWithMapReduce(TableSnapshotInputFormatTestBase.java:116)
          at org.apache.hadoop.hbase.mapreduce.TableSnapshotInputFormatTestBase.testWithMapReduceMultiRegion(TableSnapshotInputFormatTestBase.java:96)
          at org.apache.hadoop.hbase.mapreduce.TestLoadIncrementalHFiles.testTableWithCFNameStartWithUnderScore(TestLoadIncrementalHFiles.java:460)
          at org.apache.hadoop.hbase.mapreduce.TestLoadIncrementalHFiles.testRegionCrossingRowBloom(TestLoadIncrementalHFiles.java:128)

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html
          Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/checkstyle-aggregate.html

          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//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/12695192/5954-v3-trunk.txt against master branch at commit 15a4738470ac02e8354e92998f0899072d8f4716. ATTACHMENT ID: 12695192 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 6 new or modified tests. -1 javac . The applied patch generated 112 javac compiler warnings (more than the master's current 111 warnings). +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. -1 checkstyle . The applied patch generated 1937 checkstyle errors (more than the master's current 1936 errors). +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces the following lines longer than 100: + private int releaseSyncFutures(final long currentSequence, final long currentFSequence, final Throwable t) { + put.setDurability(opts.writeToWAL ? opts.fsyncWAL ? Durability.FSYNC_WAL : Durability.SYNC_WAL : Durability.SKIP_WAL); + put.setDurability(opts.writeToWAL ? opts.fsyncWAL ? Durability.FSYNC_WAL : Durability.SYNC_WAL : Durability.SKIP_WAL); +1 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.regionserver.TestSCVFWithMiniCluster org.apache.hadoop.hbase.mapreduce.TestTimeRangeMapRed org.apache.hadoop.hbase.replication.TestReplicationDisableInactivePeer org.apache.hadoop.hbase.coprocessor.TestRegionObserverBypass org.apache.hadoop.hbase.replication.TestReplicationSmallTests org.apache.hadoop.hbase.wal.TestDefaultWALProviderWithHLogKey org.apache.hadoop.hbase.replication.TestReplicationKillMasterRSCompressed org.apache.hadoop.hbase.replication.TestReplicationEndpoint org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat org.apache.hadoop.hbase.replication.TestReplicationKillMasterRS org.apache.hadoop.hbase.mapreduce.TestCellCounter org.apache.hadoop.hbase.mapreduce.TestMultiTableInputFormat org.apache.hadoop.hbase.mapreduce.TestMultithreadedTableMapper org.apache.hadoop.hbase.replication.TestReplicationKillSlaveRS -1 core zombie tests . There are 13 zombie test(s): at org.apache.hadoop.hbase.mapreduce.TestImportTsv.testMROnTableWithCustomMapper(TestImportTsv.java:153) at org.apache.hadoop.hbase.mapreduce.TestLoadIncrementalHFiles.testRegionCrossingHFileSplit(TestLoadIncrementalHFiles.java:193) at org.apache.hadoop.hbase.mapreduce.TestLoadIncrementalHFiles.testRegionCrossingHFileSplitRowColBloom(TestLoadIncrementalHFiles.java:189) at org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat2.testMRIncrementalLoad(TestHFileOutputFormat2.java:374) at org.apache.hadoop.hbase.mapreduce.TableSnapshotInputFormatTestBase.testWithMapReduce(TableSnapshotInputFormatTestBase.java:116) at org.apache.hadoop.hbase.mapreduce.TableSnapshotInputFormatTestBase.testWithMapReduceMultiRegion(TableSnapshotInputFormatTestBase.java:96) at org.apache.hadoop.hbase.mapreduce.TestLoadIncrementalHFiles.testTableWithCFNameStartWithUnderScore(TestLoadIncrementalHFiles.java:460) at org.apache.hadoop.hbase.mapreduce.TestLoadIncrementalHFiles.testRegionCrossingRowBloom(TestLoadIncrementalHFiles.java:128) Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-rest.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/newPatchFindbugsWarningshbase-annotations.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/12631//console This message is automatically generated.
          Hide
          Lars Hofhansl added a comment - - edited

          Another thought. If we had HDFS storage classes and thus the ability to place the WAL on SSD, would we still want to allow (and force) the application to decide between hflush and hsync? We could simplify and offer two options:

          1. hflush (what we have now)
          2. hsync on SSD

          That would be a global HBase option.
          We might want to think about a group commit option as well: Wait a bit (a few ms, maybe, in the ballpark of a disk rotation or so), and then sync all accumulated edits in one go. Callers would wait until the accumulated batch is done. Almost all relational databases do that. In that case we can allow fsync even with rotating disks and lose less or none of the throughput, at the expense of a slight increase tail latency (those callers who got into the batch first and have to wait until it filled up or reached its time limit).

          Here's the Postgres blurb on that: http://www.postgresql.org/message-id/CAEYLb_V5Q8Zdjnkb4+30_dpD3NrgfoXhEurney3HsrCQsyDLWw@mail.gmail.com

          Just saying. I'm also happy enough with this patch for now, and this patch would allow the application to strategically sync some of the edits even on rotating media.

          Show
          Lars Hofhansl added a comment - - edited Another thought. If we had HDFS storage classes and thus the ability to place the WAL on SSD, would we still want to allow (and force) the application to decide between hflush and hsync? We could simplify and offer two options: hflush (what we have now) hsync on SSD That would be a global HBase option. We might want to think about a group commit option as well: Wait a bit (a few ms, maybe, in the ballpark of a disk rotation or so), and then sync all accumulated edits in one go. Callers would wait until the accumulated batch is done. Almost all relational databases do that. In that case we can allow fsync even with rotating disks and lose less or none of the throughput, at the expense of a slight increase tail latency (those callers who got into the batch first and have to wait until it filled up or reached its time limit). Here's the Postgres blurb on that: http://www.postgresql.org/message-id/CAEYLb_V5Q8Zdjnkb4+30_dpD3NrgfoXhEurney3HsrCQsyDLWw@mail.gmail.com Just saying. I'm also happy enough with this patch for now, and this patch would allow the application to strategically sync some of the edits even on rotating media.
          Hide
          Lars Hofhansl added a comment -

          stack, can I use the traces to get the metrics I want? If so how do I enable that and get the metrics?

          Show
          Lars Hofhansl added a comment - stack , can I use the traces to get the metrics I want? If so how do I enable that and get the metrics?
          Hide
          Lars Hofhansl added a comment -

          Updated patch. Let's get a test run.

          Show
          Lars Hofhansl added a comment - Updated patch. Let's get a test run.
          Hide
          Lars Hofhansl added a comment -

          I'll rebase the patch (if needed), and get a precommit run.
          It'll be hard to verify with a test that this is doing what it should.
          Maybe we should generally add some metrics around WALEdit, how many log entries, how many edits flushed and sync'ed, etc. That would be generally useful, and let us write unittests.

          Show
          Lars Hofhansl added a comment - I'll rebase the patch (if needed), and get a precommit run. It'll be hard to verify with a test that this is doing what it should. Maybe we should generally add some metrics around WALEdit, how many log entries, how many edits flushed and sync'ed, etc. That would be generally useful, and let us write unittests.
          Hide
          stack added a comment -

          fsync every meta edit? I suppose that makes sense. Something we've always wanted even.

          Do we want to keep a syncCount and an fSyncCount?

          It looks like

          currentSequence = updateHighestSyncedSequence(currentSequence);

          highestSyncedSequence is the absolute highest, the highest for sync and fsync, which (I think) I follow.

          The patch looks good. I can't see anything obviously wrong. Lets get it in. As per Sean Busbey, need write up that this long-wanted facility is now available. Would be cool to get numbers but that can be post-commit too.

          Show
          stack added a comment - fsync every meta edit? I suppose that makes sense. Something we've always wanted even. Do we want to keep a syncCount and an fSyncCount? It looks like currentSequence = updateHighestSyncedSequence(currentSequence); highestSyncedSequence is the absolute highest, the highest for sync and fsync, which (I think) I follow. The patch looks good. I can't see anything obviously wrong. Lets get it in. As per Sean Busbey , need write up that this long-wanted facility is now available. Would be cool to get numbers but that can be post-commit too.
          Hide
          Lars Hofhansl added a comment - - edited

          stack since you mention it's more mess. Mind having a look?
          I'll get to some actual testing hopefully this week.

          Show
          Lars Hofhansl added a comment - - edited stack since you mention it's more mess. Mind having a look? I'll get to some actual testing hopefully this week.
          Hide
          Lars Hofhansl added a comment -

          rebased patch... Doesn't compile because of HBASE-5162.
          Just want to park it.

          Show
          Lars Hofhansl added a comment - rebased patch... Doesn't compile because of HBASE-5162 . Just want to park it.
          Hide
          Sean Busbey added a comment -

          Oh certainly. I expect there's going to be a follow on ticket to add docs to the ref guide explaining the basic trade off and I want to make sure the code bits are in place so that whoever has to write that section can focus on tests rather than code gaps.

          Show
          Sean Busbey added a comment - Oh certainly. I expect there's going to be a follow on ticket to add docs to the ref guide explaining the basic trade off and I want to make sure the code bits are in place so that whoever has to write that section can focus on tests rather than code gaps.
          Hide
          Lars Hofhansl added a comment -

          That's a good idea. Will do.
          (In the end... We know there will be a non-trivial impact on rotating disks, we're doing this for to guard against power outages).

          Show
          Lars Hofhansl added a comment - That's a good idea. Will do. (In the end... We know there will be a non-trivial impact on rotating disks, we're doing this for to guard against power outages).
          Hide
          Sean Busbey added a comment -

          Can you update the patch to include changes to WALPerformanceEvaluation to allow a mix of sync / fsync calls so we can see what the perf impact will look like?

          Show
          Sean Busbey added a comment - Can you update the patch to include changes to WALPerformanceEvaluation to allow a mix of sync / fsync calls so we can see what the perf impact will look like?
          Hide
          Lars Hofhansl added a comment -

          After thinking about this for a little bit, I think we need to do this by edit and column family, the way my patch has it.
          Please have a look at this patch, I think we should get this in. Along with this we need to document that you have to run HDFS with sync-on-close and sync-behind-writes. (In fact everybody should do that in any case!)

          Show
          Lars Hofhansl added a comment - After thinking about this for a little bit, I think we need to do this by edit and column family, the way my patch has it. Please have a look at this patch, I think we should get this in. Along with this we need to document that you have to run HDFS with sync-on-close and sync-behind-writes. (In fact everybody should do that in any case!)
          Hide
          Lars Hofhansl added a comment -

          Yeah, we probably need the mixed handling.

          So any comment on the WIP patch?

          Show
          Lars Hofhansl added a comment - Yeah, we probably need the mixed handling. So any comment on the WIP patch?
          Hide
          stack added a comment -

          Thinking on it, blanket fsync, or not, would be a little obnoxious. Fine if all is running ssd but for the majority for a while to come who have less than this, they'd appreciate no-sync, deferred-sync, sync, and fsync on a per-cf or even per-edit basis ("application knows best"). Stuff like fsync on close we should do for sure under the covers.

          Show
          stack added a comment - Thinking on it, blanket fsync, or not, would be a little obnoxious. Fine if all is running ssd but for the majority for a while to come who have less than this, they'd appreciate no-sync, deferred-sync, sync, and fsync on a per-cf or even per-edit basis ("application knows best"). Stuff like fsync on close we should do for sure under the covers.
          Hide
          Lars Hofhansl added a comment -

          Other related setting that would be absolutely required is HDFS's sync on close. Originally I thought we only need to put the WAL into sync on close mode (see HDFS-744), but since any compaction can randomly re-write old data and not sync it to disk unless instructed, it does not make much sense, unless we review every write in HBase and tag the DFS stream with the correct flags.
          (If sync on close is paired with sync behind writes its performance effect is not too bad)

          Show
          Lars Hofhansl added a comment - Other related setting that would be absolutely required is HDFS's sync on close. Originally I thought we only need to put the WAL into sync on close mode (see HDFS-744 ), but since any compaction can randomly re-write old data and not sync it to disk unless instructed, it does not make much sense, unless we review every write in HBase and tag the DFS stream with the correct flags. (If sync on close is paired with sync behind writes its performance effect is not too bad)
          Hide
          Lars Hofhansl added a comment -

          Well... We do have the APIs already. We do not have a "sync always means hsync" flag, that would have been easier, but less flexible.
          We could remove the API again (as FSYNC_WAL is identical to SYNC_WAL right now anyway, see the Durability class)... I.e. deprecating FSYNC_WAL.

          The idea was that the application knows best. It might do ASYNC_WAL most of the time, then issue a SYNC_WAL, and occasionally an FSYNC_WAL when it knows that this is necessary.

          Thinking about it, it would nice to get rid of this complexity (and I would no longer hate this patch either ). This needs to be a careful decision, though. If we want to avoid the mixed handling we could not allow this on a per column family or even table level, it would need be on or off per cluster. In earlier discussions we wanted to avoid this due to the high cost of an fsync (until we have storage classes in HDFS or SSDs everywhere). I'd be in favor of having this discussion again.

          BTW. My patch already has hsync for META always

          Show
          Lars Hofhansl added a comment - Well... We do have the APIs already. We do not have a "sync always means hsync" flag, that would have been easier, but less flexible. We could remove the API again (as FSYNC_WAL is identical to SYNC_WAL right now anyway, see the Durability class)... I.e. deprecating FSYNC_WAL. The idea was that the application knows best. It might do ASYNC_WAL most of the time, then issue a SYNC_WAL, and occasionally an FSYNC_WAL when it knows that this is necessary. Thinking about it, it would nice to get rid of this complexity (and I would no longer hate this patch either ). This needs to be a careful decision, though. If we want to avoid the mixed handling we could not allow this on a per column family or even table level, it would need be on or off per cluster. In earlier discussions we wanted to avoid this due to the high cost of an fsync (until we have storage classes in HDFS or SSDs everywhere). I'd be in favor of having this discussion again. BTW. My patch already has hsync for META always
          Hide
          Sean Busbey added a comment -

          Why support the mix though?

          The last time we checked the relative cost of hflush vs hsync over in Accumulo it was a severe penalty. Once we're in a world of prevalent SSDs and hsync is cheaper, why wouldn't we do it always, atleast for a given cluster?

          I'm just wondering how many different practical use cases we gain by allowing the flexibility of per-request no-wal vs flush vs sync compared to a cluster-wide setting of flush vs sync and then per-request no-wal or whichever is configured.

          Since this would presumably be a configurable property on the wal provider, we could still make meta always hsync.

          Show
          Sean Busbey added a comment - Why support the mix though? The last time we checked the relative cost of hflush vs hsync over in Accumulo it was a severe penalty. Once we're in a world of prevalent SSDs and hsync is cheaper, why wouldn't we do it always, atleast for a given cluster? I'm just wondering how many different practical use cases we gain by allowing the flexibility of per-request no-wal vs flush vs sync compared to a cluster-wide setting of flush vs sync and then per-request no-wal or whichever is configured. Since this would presumably be a configurable property on the wal provider, we could still make meta always hsync.
          Hide
          Lars Hofhansl added a comment -

          Yes, something like that.

          Here's a patch. Not pretty (I think), and entirely untested.
          It's likely that I have missed something as well.
          Didn't pay attention to java doc or variable naming, yet.

          Show
          Lars Hofhansl added a comment - Yes, something like that. Here's a patch. Not pretty (I think), and entirely untested. It's likely that I have missed something as well. Didn't pay attention to java doc or variable naming, yet.
          Hide
          stack added a comment -

          We have to support a mix of sync and fsync? I suppose we do.

          A successful fsync can let go of all handlers with sequenceid < fsync sequenceid; this would work as our sync currently does.

          A successful sync can only let go of all handlers waiting on sync – can't release handlers waiting on an fsync.

          For #1 above, you'd have highest sync sequenceid and highest fsync sequenceid?

          Show
          stack added a comment - We have to support a mix of sync and fsync? I suppose we do. A successful fsync can let go of all handlers with sequenceid < fsync sequenceid; this would work as our sync currently does. A successful sync can only let go of all handlers waiting on sync – can't release handlers waiting on an fsync. For #1 above, you'd have highest sync sequenceid and highest fsync sequenceid?
          Hide
          Lars Hofhansl added a comment -

          I don't mind having a look. Still looking into how it fits best. Seems that the options are:

          1. have each SyncRunner keep two sequences and sequence numbers (sync and fsync) and work off a single queue
          2. have different SyncRunners for sync and fsync and have two sync queues as well

          #1 will be easier but a bit messy
          #2 bit cleaner potentially will make it to share work between threads

          I have a half baked patch for #1.

          As we approach a world with ubiquitous SSDs this will become important.

          Show
          Lars Hofhansl added a comment - I don't mind having a look. Still looking into how it fits best. Seems that the options are: have each SyncRunner keep two sequences and sequence numbers (sync and fsync) and work off a single queue have different SyncRunners for sync and fsync and have two sync queues as well #1 will be easier but a bit messy #2 bit cleaner potentially will make it to share work between threads I have a half baked patch for #1. As we approach a world with ubiquitous SSDs this will become important.
          Hide
          stack added a comment -

          Lars Hofhansl I don't mind having a go at it if you want; its my mess.

          Show
          stack added a comment - Lars Hofhansl I don't mind having a go at it if you want; its my mess.
          Hide
          Lars Hofhansl added a comment -

          Looking at trunk this is a whole lot more complicated now. We need to keep two highest sequence numbers now, one for a normal sync (i.e. hflush) and fsync (i.e. hsync). The SyncFutures have to carry an fsync flag. And if FSLog.SyncRunner.run() need to keep track of two sequence numbers as well. As well as the logic to release other futures in the queue... Looks pretty tricky to merge this in.

          Show
          Lars Hofhansl added a comment - Looking at trunk this is a whole lot more complicated now. We need to keep two highest sequence numbers now, one for a normal sync (i.e. hflush) and fsync (i.e. hsync). The SyncFutures have to carry an fsync flag. And if FSLog.SyncRunner.run() need to keep track of two sequence numbers as well. As well as the logic to release other futures in the queue... Looks pretty tricky to merge this in.
          Hide
          Lars Hofhansl added a comment -

          The alternative is that fold this into the hadoop/hadoop2 compat modules, not sure even that would help with FBs version of Hadoop.

          Show
          Lars Hofhansl added a comment - The alternative is that fold this into the hadoop/hadoop2 compat modules, not sure even that would help with FBs version of Hadoop.
          Hide
          Elliott Clark added a comment -

          So I have to have split personality here. Though for the most part they agree.
          apache hat on Facebook's hdfs is ancient and shouldn't hold the apache hbase project back
          fb hat on Facebook has to keep a small patch on top of anything from Apache HBase. So as long as things are pretty contained we'll be alright.

          Show
          Elliott Clark added a comment - So I have to have split personality here. Though for the most part they agree. apache hat on Facebook's hdfs is ancient and shouldn't hold the apache hbase project back fb hat on Facebook has to keep a small patch on top of anything from Apache HBase. So as long as things are pretty contained we'll be alright.
          Hide
          Lars Hofhansl added a comment -

          Sean Busbey, I'll wait until your changes are in.

          Show
          Lars Hofhansl added a comment - Sean Busbey , I'll wait until your changes are in.
          Hide
          Sean Busbey added a comment -

          Lars around when are you planning to get work on this going? I'm hoping to wrap up HBASE-10378 early next week and it changes some APIs.

          Show
          Sean Busbey added a comment - Lars around when are you planning to get work on this going? I'm hoping to wrap up HBASE-10378 early next week and it changes some APIs.
          Hide
          Enis Soztutar added a comment -

          In HBase trunk (2.x) can I assume we'll only support versions of Hadoop that have HDFS-744 (i.e. 2.0.2 or later). I think that is the case. If that is indeed the case I can do away with all reflection and just use the APIs that allow me to set.

          We have dropped support for hadoop-1, and all hadoop versions before 2.2 is alpha or beta anyway. Only exception is FB's hadoop. cc'ing Elliott Clark.

          Show
          Enis Soztutar added a comment - In HBase trunk (2.x) can I assume we'll only support versions of Hadoop that have HDFS-744 (i.e. 2.0.2 or later). I think that is the case. If that is indeed the case I can do away with all reflection and just use the APIs that allow me to set. We have dropped support for hadoop-1, and all hadoop versions before 2.2 is alpha or beta anyway. Only exception is FB's hadoop. cc'ing Elliott Clark .
          Hide
          Lars Hofhansl added a comment - - edited

          I'd like to pick this up again.
          In HBase trunk (2.x) can I assume we'll only support versions of Hadoop that have HDFS-744 (i.e. 2.0.2 or later). I think that is the case. If that is indeed the case I can do away with all reflection and just use the APIs that allow me to set.

          Note that this puts the performance burden on the client. The client can force

          • sync every edit -> slow
          • batch the edits -> caused only one fsync
          • perform write with flush only followed by an write with forced sync -> that will force all prior edit to disk as well (works as we'll automatically sync a block when closed in this mode)

          I need to check whether sync-behind-writes can be enabled on a per file basis in HDFS. Sync-on-close without sync-behind-writes is extremely slow.

          In fact as discussed above sync-on-close and sync-behind-writes should be enabled by default in all HDFS installations. Since HDFS writes immutable files only there is no use hanging on dirty blocks (in the hopes that they'll be updated again before they get written to disk, and thus sync-behind-writes should not slow things down - it's possible that there're other effects of course).

          Show
          Lars Hofhansl added a comment - - edited I'd like to pick this up again. In HBase trunk (2.x) can I assume we'll only support versions of Hadoop that have HDFS-744 (i.e. 2.0.2 or later). I think that is the case. If that is indeed the case I can do away with all reflection and just use the APIs that allow me to set. Note that this puts the performance burden on the client. The client can force sync every edit -> slow batch the edits -> caused only one fsync perform write with flush only followed by an write with forced sync -> that will force all prior edit to disk as well (works as we'll automatically sync a block when closed in this mode) I need to check whether sync-behind-writes can be enabled on a per file basis in HDFS. Sync-on-close without sync-behind-writes is extremely slow. In fact as discussed above sync-on-close and sync-behind-writes should be enabled by default in all HDFS installations. Since HDFS writes immutable files only there is no use hanging on dirty blocks (in the hopes that they'll be updated again before they get written to disk, and thus sync-behind-writes should not slow things down - it's possible that there're other effects of course).
          Hide
          Andrew Purtell added a comment -

          Unscheduled.

          Show
          Andrew Purtell added a comment - Unscheduled.
          Hide
          Lars Hofhansl added a comment -

          It's probably best to unschedule this for now.

          Show
          Lars Hofhansl added a comment - It's probably best to unschedule this for now.
          Hide
          Andrew Purtell added a comment -

          Where are we with this issue?

          Show
          Andrew Purtell added a comment - Where are we with this issue?
          Hide
          haosdent added a comment -

          Lars Hofhansl Oh, are you sure you disks didn't have RAID card? I test hsync() again and again. The test result shows hsync() is a very heavy operation.

          Show
          haosdent added a comment - Lars Hofhansl Oh, are you sure you disks didn't have RAID card? I test hsync() again and again. The test result shows hsync() is a very heavy operation.
          Hide
          Lars Hofhansl added a comment -

          Nope. Software RAID.

          Show
          Lars Hofhansl added a comment - Nope. Software RAID.
          Hide
          haosdent added a comment -

          Lars HofhanslHaha, I have test hsync() in RAID10 before. A hsync() call would spent 4ms. Because the data are written to RAID card cache, it is very fast.

          Show
          haosdent added a comment - Lars Hofhansl Haha, I have test hsync() in RAID10 before. A hsync() call would spent 4ms. Because the data are written to RAID card cache, it is very fast.
          Hide
          Lars Hofhansl added a comment -

          Not sure on which machine I ran this now. I can redo. On my work machine I have 4 disks in RAID10.

          Show
          Lars Hofhansl added a comment - Not sure on which machine I ran this now. I can redo. On my work machine I have 4 disks in RAID10.
          Hide
          haosdent added a comment -

          >my tests above were run with write barriers enabled and data=ordered.
          Lars HofhanslIt seems very interesting. Did you use RAID?

          Show
          haosdent added a comment - >my tests above were run with write barriers enabled and data=ordered. Lars Hofhansl It seems very interesting. Did you use RAID?
          Hide
          Lars Hofhansl added a comment -

          Also, my tests above were run with write barriers enabled and data=ordered. Did you run PE, or a different test?

          Show
          Lars Hofhansl added a comment - Also, my tests above were run with write barriers enabled and data=ordered. Did you run PE, or a different test?
          Hide
          Lars Hofhansl added a comment -

          haosdent From the article you linked: "With barriers enabled, an fsync() call will also issue a storage cache flush.", which is exactly what I said.

          Show
          Lars Hofhansl added a comment - haosdent From the article you linked: "With barriers enabled, an fsync() call will also issue a storage cache flush.", which is exactly what I said.
          Hide
          Luke Lu added a comment -

          As you've found out, the fsync performance is very sensitive to the disk controller and its settings. You need a SATA/SAS controller (RAID or not) with battery or (recently) flash (FBWC in HP cards, nvcache in Dell cards) backed write cache to get good and safe sync performance on HDFS as the additional seek to sync the checksum file is really expensive without a write cache to queue the writes, which calls for HDFS-2699, which I think should be an per file option.

          Show
          Luke Lu added a comment - As you've found out, the fsync performance is very sensitive to the disk controller and its settings. You need a SATA/SAS controller (RAID or not) with battery or (recently) flash (FBWC in HP cards, nvcache in Dell cards) backed write cache to get good and safe sync performance on HDFS as the additional seek to sync the checksum file is really expensive without a write cache to queue the writes, which calls for HDFS-2699 , which I think should be an per file option.
          Hide
          haosdent added a comment -

          When we use the hsync of HDFS, the JVM in datanode will call fsync or fdatasync to ensure the dirty data of file is flush to stable storage. When I do the test as same as Lars Hofhansl, I have this test result:
          Without WAL/HFile sync: ~13s
          With WAL sync, Without HFile sync: ~120s (Sorry, I make some input mistakes before.)

          I think you may unclear about the differences between "data=writeback" and "data=order, barrier=1". These posts may be help you understand them.
          1.https://access.redhat.com/site/documentation/en-US/Red_Hat_Enterprise_Linux/6/html/Storage_Administration_Guide/writebarr.html
          2.http://lwn.net/Articles/457667/

          Show
          haosdent added a comment - When we use the hsync of HDFS, the JVM in datanode will call fsync or fdatasync to ensure the dirty data of file is flush to stable storage. When I do the test as same as Lars Hofhansl , I have this test result: Without WAL/HFile sync: ~13s With WAL sync, Without HFile sync: ~120s (Sorry, I make some input mistakes before.) I think you may unclear about the differences between "data=writeback" and "data=order, barrier=1". These posts may be help you understand them. 1. https://access.redhat.com/site/documentation/en-US/Red_Hat_Enterprise_Linux/6/html/Storage_Administration_Guide/writebarr.html 2. http://lwn.net/Articles/457667/
          Hide
          Lars Hofhansl added a comment -

          @haosdent, we can't break the laws of physics.
          If you sync every single edit you'll see terrible performance, how can we expect otherwise?
          HBase (even without fsync) wants things in batches, in PE HTable is doing it's default batching (2m batches), so that's where the cost is amortized.

          Enabling sync behind writes should improve this too (since we're writing immutable data), since by the time we issue the sync some data will already be sync'ed.

          Lastly, fsync is fsync (or rather fdatasync and friends since we're sync'ing files and not filesystems)... Once executed, previously cached data is on disk no matter what the filesystem chooses to cache during normal operations; only barriers are needed for correctness (AFAIK).

          Show
          Lars Hofhansl added a comment - @haosdent, we can't break the laws of physics. If you sync every single edit you'll see terrible performance, how can we expect otherwise? HBase (even without fsync) wants things in batches, in PE HTable is doing it's default batching (2m batches), so that's where the cost is amortized. Enabling sync behind writes should improve this too (since we're writing immutable data), since by the time we issue the sync some data will already be sync'ed. Lastly, fsync is fsync (or rather fdatasync and friends since we're sync'ing files and not filesystems)... Once executed, previously cached data is on disk no matter what the filesystem chooses to cache during normal operations; only barriers are needed for correctness (AFAIK).
          Hide
          haosdent added a comment -

          Liang XieBecause there is only have a journal file on every disk in extN, the system will commit all files metadata transactions when we open write barrier. After flush all files metadata transactions to the journal file in physics disk, the system will flush data(both metadata and block) of the special file to disk. So the fsync would spent more time when we have a lot of IO in a disk. My weibo is http://weibo.com/haosdent , welcome for more discussion.

          Show
          haosdent added a comment - Liang Xie Because there is only have a journal file on every disk in extN, the system will commit all files metadata transactions when we open write barrier. After flush all files metadata transactions to the journal file in physics disk, the system will flush data(both metadata and block) of the special file to disk. So the fsync would spent more time when we have a lot of IO in a disk. My weibo is http://weibo.com/haosdent , welcome for more discussion.
          Hide
          haosdent added a comment -

          Liang XieIf mount disk with "data=writeback", the dirty data may be in disk cache after fsync system call return. Until the data more than a ratio in disk cache or timer is trigger, them will flush to physics storage. We could improve the performance of hsync by disable journal and write cache. But after disable write cache, the whole write performance is worse than before. Fsync is a very heavy system call, I think it is unfeasible to call fsync after every write operation. Just post my test result about fsync roughly below:

          1.ext4,noatime,barrier=1,data=ordered, enable disk write cache, enable journal, append 4k to a file
          fdatasync 25ms
          fsync 25ms
          2.ext4,noatime,barrier=0,data=writeback, disable disk write cache, enable journal, append 4k to a file
          fdatasync 33ms
          fsync 33ms
          3.ext4,noatime,barrier=0,data=writeback, disable disk write cache, disable journal, append 4k to a file
          fdatasync 8ms
          fsync 8ms

          Show
          haosdent added a comment - Liang Xie If mount disk with "data=writeback", the dirty data may be in disk cache after fsync system call return. Until the data more than a ratio in disk cache or timer is trigger, them will flush to physics storage. We could improve the performance of hsync by disable journal and write cache. But after disable write cache, the whole write performance is worse than before. Fsync is a very heavy system call, I think it is unfeasible to call fsync after every write operation. Just post my test result about fsync roughly below: 1.ext4,noatime,barrier=1,data=ordered, enable disk write cache, enable journal, append 4k to a file fdatasync 25ms fsync 25ms 2.ext4,noatime,barrier=0,data=writeback, disable disk write cache, enable journal, append 4k to a file fdatasync 33ms fsync 33ms 3.ext4,noatime,barrier=0,data=writeback, disable disk write cache, disable journal, append 4k to a file fdatasync 8ms fsync 8ms
          Hide
          Liang Xie added a comment -

          haosdent, IMHO, fsync + write barrier combination should has guarantee the data be written to disk (with issuing a disk cache flush instruction). is it relatived with "data=ordered" mount option? thanks

          Show
          Liang Xie added a comment - haosdent , IMHO, fsync + write barrier combination should has guarantee the data be written to disk (with issuing a disk cache flush instruction). is it relatived with "data=ordered" mount option? thanks
          Hide
          haosdent added a comment -

          Only when we open write barrier and mount disk with "data=ordered", we could make sure that the data have been flush to physics disk after we call fsync system call.

          Show
          haosdent added a comment - Only when we open write barrier and mount disk with "data=ordered", we could make sure that the data have been flush to physics disk after we call fsync system call.
          Hide
          haosdent added a comment -

          Lars Hofhansl My test result:
          Without WAL/HFile sync: ~13s
          With WAL/HFile sync: ~120s

          Anything wrong?

          Show
          haosdent added a comment - Lars Hofhansl My test result: Without WAL/HFile sync: ~13s With WAL/HFile sync: ~120s Anything wrong?
          Hide
          haosdent added a comment -

          hi, Lars Hofhansl, whether your disks have raid or not? I have tested the hsync of hdfs again and again. I found it will spent nearly 50ms while hflush just spent 2ms on non-raid disks.

          Show
          haosdent added a comment - hi, Lars Hofhansl , whether your disks have raid or not? I have tested the hsync of hdfs again and again. I found it will spent nearly 50ms while hflush just spent 2ms on non-raid disks.
          Hide
          stack added a comment -

          This can go in any time. Doesn't have to be plugged against 0.96.0

          Show
          stack added a comment - This can go in any time. Doesn't have to be plugged against 0.96.0
          Hide
          Lars Hofhansl added a comment -

          HBASE-7801 and HBASE-8375 put the right client APIs in place, and in 0.95+ we have the Hadoop1/Hadoop2 modules to avoid reflection hell.

          This is then just a matter of:

          1. Instantiate a Writer with sync-on-close enabled
          2. pass the fsync through to our log syncer and issue the sync when requested

          I think we would not allow pairing fsync with asynchronous sync'ing (the current API would not support it anyway).

          I'll see if I can find some time next week. Although my guess is that most folks want global sync-on-close and global sync-behind-writes on the HDFS cluster backing HBase.

          Show
          Lars Hofhansl added a comment - HBASE-7801 and HBASE-8375 put the right client APIs in place, and in 0.95+ we have the Hadoop1/Hadoop2 modules to avoid reflection hell. This is then just a matter of: Instantiate a Writer with sync-on-close enabled pass the fsync through to our log syncer and issue the sync when requested I think we would not allow pairing fsync with asynchronous sync'ing (the current API would not support it anyway). I'll see if I can find some time next week. Although my guess is that most folks want global sync-on-close and global sync-behind-writes on the HDFS cluster backing HBase.
          Hide
          stack added a comment -

          What is to be done to finish this up for 0.95? I've marked it critical so gets attention.

          Show
          stack added a comment - What is to be done to finish this up for 0.95? I've marked it critical so gets attention.
          Hide
          Dave Latham added a comment -

          Should update the recommended HDFS configuration in the book then? I think losing a region of data after a compaction and power failure should be prevented by default.

          Show
          Dave Latham added a comment - Should update the recommended HDFS configuration in the book then? I think losing a region of data after a compaction and power failure should be prevented by default.
          Hide
          Lars Hofhansl added a comment -

          And yes HDFS-1539 as well as HDFS-2465 for the fadvice hints.

          Show
          Lars Hofhansl added a comment - And yes HDFS-1539 as well as HDFS-2465 for the fadvice hints.
          Hide
          Lars Hofhansl added a comment -

          This is a HDFS server side setting, though, so we cannot enforce that via an HBase config.
          From my tests I found that this must be paired with the sync behind writes hint, otherwise file creation is quite slow (50 or more ms per file in a real cluster).

          And obviously this does not help with last edits in the WAL, as they are not in a closed block.

          Show
          Lars Hofhansl added a comment - This is a HDFS server side setting, though, so we cannot enforce that via an HBase config. From my tests I found that this must be paired with the sync behind writes hint, otherwise file creation is quite slow (50 or more ms per file in a real cluster). And obviously this does not help with last edits in the WAL, as they are not in a closed block.
          Hide
          Dave Latham added a comment -

          You probably just want to enable sync-on-close in HDFS.

          Referring to HDFS-1539?

          +1

          Show
          Dave Latham added a comment - You probably just want to enable sync-on-close in HDFS. Referring to HDFS-1539 ? +1
          Hide
          Suresh Srinivas added a comment -

          Should we make this default?

          +1 from me as well.

          Show
          Suresh Srinivas added a comment - Should we make this default? +1 from me as well.
          Hide
          stack added a comment -

          Should we make this default?

          I'd vote for that.

          Show
          stack added a comment - Should we make this default? I'd vote for that.
          Hide
          Enis Soztutar added a comment -

          Would be great to see this go in to 0.96.

          Agreed. Initially not performant, this will enable us to work on the performance issues as well.

          You probably just want to enable sync-on-close in HDFS.

          Should we make this default?

          Show
          Enis Soztutar added a comment - Would be great to see this go in to 0.96. Agreed. Initially not performant, this will enable us to work on the performance issues as well. You probably just want to enable sync-on-close in HDFS. Should we make this default?
          Hide
          Lars Hofhansl added a comment -

          Note that for performance reasons you probably do not want this (sync edits in the WAL). You probably just want to enable sync-on-close in HDFS.

          Show
          Lars Hofhansl added a comment - Note that for performance reasons you probably do not want this (sync edits in the WAL). You probably just want to enable sync-on-close in HDFS.
          Hide
          Lars Hofhansl added a comment -

          With HBASE-7801 and HBASE-8375 we're almost there.
          The only part missing is to actually hook in the fsync part.
          Did not do it yet, because of reflection hell. In 0.96 we have separate modules for hadoop versions, so we can avoid reflection there.

          Show
          Lars Hofhansl added a comment - With HBASE-7801 and HBASE-8375 we're almost there. The only part missing is to actually hook in the fsync part. Did not do it yet, because of reflection hell. In 0.96 we have separate modules for hadoop versions, so we can avoid reflection there.
          Hide
          Dave Latham added a comment -

          Would be great to see this go in to 0.96. Lars or anyone still chewing on it?
          (Recently suffered a datacenter wide power loss and lost some hfiles from regions that completed a major compaction seconds beforehand).

          Show
          Dave Latham added a comment - Would be great to see this go in to 0.96. Lars or anyone still chewing on it? (Recently suffered a datacenter wide power loss and lost some hfiles from regions that completed a major compaction seconds beforehand).
          Hide
          Varun Sharma added a comment -

          Firstly, I certainly don't want to complicate this too much - this is a really nice functionality to have and we can worry about the details later.

          So, in a lot of systems, there is variability of amount of edits per second - lets say high in the night but low during the day, so the you lose more data during the night than day if this is only time bound. Some systems like mySQL have a bound on the number of edits before syncing - that saidm I am happy with what we have, in its current form. What I suggest is only a nice to have...

          THanks

          Show
          Varun Sharma added a comment - Firstly, I certainly don't want to complicate this too much - this is a really nice functionality to have and we can worry about the details later. So, in a lot of systems, there is variability of amount of edits per second - lets say high in the night but low during the day, so the you lose more data during the night than day if this is only time bound. Some systems like mySQL have a bound on the number of edits before syncing - that saidm I am happy with what we have, in its current form. What I suggest is only a nice to have... THanks
          Hide
          Lars Hofhansl added a comment -

          We can make this arbitrarily complicated.

          A flush or deferred log flush gets us pretty far. It'll flush the data to the data nodes, which will then asynchronously (but ASAP) flush it to the OS buffers. In Linux the dirty page cache is periodically flushed to disk (that can be configured - default is 30s). I am not sure what else you want?

          sync'ing only really makes sense (IMHO) when it is done synchronously.

          Show
          Lars Hofhansl added a comment - We can make this arbitrarily complicated. A flush or deferred log flush gets us pretty far. It'll flush the data to the data nodes, which will then asynchronously (but ASAP) flush it to the OS buffers. In Linux the dirty page cache is periodically flushed to disk (that can be configured - default is 30s). I am not sure what else you want? sync'ing only really makes sense (IMHO) when it is done synchronously.
          Hide
          Varun Sharma added a comment -

          No I am not talking about deferred WAL flush. This is what I know but i maybe wrong:
          1) HBase uses hflush for WAL which ensures that data is in OS buffers and leaves the data in the hands of the OS - after that the time from OS cache -> disk persistence is variable
          2) With sync, we will synchronize the WAL to disk so there is no data loss

          I am asking about the possibility of intermittent sync(s) performed by the region server every N edits - so N edits where we do hflush and then we do hsync or every N seconds. Because, going from hflush -> hsync for WAL will kill performance. If we can have gaurantees that say last 1 or 0.5 second worth of data is intact and similarly, you can lose 1000 edits in case of power failure - that is a nice to have.

          Show
          Varun Sharma added a comment - No I am not talking about deferred WAL flush. This is what I know but i maybe wrong: 1) HBase uses hflush for WAL which ensures that data is in OS buffers and leaves the data in the hands of the OS - after that the time from OS cache -> disk persistence is variable 2) With sync, we will synchronize the WAL to disk so there is no data loss I am asking about the possibility of intermittent sync(s) performed by the region server every N edits - so N edits where we do hflush and then we do hsync or every N seconds. Because, going from hflush -> hsync for WAL will kill performance. If we can have gaurantees that say last 1 or 0.5 second worth of data is intact and similarly, you can lose 1000 edits in case of power failure - that is a nice to have.
          Hide
          Harsh J added a comment -

          A middle ground feature like syncing every N seconds (like there is in REDIS) or every N edits (like there is in MySQL) would be a nice to have.

          Are you looking for deferred WAL flush (per-CF property)?

          Show
          Harsh J added a comment - A middle ground feature like syncing every N seconds (like there is in REDIS) or every N edits (like there is in MySQL) would be a nice to have. Are you looking for deferred WAL flush (per-CF property)?
          Hide
          Varun Sharma added a comment -

          Btw, are we going to provide a hard option of whether to do either a "sync" or a "flush" per transaction ? A middle ground feature like syncing every N seconds (like there is in REDIS) or every N edits (like there is in MySQL) would be a nice to have. This might also be doable on the client side by forcing the N-th RPC to be a sync operation but would be nice on the server side.

          Show
          Varun Sharma added a comment - Btw, are we going to provide a hard option of whether to do either a "sync" or a "flush" per transaction ? A middle ground feature like syncing every N seconds (like there is in REDIS) or every N edits (like there is in MySQL) would be a nice to have. This might also be doable on the client side by forcing the N-th RPC to be a sync operation but would be nice on the server side.
          Hide
          Lars Hofhansl added a comment -

          Filed HBASE-7215.

          Show
          Lars Hofhansl added a comment - Filed HBASE-7215 .
          Hide
          Andrew Purtell added a comment -

          I'd vote for removing them. If we keep these we have failed with wire compatibility and all the protofuf stuff was for nothing.

          +1

          Show
          Andrew Purtell added a comment - I'd vote for removing them. If we keep these we have failed with wire compatibility and all the protofuf stuff was for nothing. +1
          Hide
          Lars Hofhansl added a comment -

          I'd vote for removing them. If we keep these we have failed with wire compatibility and all the protofuf stuff was for nothing.

          Put/Delete is still used as writable at least in these cases:

          • IdentityTableReduce.java
          • MultiPut.java
          • HRegionServer.checkAndMutate
          Show
          Lars Hofhansl added a comment - I'd vote for removing them. If we keep these we have failed with wire compatibility and all the protofuf stuff was for nothing. Put/Delete is still used as writable at least in these cases: IdentityTableReduce.java MultiPut.java HRegionServer.checkAndMutate
          Hide
          Jimmy Xiang added a comment -

          Those methods should be removed. But old applications may not compile any more if someone happens to use them. Are we ok with that?

          Show
          Jimmy Xiang added a comment - Those methods should be removed. But old applications may not compile any more if someone happens to use them. Are we ok with that?
          Hide
          Lars Hofhansl added a comment - - edited

          In 0.96 Put.java, Delete.java, and Increment.java still have readFields() and write() methods from writable.
          Were they left over by accident? I assume those can be removed now?

          Show
          Lars Hofhansl added a comment - - edited In 0.96 Put.java, Delete.java, and Increment.java still have readFields() and write() methods from writable. Were they left over by accident? I assume those can be removed now?
          Hide
          Liang Xie added a comment -

          sound good for me. hope more guys have an eye on it

          Show
          Liang Xie added a comment - sound good for me. hope more guys have an eye on it
          Hide
          Lars Hofhansl added a comment - - edited

          While I'm at it, I'd like to add deferred log flush as a per operation option as well.
          So we'd have:

          1. No WAL update (for the existing option to disable writing to the WAL)
          2. deferred log flush
          3. flush WAL (default)
          4. sync WAL

          If there are multiple mutations in a batch the highest option will be used for the entire batch.

          Since these options cannot be combined in a sensible way. This is best represented by an enum.
          In 0.96, we can break wire compatibility. I'll just add a protobuf enum, and remove the current writeToWal bit. The actual logic will be put in the hadoop-2 shim module.

          In 0.94 this is a bit more tricky. Both in terms of doing this in a wire compatible way and in terms of being forced to use reflection to detect and use Hadoop-2 vs Hadoop-1. Leaning towards only doing this in 0.96, even though I really wanted this in 0.94.

          Comments?

          Show
          Lars Hofhansl added a comment - - edited While I'm at it, I'd like to add deferred log flush as a per operation option as well. So we'd have: No WAL update (for the existing option to disable writing to the WAL) deferred log flush flush WAL (default) sync WAL If there are multiple mutations in a batch the highest option will be used for the entire batch. Since these options cannot be combined in a sensible way. This is best represented by an enum. In 0.96, we can break wire compatibility. I'll just add a protobuf enum, and remove the current writeToWal bit. The actual logic will be put in the hadoop-2 shim module. In 0.94 this is a bit more tricky. Both in terms of doing this in a wire compatible way and in terms of being forced to use reflection to detect and use Hadoop-2 vs Hadoop-1. Leaning towards only doing this in 0.96, even though I really wanted this in 0.94. Comments?
          Hide
          Lars Hofhansl added a comment -

          I'm planning to pick this up again, soon.

          Show
          Lars Hofhansl added a comment - I'm planning to pick this up again, soon.
          Hide
          Liang Xie added a comment -

          Hi Lars Hofhansl,the HDFS-3979 has been committed, so maybe we can have a more clear target/fix version plan on HBASE-5954 now, right?

          Show
          Liang Xie added a comment - Hi Lars Hofhansl ,the HDFS-3979 has been committed, so maybe we can have a more clear target/fix version plan on HBASE-5954 now, right?
          Hide
          Lars Hofhansl added a comment -

          Unscheduling from both 0.94 and 0.96 until HDFS-3979 is committed.

          Show
          Lars Hofhansl added a comment - Unscheduling from both 0.94 and 0.96 until HDFS-3979 is committed.
          Hide
          Lars Hofhansl added a comment -

          I see, you're right, the expectations are different. Let's fix HDFS-3979 already

          Show
          Lars Hofhansl added a comment - I see, you're right, the expectations are different. Let's fix HDFS-3979 already
          Hide
          Kan Zhang added a comment -

          Hi Lars, I think you did misunderstand what I said, esp. on the part "... with respect to the semantics they support." As you know, post HDFS-265 hflush implements API3 and when hflush returns data is guaranteed to be in the DN buffers on all replica nodes, which is what API3 promises and client can't expect anything more than that by calling hflush. But client would expect data to hit disk by calling hsync, which is not guaranteed to happen in the current implementation. The expected semantics are simply different for hflush and hsync.

          Show
          Kan Zhang added a comment - Hi Lars, I think you did misunderstand what I said, esp. on the part "... with respect to the semantics they support." As you know, post HDFS-265 hflush implements API3 and when hflush returns data is guaranteed to be in the DN buffers on all replica nodes, which is what API3 promises and client can't expect anything more than that by calling hflush. But client would expect data to hit disk by calling hsync, which is not guaranteed to happen in the current implementation. The expected semantics are simply different for hflush and hsync.
          Hide
          Lars Hofhansl added a comment -

          Wait, in all cases the data was flushed to the DN. hsync and hflush give exactly the same guarantees here (it's the same code path).
          If hsync is broken here, so it hflush.

          What can currently happen (past HDFS-265) for both hflush and hsync is that the data is still in the DN buffers (not in the OS buffers - in case of hflush, or on disk - in case of hsync)... Unless I seriously misunderstand.

          Show
          Lars Hofhansl added a comment - Wait, in all cases the data was flushed to the DN. hsync and hflush give exactly the same guarantees here (it's the same code path). If hsync is broken here, so it hflush. What can currently happen (past HDFS-265 ) for both hflush and hsync is that the data is still in the DN buffers (not in the OS buffers - in case of hflush, or on disk - in case of hsync)... Unless I seriously misunderstand.
          Hide
          Kan Zhang added a comment -

          It's true hsync is better than hflush in terms of persisting to disk. However, IMO, what's important for apps is whether it is safe to discard data from their buffers when acknowledged (without having to worry about retrying the writes in case of cluster failures). The current hsync doesn't give you that assurance, while both pre and after HDFS-265 hflush implementations do with respect to the semantics they support.

          Show
          Kan Zhang added a comment - It's true hsync is better than hflush in terms of persisting to disk. However, IMO, what's important for apps is whether it is safe to discard data from their buffers when acknowledged (without having to worry about retrying the writes in case of cluster failures). The current hsync doesn't give you that assurance, while both pre and after HDFS-265 hflush implementations do with respect to the semantics they support.
          Hide
          Lars Hofhansl added a comment -

          I disagree, having the signal out for the DNs to sync durably to disk now (even if they only do as soon as they get to it) is better than having an indefinite amount in which the data on only in volatile memory (which is as good as we can get with only hflush).

          But let's not harp this point... We all agree that it needs to be fixed.

          I could use some help testing HDFS-3979. I believe the pipeline tests cover the failure scenarios (which are the same whether we flush/sync or not), the test needed there is verifying that the fsync is on the synchronous path of the client.

          Show
          Lars Hofhansl added a comment - I disagree, having the signal out for the DNs to sync durably to disk now (even if they only do as soon as they get to it) is better than having an indefinite amount in which the data on only in volatile memory (which is as good as we can get with only hflush). But let's not harp this point... We all agree that it needs to be fixed. I could use some help testing HDFS-3979 . I believe the pipeline tests cover the failure scenarios (which are the same whether we flush/sync or not), the test needed there is verifying that the fsync is on the synchronous path of the client.
          Hide
          Luke Lu added a comment -

          hsync is still better (even when it is not on the synchronous path, so there's a little gap where a client was told that everything is on disk when in fact it isn't).

          No. It (hsync) is worse in the sense that there will be data loss (i.e., inconsistency: acked writes missing) if people bounce hdfs, while hbase is still writing. Bouncing hdfs happens much more often in practice than actual machine/pdu failures. To put it more strongly, hsync in the current hadoop 2 is semantically wrong.

          Show
          Luke Lu added a comment - hsync is still better (even when it is not on the synchronous path, so there's a little gap where a client was told that everything is on disk when in fact it isn't). No. It (hsync) is worse in the sense that there will be data loss (i.e., inconsistency: acked writes missing) if people bounce hdfs, while hbase is still writing. Bouncing hdfs happens much more often in practice than actual machine/pdu failures. To put it more strongly, hsync in the current hadoop 2 is semantically wrong.
          Hide
          Lars Hofhansl added a comment -

          Hi Luke,

          you mean hflush in hadoop 2 less durable than hflush in hadoop 1. hsync is still better (even when it is not on the synchronous path, so there's a little gap where a client was told that everything is on disk when in fact it isn't).

          Filed HDFS-3979 (you know that , just for the benefit of others reading here), which needs some testing to be committed.

          Since the hsync code is only in hadoop 2.1.0+ we'd need a new shim here for that version (or reflect the sh*t out of it).

          I'm still happy to commit this to 0.94.x.

          Show
          Lars Hofhansl added a comment - Hi Luke, you mean hflush in hadoop 2 less durable than hflush in hadoop 1. hsync is still better (even when it is not on the synchronous path, so there's a little gap where a client was told that everything is on disk when in fact it isn't). Filed HDFS-3979 (you know that , just for the benefit of others reading here), which needs some testing to be committed. Since the hsync code is only in hadoop 2.1.0+ we'd need a new shim here for that version (or reflect the sh*t out of it). I'm still happy to commit this to 0.94.x.
          Hide
          Luke Lu added a comment -

          Hi Lars,

          We just noticed that HDFS-744 did not implement the correct hsync semantics (mostly due to HDFS-265) so that the hsync is slower AND (arguably) less durable than hflush in Hadoop 1.x.

          Show
          Luke Lu added a comment - Hi Lars, We just noticed that HDFS-744 did not implement the correct hsync semantics (mostly due to HDFS-265 ) so that the hsync is slower AND (arguably) less durable than hflush in Hadoop 1.x.
          Hide
          Lars Hofhansl added a comment -

          Sigh... The WAL files would still need to be sync'ed upon blockclose. Since they mix data from different stores, there's no telling ahead of time. Which leads to:

          1. global hsync option upon close block for HFiles and Hlogs (makes no sense to sync HLogs but not HFiles or vice versa)
          2. global WAL edit hsync option
          3. hsync CF's WAL edits
          4. WAL hsync per Put/Delete/Append/Increment/etc
          Show
          Lars Hofhansl added a comment - Sigh... The WAL files would still need to be sync'ed upon blockclose. Since they mix data from different stores, there's no telling ahead of time. Which leads to: global hsync option upon close block for HFiles and Hlogs (makes no sense to sync HLogs but not HFiles or vice versa) global WAL edit hsync option hsync CF's WAL edits WAL hsync per Put/Delete/Append/Increment/etc
          Hide
          Lars Hofhansl added a comment -

          Created HBASE-6492.
          Since I am interested in having this in 0.94 I'll start with the reflection based approach (but still in trunk for HadoopQA).

          This is what I am going to do:

          1. global HFiles hsync option upon close block (this will also apply sync'ing the WAL on close)
          2. global WAL edit hsync opion
          3. hsync CF's HFiles
          4. hsync CF's WAL edits
          5. WAL hsync per Put/Delete/Append/Increment/etc
          Show
          Lars Hofhansl added a comment - Created HBASE-6492 . Since I am interested in having this in 0.94 I'll start with the reflection based approach (but still in trunk for HadoopQA). This is what I am going to do: global HFiles hsync option upon close block (this will also apply sync'ing the WAL on close) global WAL edit hsync opion hsync CF's HFiles hsync CF's WAL edits WAL hsync per Put/Delete/Append/Increment/etc
          Hide
          Andrew Purtell added a comment -

          More on my comment above. We have two options: we can start breaking out reflection into modules in 0.94 too, or save all of that for 0.96. I don't have a strong opinion but if I had to make a choice the refactoring should be in next major version / trunk / currently unstable.

          Show
          Andrew Purtell added a comment - More on my comment above. We have two options: we can start breaking out reflection into modules in 0.94 too, or save all of that for 0.96. I don't have a strong opinion but if I had to make a choice the refactoring should be in next major version / trunk / currently unstable.
          Hide
          Andrew Purtell added a comment -

          a flag per Put/Delete/Increment/Append/etc, would be best option

          Makes sense, since both you and Todd got here after giving it consideration.

          On the other hand I do not want to have completely diverging implementations for this for 0.94 and 0.96

          I think we have to take this pain, a reflection based strategy in 0.94 and a module based strategy in 0.96. I this case I'd judge it worth it. But that's going to be a high bar for other things.

          Show
          Andrew Purtell added a comment - a flag per Put/Delete/Increment/Append/etc, would be best option Makes sense, since both you and Todd got here after giving it consideration. On the other hand I do not want to have completely diverging implementations for this for 0.94 and 0.96 I think we have to take this pain, a reflection based strategy in 0.94 and a module based strategy in 0.96. I this case I'd judge it worth it. But that's going to be a high bar for other things.
          Hide
          Lars Hofhansl added a comment -

          Another question: HBase 0.96 now has the hadoop

          {1|2}

          -compat projects.
          On the other hand I do not want to have completely diverging implementations for this for 0.94 and 0.96 (which would mean to use reflection in both branches). Any thoughts on that?

          Show
          Lars Hofhansl added a comment - Another question: HBase 0.96 now has the hadoop {1|2} -compat projects. On the other hand I do not want to have completely diverging implementations for this for 0.94 and 0.96 (which would mean to use reflection in both branches). Any thoughts on that?
          Hide
          Lars Hofhansl added a comment -

          Thinking more about HTable#hsync, I think that would hard to make useful to an application. The application would need to do know which RegionServers to hsync the WAL on (unless we want to flush for a Table, which means all RegionServer hosting regions for this table need to hsync their WAL, and that does not seem to be useful).

          A similar argument goes for the RPCs (which will be split to multiple RegionServers).
          So #6 and #7 are out I think.

          I think Todd was right after all (just took me a long time to come around to it), a flag per Put/Delete/Increment/Append/etc, would be best option.

          Show
          Lars Hofhansl added a comment - Thinking more about HTable#hsync, I think that would hard to make useful to an application. The application would need to do know which RegionServers to hsync the WAL on (unless we want to flush for a Table, which means all RegionServer hosting regions for this table need to hsync their WAL, and that does not seem to be useful). A similar argument goes for the RPCs (which will be split to multiple RegionServers). So #6 and #7 are out I think. I think Todd was right after all (just took me a long time to come around to it), a flag per Put/Delete/Increment/Append/etc, would be best option.
          Hide
          stack added a comment -

          Would be nice to be able to enable this (when using Hadoop-2).

          Can we do it in the hadoop2 compatibility module?

          I think its fine its optionally off in 0.94 and that in 0.96 its on by default.

          I like your list. Would think a CF/Table option and the HTable#hsync the more important options to offer (though on the latter, perhaps a Put+hsync would be better given extra rpc).

          Show
          stack added a comment - Would be nice to be able to enable this (when using Hadoop-2). Can we do it in the hadoop2 compatibility module? I think its fine its optionally off in 0.94 and that in 0.96 its on by default. I like your list. Would think a CF/Table option and the HTable#hsync the more important options to offer (though on the latter, perhaps a Put+hsync would be better given extra rpc).
          Hide
          Lars Hofhansl added a comment - - edited

          I think the API going multiple ways (these are not mutually exclusive):

          1. hsync for HFiles (would guard compactions, etc, very lightweight), enabled with a config option (default on I think)
          2. hsync all WAL edits (very expensive, but would not require client changes), enabled with a config option (default off)
          3. hsync for tables or column families for HFiles (configured in the table/column descriptor)
          4. hsync for tables or column families for the WAL (configured in the table/column descriptor)
          5. WAL hsync per Put. Gives control to the application. A batch put would hsync the WAL if at least one Put in the batch was market with hsync. What about deletes? In 0.94 they are not batched; could it at the end of operation there.
          6. WAL hsync per RPC. Could send flag with the RPC from the client. I.e. HTable would have a Put(List<Put> puts, boolean hsync) method
          7. HTable.hsync. Client calls this when WAL must be sync'ed. Most flexible, but incurs an extra RPC to the RegionServer just to force the hsync.

          Comments welcome.

          Edit: Forgot some options.

          Show
          Lars Hofhansl added a comment - - edited I think the API going multiple ways (these are not mutually exclusive): hsync for HFiles (would guard compactions, etc, very lightweight), enabled with a config option (default on I think) hsync all WAL edits (very expensive, but would not require client changes), enabled with a config option (default off) hsync for tables or column families for HFiles (configured in the table/column descriptor) hsync for tables or column families for the WAL (configured in the table/column descriptor) WAL hsync per Put. Gives control to the application. A batch put would hsync the WAL if at least one Put in the batch was market with hsync. What about deletes? In 0.94 they are not batched; could it at the end of operation there. WAL hsync per RPC. Could send flag with the RPC from the client. I.e. HTable would have a Put(List<Put> puts, boolean hsync) method HTable.hsync. Client calls this when WAL must be sync'ed. Most flexible, but incurs an extra RPC to the RegionServer just to force the hsync. Comments welcome. Edit: Forgot some options.
          Hide
          Lars Hofhansl added a comment -

          Would be nice to be able to enable this (when using Hadoop-2).
          Not sure about the 0.94 target, but since the change is not disruptive to HBase and purely optional it would be a good addition.

          Show
          Lars Hofhansl added a comment - Would be nice to be able to enable this (when using Hadoop-2). Not sure about the 0.94 target, but since the change is not disruptive to HBase and purely optional it would be a good addition.
          Hide
          Lars Hofhansl added a comment -

          I almost feel like the HFile sync should be enabled by default. This ensures that after an HFile is written and closed all of its blocks are sync'ed to disk. The overhead of this is "fairly" minimal.
          The edit-by-edit sync for the WAL is definitely optional.

          Show
          Lars Hofhansl added a comment - I almost feel like the HFile sync should be enabled by default. This ensures that after an HFile is written and closed all of its blocks are sync'ed to disk. The overhead of this is "fairly" minimal. The edit-by-edit sync for the WAL is definitely optional.
          Hide
          Lars Hofhansl added a comment -

          HDFS-744 is in Hadoop-2.0 now.
          I'd like to make a clean patch with only the cluster wide option first. That way, this can be enabled and allow HBase to still be a nice HDFS citizen.

          Then we can add other options (per CF, per Put, etc).

          Show
          Lars Hofhansl added a comment - HDFS-744 is in Hadoop-2.0 now. I'd like to make a clean patch with only the cluster wide option first. That way, this can be enabled and allow HBase to still be a nice HDFS citizen. Then we can add other options (per CF, per Put, etc).
          Hide
          Todd Lipcon added a comment -

          Even per-put would be pretty nice, actually – I can imagine some applications where most updates are "unimportant" but the occasional one should be hard-persisted.

          Show
          Todd Lipcon added a comment - Even per-put would be pretty nice, actually – I can imagine some applications where most updates are "unimportant" but the occasional one should be hard-persisted.
          Hide
          Lars Hofhansl added a comment -

          HDFS-744 was committed. Time to finalize this patch.
          Todd suggested at the hack-a-thon to have a sync option per column family rather than per HBase cluster.

          Show
          Lars Hofhansl added a comment - HDFS-744 was committed. Time to finalize this patch. Todd suggested at the hack-a-thon to have a sync option per column family rather than per HBase cluster.
          Hide
          Lars Hofhansl added a comment -

          Previous patch accidentally always enable WAL sync if HDFS-744 is present.

          Show
          Lars Hofhansl added a comment - Previous patch accidentally always enable WAL sync if HDFS-744 is present.
          Hide
          Lars Hofhansl added a comment -

          Minor change, matching latest HDFS-744 patch.

          Show
          Lars Hofhansl added a comment - Minor change, matching latest HDFS-744 patch.
          Hide
          Lars Hofhansl added a comment -

          Patch matching latest patch on HDFS-744. Will probably change again soon.

          Show
          Lars Hofhansl added a comment - Patch matching latest patch on HDFS-744 . Will probably change again soon.
          Hide
          Lars Hofhansl added a comment -

          I'm using ext4 with default mount options (indeed the numbers would be useless otherwise)

          Show
          Lars Hofhansl added a comment - I'm using ext4 with default mount options (indeed the numbers would be useless otherwise)
          Hide
          Luke Lu added a comment -

          Thanks for the numbers, Lars! Are you using ext3? I wonder what the numbers would look like if you enable barrier=1 in the mount options or just use ext4 (with barrier turned on by default). If the underlying fs doesn't do barrier, the result is somewhat meaningless (you might as well use hflush).

          Show
          Luke Lu added a comment - Thanks for the numbers, Lars! Are you using ext3? I wonder what the numbers would look like if you enable barrier=1 in the mount options or just use ext4 (with barrier turned on by default). If the underlying fs doesn't do barrier, the result is somewhat meaningless (you might as well use hflush).
          Hide
          Lars Hofhansl added a comment -

          Patch that works correctly for WAL and HFile sync. (This is the one I used for above performance tests)

          Show
          Lars Hofhansl added a comment - Patch that works correctly for WAL and HFile sync. (This is the one I used for above performance tests)
          Hide
          Lars Hofhansl added a comment -

          Ok... Now with HFile sync.
          With HFile sync: ~20s
          Both WAL and HFile sync: ~35s

          With HFile sync enabled, I seek occasional IO spikes on top of the constant WAL IO.

          Show
          Lars Hofhansl added a comment - Ok... Now with HFile sync. With HFile sync: ~20s Both WAL and HFile sync: ~35s With HFile sync enabled, I seek occasional IO spikes on top of the constant WAL IO.
          Hide
          Lars Hofhansl added a comment -

          Finally some numbers for "PerformanceEvaluation --nomapred --rows=100000 --presplit=3 randomWrite 10"
          Without WAL/HFile sync: ~18s
          With WAL sync: ~34s

          With WAL sync on, I see a constant ~70mb/s write load. Without WAL sync I see a few spikes of far higher IO load.

          This is all on a single machine with HBase in fully distributed mode on top of a pseudo distributed HDFS.

          Note that my patch does not yet do HFile sync'ing correctly.

          Show
          Lars Hofhansl added a comment - Finally some numbers for "PerformanceEvaluation --nomapred --rows=100000 --presplit=3 randomWrite 10" Without WAL/HFile sync: ~18s With WAL sync: ~34s With WAL sync on, I see a constant ~70mb/s write load. Without WAL sync I see a few spikes of far higher IO load. This is all on a single machine with HBase in fully distributed mode on top of a pseudo distributed HDFS. Note that my patch does not yet do HFile sync'ing correctly.
          Hide
          Lars Hofhansl added a comment -

          This is only partially true. It is true for the sync packet stuff that I added, because this does not require closing the block.

          If the block needs to be closed (which causes it to be fsync'ed) it is done after the ack from the downstream DN and before the ack to the upstream DN. Here in that case the fsyncs are serial. Looking at the code, that part seems hard to change.

          Good news is: HLog files are smaller than a DFS block, so for HBase we never run into the 2nd issue.
          Semi bad news: HFiles also need to fsync'ed at least on block close, so here we'd see the issue. But since HFiles are written asynchronously it should be OK.

          Show
          Lars Hofhansl added a comment - This is only partially true. It is true for the sync packet stuff that I added, because this does not require closing the block. If the block needs to be closed (which causes it to be fsync'ed) it is done after the ack from the downstream DN and before the ack to the upstream DN. Here in that case the fsyncs are serial. Looking at the code, that part seems hard to change. Good news is: HLog files are smaller than a DFS block, so for HBase we never run into the 2nd issue. Semi bad news: HFiles also need to fsync'ed at least on block close, so here we'd see the issue. But since HFiles are written asynchronously it should be OK.
          Hide
          Luke Lu added a comment -

          Actually, I don't think the result should be too bad (within an order of magnitude), as fsync on replicas should happen in parallel (upstream DN should forward the sync packet before doing its own fsync and wait for the ack from downstream DN). I do expect that the increased sync latency would expose more bugs (especially lock contention and races) both in HDFS and HBase.

          Show
          Luke Lu added a comment - Actually, I don't think the result should be too bad (within an order of magnitude), as fsync on replicas should happen in parallel (upstream DN should forward the sync packet before doing its own fsync and wait for the ack from downstream DN). I do expect that the increased sync latency would expose more bugs (especially lock contention and races) both in HDFS and HBase.
          Hide
          Lars Hofhansl added a comment - - edited

          It's hard to get even a single run of PE run to completion (even without durable sync enabled). I'm getting various exceptions. Sometimes: "could only replicate to 0 datanodes", sometime name node problems: "LeaseExpiredException" on an HLog file.

          At this point I do not think these are due to my HDFS changes, this is all in code that I did not touch.

          Show
          Lars Hofhansl added a comment - - edited It's hard to get even a single run of PE run to completion (even without durable sync enabled). I'm getting various exceptions. Sometimes: "could only replicate to 0 datanodes", sometime name node problems: "LeaseExpiredException" on an HLog file. At this point I do not think these are due to my HDFS changes, this is all in code that I did not touch.
          Hide
          Lars Hofhansl added a comment -

          Will do. I assume the result will be devastating
          With the HDFS replica chaining we'd eat the fsync time N times (for N replicas).

          Show
          Lars Hofhansl added a comment - Will do. I assume the result will be devastating With the HDFS replica chaining we'd eat the fsync time N times (for N replicas).
          Hide
          Luke Lu added a comment -

          Hi Lars, could you post some preliminary benchmark numbers for hsync vs hflush? Thanks.

          Show
          Luke Lu added a comment - Hi Lars, could you post some preliminary benchmark numbers for hsync vs hflush? Thanks.
          Hide
          Lars Hofhansl added a comment -

          Patch that makes durable sync configurable (separately for WAL and HFiles).

          Should probably allow all file creation to be durable (losing tableinfo or regioninfo files would be bad too).

          Show
          Lars Hofhansl added a comment - Patch that makes durable sync configurable (separately for WAL and HFiles). Should probably allow all file creation to be durable (losing tableinfo or regioninfo files would be bad too).
          Hide
          Lars Hofhansl added a comment -

          Btw. the last attached patch also includes a changed pom.xml file. -Dhadoop.profile=30 can be used to build HBase against hadoop 3.0.0 snapshots.

          In order to build hadoop-trunk and hbase-trunk just do:

          1. mvn -Pnative -Pdist -Dtar -DskipTests clean install
          2. mvn clean install -Dhadoop.profile=30 -DskipTests
          Show
          Lars Hofhansl added a comment - Btw. the last attached patch also includes a changed pom.xml file. -Dhadoop.profile=30 can be used to build HBase against hadoop 3.0.0 snapshots. In order to build hadoop-trunk and hbase-trunk just do: mvn -Pnative -Pdist -Dtar -DskipTests clean install mvn clean install -Dhadoop.profile=30 -DskipTests
          Hide
          Lars Hofhansl added a comment -

          Here's a patch against HBase-trunk matching the v2-trunk patch in HDFS-744.

          Again, this is not yet configurable and just for testing.

          Show
          Lars Hofhansl added a comment - Here's a patch against HBase-trunk matching the v2-trunk patch in HDFS-744 . Again, this is not yet configurable and just for testing.
          Hide
          Lars Hofhansl added a comment -

          Here's what I have. Not configurable.
          With this and HDFS-744, it should be possible to test this out.

          I will be doing that.

          Show
          Lars Hofhansl added a comment - Here's what I have. Not configurable. With this and HDFS-744 , it should be possible to test this out. I will be doing that.
          Hide
          Lars Hofhansl added a comment - - edited

          Over in HDFS-744 I propose changes to HDFS to allow fsync on files.
          Without that both WAL and HFiles (resulting from compactions/flushes) are not guaranteed to be on disk at the DFS replicas.

          The HDFS-744 is becoming big, so I'm not sure how the hadoop folks will receive it. I any case, this issues tracks the necessary changes on the HBase side.

          Show
          Lars Hofhansl added a comment - - edited Over in HDFS-744 I propose changes to HDFS to allow fsync on files. Without that both WAL and HFiles (resulting from compactions/flushes) are not guaranteed to be on disk at the DFS replicas. The HDFS-744 is becoming big, so I'm not sure how the hadoop folks will receive it. I any case, this issues tracks the necessary changes on the HBase side.

            People

            • Assignee:
              Lars Hofhansl
              Reporter:
              Lars Hofhansl
            • Votes:
              2 Vote for this issue
              Watchers:
              40 Start watching this issue

              Dates

              • Created:
                Updated:

                Development