Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.94.6, 0.95.0
    • Fix Version/s: 0.98.0, 0.94.7, 0.95.1
    • Component/s: None
    • Labels:
      None
    • Release Note:
      Hide
      HBase clients from 0.94.7 going forward support the following new API for Mutations (Put/Delete/Append/Increment).
      Mutation.setDurability(Durability). Possible durability settings are: USE_DEFAULT (use whatever the table has been configured with), SKIP_WAL (do not write anything to the WAL), ASYNC_WAL (write to the WAL asynchronously), SYNC (write to the WAL synchrously), FSYNC (write to the WAL synchronously and force to disc everywhere - currently not supported).

      Regionservers prior to 0.94.7 with ignore anything but SKIP_WAL and assume USE_DEFAULT.
      Show
      HBase clients from 0.94.7 going forward support the following new API for Mutations (Put/Delete/Append/Increment). Mutation.setDurability(Durability). Possible durability settings are: USE_DEFAULT (use whatever the table has been configured with), SKIP_WAL (do not write anything to the WAL), ASYNC_WAL (write to the WAL asynchronously), SYNC (write to the WAL synchrously), FSYNC (write to the WAL synchronously and force to disc everywhere - currently not supported). Regionservers prior to 0.94.7 with ignore anything but SKIP_WAL and assume USE_DEFAULT.

      Description

      Won't have time for parent. But a deferred sync option on a per operation basis comes up quite frequently.
      In 0.96 this can be handled cleanly via protobufs and 0.94 we can have a special mutation attribute.

      For batch operation we'd take the safest sync option of any of the mutations. I.e. if there is at least one that wants to be flushed we'd sync the batch, if there's none of those but at least one that wants deferred flush we defer flush the batch, etc.

      1. 7801-0.94-v7.txt
        16 kB
        Lars Hofhansl
      2. 7801-0.94-v6.txt
        16 kB
        Lars Hofhansl
      3. hbase-7801-addendum.patch
        0.8 kB
        Jeffrey Zhong
      4. 7801-0.94-v5.txt
        16 kB
        Lars Hofhansl
      5. 7801-0.94-v4.txt
        15 kB
        Lars Hofhansl
      6. 7801-0.96-v10.txt
        162 kB
        Lars Hofhansl
      7. 7801-0.96-v9.txt
        156 kB
        Lars Hofhansl
      8. 7801-0.96-v8.txt
        156 kB
        Lars Hofhansl
      9. 7801-0.96-v7.txt
        156 kB
        Lars Hofhansl
      10. 7801-0.96-v6.txt
        156 kB
        Lars Hofhansl
      11. 7801-0.96-full-v5.txt
        152 kB
        Lars Hofhansl
      12. 7801-0.96-full-v4.txt
        150 kB
        Lars Hofhansl
      13. 7801-0.96-full-v3.txt
        151 kB
        Lars Hofhansl
      14. 7801-0.96-full-v2.txt
        149 kB
        Lars Hofhansl
      15. 7801-0.94-v3.txt
        7 kB
        Lars Hofhansl
      16. 7801-0.96-v1.txt
        6 kB
        Lars Hofhansl
      17. 7801-0.94-v2.txt
        5 kB
        Lars Hofhansl
      18. 7801-0.94-v1.txt
        2 kB
        Lars Hofhansl

        Issue Links

          Activity

          Hide
          Andrew Purtell added a comment -

          In 0.96 this can be handled cleanly via protobufs and 0.94 we can have a special mutation attribute.

          We still have op attributes in 0.96 too. Could you say more about how it would be done differently between 0.94 and 0.96+?

          Show
          Andrew Purtell added a comment - In 0.96 this can be handled cleanly via protobufs and 0.94 we can have a special mutation attribute. We still have op attributes in 0.96 too. Could you say more about how it would be done differently between 0.94 and 0.96+?
          Hide
          Lars Hofhansl added a comment -

          We already have a writeToWal on Mutation. Could either use an enum or have an extra deferredFlush flag.
          In 0.94 I can't break the RPC compatibility, so I have to use an attribute. In 0.96 I can do it the right way by changing the RPC.

          Show
          Lars Hofhansl added a comment - We already have a writeToWal on Mutation. Could either use an enum or have an extra deferredFlush flag. In 0.94 I can't break the RPC compatibility, so I have to use an attribute. In 0.96 I can do it the right way by changing the RPC.
          Hide
          Anoop Sam John added a comment -

          For batch operation we'd take the safest sync option of any of the mutations. I.e. if there is at least one that wants to be flushed we'd sync the batch, if there's none of those but at least one that wants deferred flush we defer flush the batch

          This also to be default to defered = false
          When the attribute is missing we can assume the value to be that set for the table?
          When the attribute is there we go by that for the Mutation. And when atleast one need WAL to be synced immediately , go with that. If all need WAL to be defered synced or none have this attribute but at the table level it is defered, then go with defered.
          Lars Hofhansl Here batch will be the mini batch. Which all Mutations will come under one mini batch depends on the row locking.

          Another point not directly related to this issue. Now we have the LogSyncer thread started and running. This thread can come at any point during the put and can do a sync. If all my writes I want to be defered sync = false, this thread may be of no real use for me. In our case of secondary indexing we wanted the sync to happen as one operation (sync write for actual region and that for index region). This is possible but the issue is in between the LogSyncer thread can come and do a sync. So should we make a way to so as to control the start of LogSyncer ?

          Show
          Anoop Sam John added a comment - For batch operation we'd take the safest sync option of any of the mutations. I.e. if there is at least one that wants to be flushed we'd sync the batch, if there's none of those but at least one that wants deferred flush we defer flush the batch This also to be default to defered = false When the attribute is missing we can assume the value to be that set for the table? When the attribute is there we go by that for the Mutation. And when atleast one need WAL to be synced immediately , go with that. If all need WAL to be defered synced or none have this attribute but at the table level it is defered, then go with defered. Lars Hofhansl Here batch will be the mini batch. Which all Mutations will come under one mini batch depends on the row locking. Another point not directly related to this issue. Now we have the LogSyncer thread started and running. This thread can come at any point during the put and can do a sync. If all my writes I want to be defered sync = false, this thread may be of no real use for me. In our case of secondary indexing we wanted the sync to happen as one operation (sync write for actual region and that for index region). This is possible but the issue is in between the LogSyncer thread can come and do a sync. So should we make a way to so as to control the start of LogSyncer ?
          Hide
          Lars Hofhansl added a comment -

          This also to be default to defered = false

          Absolutely. By default nothing will change.

          When the attribute is missing we can assume the value to be that set for the table?

          Yep

          Here batch will be the mini batch.

          Yes. Ideally we would only sync or defer those edits that need it, but if we sync or defer a few more in the batch it is still correct behavior.

          Not sure I completely follow your last paragraph. We need to control the sync and a periodic backround sync is not good enough?
          By setting defer sync you implicitly declare that you are OK with losing (some of) the data in rare cases. If that is not acceptable you'd need sync (not defer) I think.

          Maybe in 0.96 we can think about a way how group edits for different regions (on the same server) and write them into a single WALEdit (similar to RowMutations, but for multiple regions).

          Show
          Lars Hofhansl added a comment - This also to be default to defered = false Absolutely. By default nothing will change. When the attribute is missing we can assume the value to be that set for the table? Yep Here batch will be the mini batch. Yes. Ideally we would only sync or defer those edits that need it, but if we sync or defer a few more in the batch it is still correct behavior. Not sure I completely follow your last paragraph. We need to control the sync and a periodic backround sync is not good enough? By setting defer sync you implicitly declare that you are OK with losing (some of) the data in rare cases. If that is not acceptable you'd need sync (not defer) I think. Maybe in 0.96 we can think about a way how group edits for different regions (on the same server) and write them into a single WALEdit (similar to RowMutations, but for multiple regions).
          Hide
          Anoop Sam John added a comment -

          By setting defer sync you implicitly declare that you are OK with losing (some of) the data in rare cases. If that is not acceptable you'd need sync (not defer) I think.

          Lars I dont need the defered sync in any of the mutation. So every mini batch operation will do a wal write and then one sync at [STEP 7. Sync wal]
          What my point was we have the LogSyncer thread running. It can come and do a sync before the thread, doing the write, coming and doing the sync at step#7. In our case we have to make sure the sync is happening as one unit for the main region and index region. We do write WAL entry for the index region in CP WAL write hooks.

          Maybe in 0.96 we can think about a way how group edits for different regions (on the same server) and write them into a single WALEdit (similar to RowMutations, but for multiple regions).

          I was also thinking in this way. This is already a backlog item I have added for myself here which comes in our sec index further optimization points
          Thanks Lars. I will see working with item.

          Show
          Anoop Sam John added a comment - By setting defer sync you implicitly declare that you are OK with losing (some of) the data in rare cases. If that is not acceptable you'd need sync (not defer) I think. Lars I dont need the defered sync in any of the mutation. So every mini batch operation will do a wal write and then one sync at [STEP 7. Sync wal] What my point was we have the LogSyncer thread running. It can come and do a sync before the thread, doing the write, coming and doing the sync at step#7. In our case we have to make sure the sync is happening as one unit for the main region and index region. We do write WAL entry for the index region in CP WAL write hooks. Maybe in 0.96 we can think about a way how group edits for different regions (on the same server) and write them into a single WALEdit (similar to RowMutations, but for multiple regions). I was also thinking in this way. This is already a backlog item I have added for myself here which comes in our sec index further optimization points Thanks Lars. I will see working with item.
          Hide
          Lars Hofhansl added a comment - - edited

          Here's a surprisingly simple work in progress patch for 0.94.
          This is back and forward compatible between old clients and new servers or vice versa.

          Note: COMPLETELY UNTESTED.

          Show
          Lars Hofhansl added a comment - - edited Here's a surprisingly simple work in progress patch for 0.94. This is back and forward compatible between old clients and new servers or vice versa. Note: COMPLETELY UNTESTED.
          Hide
          Lars Hofhansl added a comment - - edited

          I tested the patch locally. Works fine.
          Any comments?

          I could do the same for 0.96, or make a proper protobuf change there. (btw. for setting the cluster id in trunk we're still using attributes as opposed to encoding this in the protobuf message).
          It is actually an interesting question whether we should maintain optional Mutation attributes via attributes or via a protobuf change.

          Show
          Lars Hofhansl added a comment - - edited I tested the patch locally. Works fine. Any comments? I could do the same for 0.96, or make a proper protobuf change there. (btw. for setting the cluster id in trunk we're still using attributes as opposed to encoding this in the protobuf message). It is actually an interesting question whether we should maintain optional Mutation attributes via attributes or via a protobuf change.
          Hide
          Anoop Sam John added a comment -
          +      if (walEdit.size() > 0 && shouldSyncWal) {
                   syncOrDefer(txid);
                 }
          
          private void syncOrDefer(long txid) throws IOException {
              if (this.regionInfo.isMetaRegion() ||
                !this.htableDescriptor.isDeferredLogFlush()) {
                this.log.sync(txid);
              }
            }
          

          When Mutation specifically say to sync WAL immediately, we need to do so with out considering what is set for Table? If so the syncOrDefer will again defer it right?

          Show
          Anoop Sam John added a comment - + if (walEdit.size() > 0 && shouldSyncWal) { syncOrDefer(txid); } private void syncOrDefer( long txid) throws IOException { if ( this .regionInfo.isMetaRegion() || ! this .htableDescriptor.isDeferredLogFlush()) { this .log.sync(txid); } } When Mutation specifically say to sync WAL immediately, we need to do so with out considering what is set for Table? If so the syncOrDefer will again defer it right?
          Hide
          ramkrishna.s.vasudevan added a comment -

          For Anoop's point on the LogSyncerThread, what we were thinking is to have a config in the RS level itself to say if to allow the LogSyncer thread to start or not.
          But not sure if this can fit in the kernel code.

          Show
          ramkrishna.s.vasudevan added a comment - For Anoop's point on the LogSyncerThread, what we were thinking is to have a config in the RS level itself to say if to allow the LogSyncer thread to start or not. But not sure if this can fit in the kernel code.
          Hide
          Lars Hofhansl added a comment -

          Anoop Sam John I was thinking that if the table is set up with deferred flush then we'd honor that like we do now. Otherwise that option would be always ignored unless all Mutations are marked with deferFlush (something that an old client cannot even do).
          Note that a Mutation does not explicitly say it wants to sync, it only explicitly state when it doesn't.

          Another option to make this a bit nicer is to pass the shouldSyncWal flag to syncOrDefer and hence all the decision logic about whether to flush or not in that method.

          Show
          Lars Hofhansl added a comment - Anoop Sam John I was thinking that if the table is set up with deferred flush then we'd honor that like we do now. Otherwise that option would be always ignored unless all Mutations are marked with deferFlush (something that an old client cannot even do). Note that a Mutation does not explicitly say it wants to sync, it only explicitly state when it doesn't. Another option to make this a bit nicer is to pass the shouldSyncWal flag to syncOrDefer and hence all the decision logic about whether to flush or not in that method.
          Hide
          Lars Hofhansl added a comment -

          ramkrishna.s.vasudevan Not running the LogSyncer thread at all seems like a recipe for unexpected behavior.
          Or are you saying you to temporarily disable the syncer thread, while a bigger "transaction" is in progress?

          Show
          Lars Hofhansl added a comment - ramkrishna.s.vasudevan Not running the LogSyncer thread at all seems like a recipe for unexpected behavior. Or are you saying you to temporarily disable the syncer thread, while a bigger "transaction" is in progress?
          Hide
          ramkrishna.s.vasudevan added a comment -

          Yes right Lars.
          If the Log syncer is on, suppose thro the WALObserver we have done a WAL edit and in the main flow we do another WAlEdit but both needs to be synced together as atomic, then we need to the logsyncer to be disabled.
          Or else the logsyncer may sync up the first Edit that happened thro WALObserver and then when the main sync happens there could be a failure leading to inconsistency.

          Show
          ramkrishna.s.vasudevan added a comment - Yes right Lars. If the Log syncer is on, suppose thro the WALObserver we have done a WAL edit and in the main flow we do another WAlEdit but both needs to be synced together as atomic, then we need to the logsyncer to be disabled. Or else the logsyncer may sync up the first Edit that happened thro WALObserver and then when the main sync happens there could be a failure leading to inconsistency.
          Hide
          Anoop Sam John added a comment -

          Lars Hofhansl What all use cases you see? I have one table for which I want a sync immediately for almost all Mutations. For certain others I can have defered flush.
          Can there be reverse case also? In the current implementation how we can achieve that. Sorry I dont have any clear use case with me for this. What do you say?

          Show
          Anoop Sam John added a comment - Lars Hofhansl What all use cases you see? I have one table for which I want a sync immediately for almost all Mutations. For certain others I can have defered flush. Can there be reverse case also? In the current implementation how we can achieve that. Sorry I dont have any clear use case with me for this. What do you say?
          Hide
          Anoop Sam John added a comment -

          Not running the LogSyncer thread at all seems like a recipe for unexpected behavior.

          Or are you saying you to temporarily disable the syncer thread, while a bigger "transaction" is in progress?

          In our case we didn't want the LogSyncer to be there at all as all the Mutation to all tables need to be sync immediately.

          Show
          Anoop Sam John added a comment - Not running the LogSyncer thread at all seems like a recipe for unexpected behavior. Or are you saying you to temporarily disable the syncer thread, while a bigger "transaction" is in progress? In our case we didn't want the LogSyncer to be there at all as all the Mutation to all tables need to be sync immediately.
          Hide
          Lars Hofhansl added a comment -

          The use case that I see a lot at Saleforce is similar to the writeToWAL use case. By default the Mutation is written to the WAL unless the Mutation says not to. The defer case is the same IMHO.

          Doing the reverse without making it confusing would require a "forceFlush" (or something like this) option in the Mutation. So we'd have writeToWal, deferFlush, and forceFlush. Even then IMHO that would be more confusing the helpful.

          The key here is that it should possible to decide per Mutation whether to defer or not. Without this change that is not possible. With this change it is possible by leaving the table's default (flush) and then set the defer bit on a Mutation.

          Show
          Lars Hofhansl added a comment - The use case that I see a lot at Saleforce is similar to the writeToWAL use case. By default the Mutation is written to the WAL unless the Mutation says not to. The defer case is the same IMHO. Doing the reverse without making it confusing would require a "forceFlush" (or something like this) option in the Mutation. So we'd have writeToWal, deferFlush, and forceFlush. Even then IMHO that would be more confusing the helpful. The key here is that it should possible to decide per Mutation whether to defer or not. Without this change that is not possible. With this change it is possible by leaving the table's default (flush) and then set the defer bit on a Mutation.
          Hide
          Anoop Sam John added a comment -

          Lars Hofhansl Thanks for sharing the use case.
          So we can say
          1. When for a table most of the Mutations need WAL write flush immediately and very few can come with defer flush, we will not set the table defer flush but those Mutations to be defer flush
          2. When for a table most of the Mutations can be defer flushed and few must be immediately flushed, we will not set table defer. On majority of the Mutations we will set defer flush attribute. Those which need immediate flush, we wont set the defer attribute.
          3. When all the Mutations on a table can go with defer flush, set the table defer flush attribute and no other change is needed.

          Just saying as a conclusion. If most of the use case will be like #1, then I am fine Lars. BTW looked at the patch. Looks fine with me otherwise.

          Show
          Anoop Sam John added a comment - Lars Hofhansl Thanks for sharing the use case. So we can say 1. When for a table most of the Mutations need WAL write flush immediately and very few can come with defer flush, we will not set the table defer flush but those Mutations to be defer flush 2. When for a table most of the Mutations can be defer flushed and few must be immediately flushed, we will not set table defer. On majority of the Mutations we will set defer flush attribute. Those which need immediate flush, we wont set the defer attribute. 3. When all the Mutations on a table can go with defer flush, set the table defer flush attribute and no other change is needed. Just saying as a conclusion. If most of the use case will be like #1, then I am fine Lars. BTW looked at the patch. Looks fine with me otherwise.
          Hide
          Lars Hofhansl added a comment -

          Both #1 and #2 will be use cases, and both are possible with this.

          Generally, though, I do not see much interest in this from other folks. Is this not something people come across periodically?

          Show
          Lars Hofhansl added a comment - Both #1 and #2 will be use cases, and both are possible with this. Generally, though, I do not see much interest in this from other folks. Is this not something people come across periodically?
          Hide
          ramkrishna.s.vasudevan added a comment -

          @Lars
          We need this. As far as me and Anoop are concerned, have felt a need for such things.
          +1 on patch.

          Show
          ramkrishna.s.vasudevan added a comment - @Lars We need this. As far as me and Anoop are concerned, have felt a need for such things. +1 on patch.
          Hide
          Lars Hofhansl added a comment -

          Here's a more complete 0.94 patch. Also handles Appends and mutateRow.
          (Note that Increment cannot be supported without an RPC change).

          Show
          Lars Hofhansl added a comment - Here's a more complete 0.94 patch. Also handles Appends and mutateRow. (Note that Increment cannot be supported without an RPC change).
          Hide
          Lars Hofhansl added a comment - - edited

          Here's a 0.96 patch. Also uses Mutation attributes to avoid PB changes (could do this through PB as well).
          Again, Increment is not supported as it does not extend Mutation (but it could be supported if we make an PB change).

          0.96 has RowProcessors, so I added an internal API to allow a row processor to decided whether it requests a sync. By default the response is true.

          Show
          Lars Hofhansl added a comment - - edited Here's a 0.96 patch. Also uses Mutation attributes to avoid PB changes (could do this through PB as well). Again, Increment is not supported as it does not extend Mutation (but it could be supported if we make an PB change). 0.96 has RowProcessors, so I added an internal API to allow a row processor to decided whether it requests a sync. By default the response is true.
          Hide
          Lars Hofhansl added a comment -

          Getting a HadoopQA run.

          Show
          Lars Hofhansl added a comment - Getting a HadoopQA run.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12569695/7801-0.96-v1.txt
          against trunk revision .

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

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

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

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

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

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

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

          -1 lineLengths. The patch introduces lines longer than 100

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

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4450//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4450//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4450//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4450//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4450//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4450//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4450//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4450//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4450//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/12569695/7801-0.96-v1.txt against trunk revision . +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces lines longer than 100 +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4450//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4450//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4450//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4450//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4450//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4450//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4450//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4450//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4450//console This message is automatically generated.
          Hide
          Lars Hofhansl added a comment -

          Will fix the line length. As for this patch, any comments?
          Have been thinking about unittest, but this would tricky to verify (we do not have any test for deferred log flush, probably for the same reason).

          In our case we didn't want the LogSyncer to be there at all

          Looking at the code, we could just not start the LogSyncer if hbase.regionserver.optionallogflushinterval is set to 0. I'd fear there might be other side effects, but it could just work. But that's for a different jira anyway.

          Show
          Lars Hofhansl added a comment - Will fix the line length. As for this patch, any comments? Have been thinking about unittest, but this would tricky to verify (we do not have any test for deferred log flush, probably for the same reason). In our case we didn't want the LogSyncer to be there at all Looking at the code, we could just not start the LogSyncer if hbase.regionserver.optionallogflushinterval is set to 0. I'd fear there might be other side effects, but it could just work. But that's for a different jira anyway.
          Hide
          Ted Yu added a comment -

          I looked at trunk patch. It looks good.

          +   * @return true of the resulting edit should sync'ed to the WAL, false if not.
          

          Please correct the syntax of above javadoc.

          Show
          Ted Yu added a comment - I looked at trunk patch. It looks good. + * @ return true of the resulting edit should sync'ed to the WAL, false if not. Please correct the syntax of above javadoc.
          Hide
          Anoop Sam John added a comment -

          +1
          We can add in release notes about how to achieve the different use cases.

          Increment is not supported as it does not extend Mutation (but it could be supported if we make an PB change).

          For 0.96 we have a JIRA issue for making Increment also a Mutation right? I forgot the issue id.

          Show
          Anoop Sam John added a comment - +1 We can add in release notes about how to achieve the different use cases. Increment is not supported as it does not extend Mutation (but it could be supported if we make an PB change). For 0.96 we have a JIRA issue for making Increment also a Mutation right? I forgot the issue id.
          Hide
          Enis Soztutar added a comment -

          For 0.94, it looks good, but for trunk I think we can merge writeToWAL and syncWAL into one enum. Smt like:

          message Mutate {
            required bytes row = 1;
            ....
            enum WriteGuarantee {
              SYNC_WAL  = 0, 
              ASYNC_WAL = 1,
              SKIP_WAL  = 2
            }
            optional WriteGuarantee writeGuarantee = 6 [default = SYNC_WAL];
          

          I am sure, we can think of a better name than WriteGuarantee though.

          Show
          Enis Soztutar added a comment - For 0.94, it looks good, but for trunk I think we can merge writeToWAL and syncWAL into one enum. Smt like: message Mutate { required bytes row = 1; .... enum WriteGuarantee { SYNC_WAL = 0, ASYNC_WAL = 1, SKIP_WAL = 2 } optional WriteGuarantee writeGuarantee = 6 [ default = SYNC_WAL]; I am sure, we can think of a better name than WriteGuarantee though.
          Hide
          Lars Hofhansl added a comment -

          Was trying to avoid a PB change, but there's really no reason to.

          Maybe while I'm at it I might remove the cluster id attribute as well in favor of an attribute on the PB message.

          Show
          Lars Hofhansl added a comment - Was trying to avoid a PB change, but there's really no reason to. Maybe while I'm at it I might remove the cluster id attribute as well in favor of an attribute on the PB message.
          Hide
          Lars Hofhansl added a comment -

          Oh another problem with PB is that we need to introduce two enums, because we do not want to leak the PB definition into the public API. So a public enum that is part of the API that is then translated into the PB enum. (Did I mention I do not like PB? )

          Show
          Lars Hofhansl added a comment - Oh another problem with PB is that we need to introduce two enums, because we do not want to leak the PB definition into the public API. So a public enum that is part of the API that is then translated into the PB enum. (Did I mention I do not like PB? )
          Hide
          Enis Soztutar added a comment -

          Makes sense. I think it is not a horrible idea to expose the PB's as client API's. If we want to have non-java clients (especially third party ones), then the java api is not our only client-facing API. Our RPC, and the PB's are client facing as well. Also, we might get rid of the client objects -> PB -> wire -> PB -> server objects flow, and go with client PB -> wire -> server PB -> wal/memstore.

          Show
          Enis Soztutar added a comment - Makes sense. I think it is not a horrible idea to expose the PB's as client API's. If we want to have non-java clients (especially third party ones), then the java api is not our only client-facing API. Our RPC, and the PB's are client facing as well. Also, we might get rid of the client objects -> PB -> wire -> PB -> server objects flow, and go with client PB -> wire -> server PB -> wal/memstore.
          Hide
          Lars Hofhansl added a comment -

          As far the Java API in concerned the fact that we use PB is an implementation detail and should not leak out.

          Are you also suggesting changing the Mutation API? I.e. remove

          {set|get}WriteToWAL and replace it with setWriteGuarantee(WriteGuarantee wg)?

          Could do that in 0.94 as well (but still use Attributes to transmit the information - and obviously we have to keep {set|get}

          WriteToWAL in 0.94)

          Something like setWriteGuarantee(WriteGuarantee wg) is certainly nice, because it let's us add more options later (like a true sync to disk).

          Show
          Lars Hofhansl added a comment - As far the Java API in concerned the fact that we use PB is an implementation detail and should not leak out. Are you also suggesting changing the Mutation API? I.e. remove {set|get}WriteToWAL and replace it with setWriteGuarantee(WriteGuarantee wg)? Could do that in 0.94 as well (but still use Attributes to transmit the information - and obviously we have to keep {set|get} WriteToWAL in 0.94) Something like setWriteGuarantee(WriteGuarantee wg) is certainly nice, because it let's us add more options later (like a true sync to disk).
          Hide
          Enis Soztutar added a comment - - edited

          As far the Java API in concerned the fact that we use PB is an implementation detail and should not leak out.

          My argument above is definitely longer term, when we decide to officially support multi-language clients.

          Are you also suggesting changing the Mutation API? I.e. remove (set|get)WriteToWAL and replace it with setWriteGuarantee(WriteGuarantee wg)

          we can do both. Add the enums in the java client API, and the PB serialization. We can deprecate (set|get)WriteToWAL() in 0.94, and remove in 0.96.

          Show
          Enis Soztutar added a comment - - edited As far the Java API in concerned the fact that we use PB is an implementation detail and should not leak out. My argument above is definitely longer term, when we decide to officially support multi-language clients. Are you also suggesting changing the Mutation API? I.e. remove (set|get)WriteToWAL and replace it with setWriteGuarantee(WriteGuarantee wg) we can do both. Add the enums in the java client API, and the PB serialization. We can deprecate (set|get)WriteToWAL() in 0.94, and remove in 0.96.
          Hide
          Sergey Shelukhin added a comment -
          +        } else if (!shouldSyncWal) {
          +          shouldSyncWal |= !m.getDeferWALFlush();
          
          = is not necessary.
          Show
          Sergey Shelukhin added a comment - + } else if (!shouldSyncWal) { + shouldSyncWal |= !m.getDeferWALFlush(); = is not necessary.
          Hide
          Lars Hofhansl added a comment -

          Yep that was an artifact of of my first attempt where I just or'd shouldSyncWal in each iterarion before I added the else if.

          Show
          Lars Hofhansl added a comment - Yep that was an artifact of of my first attempt where I just or'd shouldSyncWal in each iterarion before I added the else if.
          Hide
          Lars Hofhansl added a comment - - edited

          0.94 patch implementing {get|set}WriteGuarantee on Mutation.

          Not exactly pretty, because this must be backwards compatible with older servers and clients (so still using the writeToWal and deferredFlus bits).

          But at least we can deprecate the API now and remove in 0.95.

          Show
          Lars Hofhansl added a comment - - edited 0.94 patch implementing { get|set}WriteGuarantee on Mutation. Not exactly pretty, because this must be backwards compatible with older servers and clients (so still using the writeToWal and deferredFlus bits). But at least we can deprecate the API now and remove in 0.95.
          Hide
          Lars Hofhansl added a comment -

          new trunk patch with PB coming tomorrow.

          Show
          Lars Hofhansl added a comment - new trunk patch with PB coming tomorrow.
          Hide
          Lars Hofhansl added a comment -

          Sigh... I will try to have trunk patch tomorrow.

          Show
          Lars Hofhansl added a comment - Sigh... I will try to have trunk patch tomorrow.
          Hide
          Lars Hofhansl added a comment -

          Sigh... Need to push, I just do not have time to get to this.

          Show
          Lars Hofhansl added a comment - Sigh... Need to push, I just do not have time to get to this.
          Hide
          Lars Hofhansl added a comment -

          Here's a fairly large patch that changes uses PB and does away with the old writeToWAL flag.
          (It compiles, that's all I tests with this patch)

          Show
          Lars Hofhansl added a comment - Here's a fairly large patch that changes uses PB and does away with the old writeToWAL flag. (It compiles, that's all I tests with this patch)
          Hide
          Lars Hofhansl added a comment -

          Any comments?

          Show
          Lars Hofhansl added a comment - Any comments?
          Hide
          Anoop Sam John added a comment -

          Lars Hofhansl I am going through the patch.

          Show
          Anoop Sam John added a comment - Lars Hofhansl I am going through the patch.
          Hide
          ramkrishna.s.vasudevan added a comment -

          Will check this later today Lars. By the time if Anoop has gone over then fine.

          Show
          ramkrishna.s.vasudevan added a comment - Will check this later today Lars. By the time if Anoop has gone over then fine.
          Hide
          Anoop Sam John added a comment -

          Gone through the changes in src code
          1.

          /**
          +   * Write the Mutation of the WAL synchronously.
          

          you mean Mutation to the WAL? In other places too

          2. HRegion#increment(Increment increment)

          -        syncOrDefer(txid); // sync the transaction log outside the rowlock
          +        syncOrDefer(txid, true); // sync the transaction log outside the rowlock
          

          Hard coded true. Can check whether defered sync or not? Increment is a Mutation.

          Will see test code part now. Patch not cleanly applying on TestRestoreFlushSnapshotFromClient.java

          Show
          Anoop Sam John added a comment - Gone through the changes in src code 1. /** + * Write the Mutation of the WAL synchronously. you mean Mutation to the WAL? In other places too 2. HRegion#increment(Increment increment) - syncOrDefer(txid); // sync the transaction log outside the rowlock + syncOrDefer(txid, true ); // sync the transaction log outside the rowlock Hard coded true. Can check whether defered sync or not? Increment is a Mutation. Will see test code part now. Patch not cleanly applying on TestRestoreFlushSnapshotFromClient.java
          Hide
          Anoop Sam John added a comment - - edited

          Change of writeToWAL -> writeGuarantee not handled in all the places?
          eg: Delete.java - Delete(final Delete d)
          Put(Put putToCopy)
          etc

          Show
          Anoop Sam John added a comment - - edited Change of writeToWAL -> writeGuarantee not handled in all the places? eg: Delete.java - Delete(final Delete d) Put(Put putToCopy) etc
          Hide
          Anoop Sam John added a comment -

          1. HBaseTestCase

          -      this.region.delete(delete, writeToWAL);
          +      this.region.delete(delete);
          

          Do we need to set SKIP_WAL/SYNC_WAL based on writeToWAL into Delete object?

          2.TestFilter

                 Delete d = new Delete(ROW);
                 d.deleteColumns(FAMILIES[0], QUALIFIERS_ONE[1]);
                 d.deleteColumns(FAMILIES[1], QUALIFIERS_ONE[1]);
          -      this.region.delete(d, false);
          +      this.region.delete(d);
          

          Do we need to set SKIP_WAL into Delete object?

          @@ -217,7 +218,7 @@
               // create new rows and column family to show how reseek works..
               for (byte[] ROW : ROWS_THREE) {
                 Put p = new Put(ROW);
          -      p.setWriteToWAL(true);
          +      p.setWriteGuarantee(WriteGuarantee.SKIP_WAL);
          

          Not SKIP_WAL but SYNC_WAL.

          3.TestProtobufUtil

          -    assertEquals(true, proto.getWriteToWAL());
          +    assertEquals(true, proto.getWriteGuarantee());
          

          Assertion is wrong now. Test will fail. Other places also where assertEquals related changes are there.

          4.TestCompaction

          -      r.delete(new Delete(results.get(0).getRow()), false);
          +      r.delete(new Delete(results.get(0).getRow()));
          

          Do we need to set SKIP_WAL into Delete object?

          5.TestGetClosestAtOrBefore

          -        mr.delete(new Delete(keys.get(0).getRow()), false);
          +        mr.delete(new Delete(keys.get(0).getRow()));
          

          Do we need to set SKIP_WAL into Delete object? And other similar places in this file

          6.TestHRegion

          -    region.delete(delete, false);
          +    region.delete(delete);
          

          Do we need to set SKIP_WAL into Delete object?

          Show
          Anoop Sam John added a comment - 1. HBaseTestCase - this .region.delete(delete, writeToWAL); + this .region.delete(delete); Do we need to set SKIP_WAL/SYNC_WAL based on writeToWAL into Delete object? 2.TestFilter Delete d = new Delete(ROW); d.deleteColumns(FAMILIES[0], QUALIFIERS_ONE[1]); d.deleteColumns(FAMILIES[1], QUALIFIERS_ONE[1]); - this .region.delete(d, false ); + this .region.delete(d); Do we need to set SKIP_WAL into Delete object? @@ -217,7 +218,7 @@ // create new rows and column family to show how reseek works.. for ( byte [] ROW : ROWS_THREE) { Put p = new Put(ROW); - p.setWriteToWAL( true ); + p.setWriteGuarantee(WriteGuarantee.SKIP_WAL); Not SKIP_WAL but SYNC_WAL. 3.TestProtobufUtil - assertEquals( true , proto.getWriteToWAL()); + assertEquals( true , proto.getWriteGuarantee()); Assertion is wrong now. Test will fail. Other places also where assertEquals related changes are there. 4.TestCompaction - r.delete( new Delete(results.get(0).getRow()), false ); + r.delete( new Delete(results.get(0).getRow())); Do we need to set SKIP_WAL into Delete object? 5.TestGetClosestAtOrBefore - mr.delete( new Delete(keys.get(0).getRow()), false ); + mr.delete( new Delete(keys.get(0).getRow())); Do we need to set SKIP_WAL into Delete object? And other similar places in this file 6.TestHRegion - region.delete(delete, false ); + region.delete(delete); Do we need to set SKIP_WAL into Delete object?
          Hide
          Anoop Sam John added a comment -

          If you are busy and need any help let me know Lars. I can help you

          Show
          Anoop Sam John added a comment - If you are busy and need any help let me know Lars. I can help you
          Hide
          Lars Hofhansl added a comment -

          Thanks Anoop.

          New patch coming soon. Please note that HRegion.delete as well as HRegion.put completely ignored the 2nd parameter, which is why I just removed it.

          Show
          Lars Hofhansl added a comment - Thanks Anoop. New patch coming soon. Please note that HRegion.delete as well as HRegion.put completely ignored the 2nd parameter, which is why I just removed it.
          Hide
          Lars Hofhansl added a comment -

          I think p.setWriteToWAL(true) in TestFilter is a typo.

          Show
          Lars Hofhansl added a comment - I think p.setWriteToWAL(true) in TestFilter is a typo.
          Hide
          Lars Hofhansl added a comment -

          Updated patch based on Anoop's comments.

          Show
          Lars Hofhansl added a comment - Updated patch based on Anoop's comments.
          Hide
          Lars Hofhansl added a comment -

          Let's get a HadoopQA run.

          Show
          Lars Hofhansl added a comment - Let's get a HadoopQA run.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12573393/7801-0.96-full-v3.txt
          against trunk revision .

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

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

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

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

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

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

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

          -1 lineLengths. The patch introduces lines longer than 100

          -1 site. The patch appears to cause mvn site goal to fail.

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.protobuf.TestProtobufUtil

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4781//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4781//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4781//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4781//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4781//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4781//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4781//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4781//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4781//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4781//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/12573393/7801-0.96-full-v3.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 153 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. -1 javadoc . The javadoc tool appears to have generated 6 warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces lines longer than 100 -1 site . The patch appears to cause mvn site goal to fail. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.protobuf.TestProtobufUtil Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4781//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4781//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4781//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4781//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4781//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4781//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4781//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4781//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4781//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4781//console This message is automatically generated.
          Hide
          Lars Hofhansl added a comment -

          Fixes for TestProtobufUtil.
          This should be close to the final version. Please have a look.

          Show
          Lars Hofhansl added a comment - Fixes for TestProtobufUtil. This should be close to the final version. Please have a look.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12573996/7801-0.96-full-v4.txt
          against trunk revision .

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

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

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

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

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

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

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

          -1 lineLengths. The patch introduces lines longer than 100

          -1 site. The patch appears to cause mvn site goal to fail.

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

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4847//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4847//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4847//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4847//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4847//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4847//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4847//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4847//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4847//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4847//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/12573996/7801-0.96-full-v4.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 153 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. -1 javadoc . The javadoc tool appears to have generated 6 warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces lines longer than 100 -1 site . The patch appears to cause mvn site goal to fail. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4847//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4847//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4847//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4847//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4847//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4847//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4847//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4847//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4847//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4847//console This message is automatically generated.
          Hide
          Ted Yu added a comment - - edited

          Can you take a look at the javadoc warnings ?

          Thanks, Lars.

          Show
          Ted Yu added a comment - - edited Can you take a look at the javadoc warnings ? Thanks, Lars.
          Hide
          Lars Hofhansl added a comment -

          Of course. Will also look at the long line complaint.

          Show
          Lars Hofhansl added a comment - Of course. Will also look at the long line complaint.
          Hide
          Lars Hofhansl added a comment -

          Should address javadoc and line length warning.

          Show
          Lars Hofhansl added a comment - Should address javadoc and line length warning.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12574030/7801-0.96-full-v5.txt
          against trunk revision .

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

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

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

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

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

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

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

          -1 lineLengths. The patch introduces lines longer than 100

          -1 site. The patch appears to cause mvn site goal to fail.

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

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4850//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4850//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4850//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4850//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4850//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4850//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4850//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4850//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4850//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4850//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/12574030/7801-0.96-full-v5.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 153 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces lines longer than 100 -1 site . The patch appears to cause mvn site goal to fail. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4850//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4850//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4850//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4850//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4850//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4850//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4850//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4850//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4850//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4850//console This message is automatically generated.
          Hide
          Lars Hofhansl added a comment -

          Are generated filed (like ClientProtos.java) excluded from the long-line check?

          Show
          Lars Hofhansl added a comment - Are generated filed (like ClientProtos.java) excluded from the long-line check?
          Hide
          Ted Yu added a comment -

          I don't think so:

            ll=`cat $PATCH_DIR/patch | grep "^+" | grep -v "^@@" | grep -v "^+++" | grep -v "import" | wc -L`
            MAX_LINE_LENGTH_PATCH=`expr $MAX_LINE_LENGTH + 1`
            if [[ "$ll" -gt "$MAX_LINE_LENGTH_PATCH" ]]; then
          
          Show
          Ted Yu added a comment - I don't think so: ll=`cat $PATCH_DIR/patch | grep "^+" | grep -v "^@@" | grep -v "^+++" | grep -v " import " | wc -L` MAX_LINE_LENGTH_PATCH=`expr $MAX_LINE_LENGTH + 1` if [[ "$ll" -gt "$MAX_LINE_LENGTH_PATCH" ]]; then
          Hide
          Lars Hofhansl added a comment -

          Might be a good thing to add anyway.
          The long lines left that I noticed were in ClientProtos.java. I'll double check the patch.

          Show
          Lars Hofhansl added a comment - Might be a good thing to add anyway. The long lines left that I noticed were in ClientProtos.java. I'll double check the patch.
          Hide
          Ted Yu added a comment -

          I logged HBASE-8129 for making long line detection smarter

          Show
          Ted Yu added a comment - I logged HBASE-8129 for making long line detection smarter
          Hide
          Anoop Sam John added a comment -

          Gone through the latest Trunk Patch. Seems good to me. +1
          It is not cleanly applying (Conflict with Hregion.java etc..)

          Show
          Anoop Sam John added a comment - Gone through the latest Trunk Patch. Seems good to me. +1 It is not cleanly applying (Conflict with Hregion.java etc..)
          Hide
          Enis Soztutar added a comment -

          Are generated filed (like ClientProtos.java) excluded from the long-line check?

          I logged HBASE-8129 for making long line detection smarter

          No, it is just a simple bash script.

          Show
          Enis Soztutar added a comment - Are generated filed (like ClientProtos.java) excluded from the long-line check? I logged HBASE-8129 for making long line detection smarter No, it is just a simple bash script.
          Hide
          Enis Soztutar added a comment -

          I think this is a good time to think about how are we going to add proper hsync support. Are you thinking smt like:

          public enum WriteGuarantee {
            SKIP_WAL,
            ASYNC_WAL,
            SYNC_WAL,
            FSYNC_WAL,
          }
          

          How about SKIP_WAL, DEFERRED_SYNC_WAL, HFLUSH_WAL, HSYNC_WAL. Just my 2 cents, I am not so good at naming.

          Show
          Enis Soztutar added a comment - I think this is a good time to think about how are we going to add proper hsync support. Are you thinking smt like: public enum WriteGuarantee { SKIP_WAL, ASYNC_WAL, SYNC_WAL, FSYNC_WAL, } How about SKIP_WAL, DEFERRED_SYNC_WAL, HFLUSH_WAL, HSYNC_WAL. Just my 2 cents, I am not so good at naming.
          Hide
          Lars Hofhansl added a comment -

          Yeah I was wondering about this too. Not sure we want to leak the Hadoop nomenclature up here. I'm not good at naming either.

          Show
          Lars Hofhansl added a comment - Yeah I was wondering about this too. Not sure we want to leak the Hadoop nomenclature up here. I'm not good at naming either.
          Hide
          Enis Soztutar added a comment -

          Not sure we want to leak the Hadoop nomenclature up here.

          We have to explain the semantics of hflush vs hsync somehow to the user.

          Show
          Enis Soztutar added a comment - Not sure we want to leak the Hadoop nomenclature up here. We have to explain the semantics of hflush vs hsync somehow to the user.
          Hide
          ramkrishna.s.vasudevan added a comment -

          Patch looks fine to me. Did not check the testcases though.
          If DefferedLogFlush is enabled in HTD and my mutation says ASYNC_WAL it will go with defferedLogFlush right? Just for clarification.

          Show
          ramkrishna.s.vasudevan added a comment - Patch looks fine to me. Did not check the testcases though. If DefferedLogFlush is enabled in HTD and my mutation says ASYNC_WAL it will go with defferedLogFlush right? Just for clarification.
          Hide
          Anoop Sam John added a comment -

          If DefferedLogFlush is enabled in HTD and my mutation says ASYNC_WAL it will go with defferedLogFlush right? Just for clarification.

          Yes

          When table is not having DefferedLogFlush set, if all the Mutations in a mini batch says for async flush, the flush will be deferred.

          Show
          Anoop Sam John added a comment - If DefferedLogFlush is enabled in HTD and my mutation says ASYNC_WAL it will go with defferedLogFlush right? Just for clarification. Yes When table is not having DefferedLogFlush set, if all the Mutations in a mini batch says for async flush, the flush will be deferred.
          Hide
          Enis Soztutar added a comment -

          Two more comments:
          Should we rename WriteGuarantee -> Durability, as in ACID? WriteGuarantee is good enough, but Durability is well understood. Alternatively DurabilityGuarantee?

          In,

          +  private void syncOrDefer(long txid, boolean syncRequested) throws IOException {
          +    if (this.getRegionInfo().isMetaRegion()
          +        || (!this.htableDescriptor.isDeferredLogFlush() && syncRequested)
          +        || this.deferredLogSyncDisabled) {
                 this.log.sync(txid);
               }
          

          The logic seems to be that deferred flush in HDT overrides WriteGuarantee in Mutation. Is this intended? I think, semantically, it would be most user friendly, if

          • Durability setting in Mutation always takes precedence
          • If no Durability setting in Mutation, the one in HTD takes into affect. This allows us to do sync mutations to otherwise deferred column families, and vice versa. This can be achieved, by defining a Durability setting in HTD (getting rid of current skipwal and deferred stuff), and defaulting it to SYNC_WAL. Mutation will have an optional Durability which defaults to null. Wdyt?
          Show
          Enis Soztutar added a comment - Two more comments: Should we rename WriteGuarantee -> Durability, as in ACID? WriteGuarantee is good enough, but Durability is well understood. Alternatively DurabilityGuarantee? In, + private void syncOrDefer( long txid, boolean syncRequested) throws IOException { + if ( this .getRegionInfo().isMetaRegion() + || (! this .htableDescriptor.isDeferredLogFlush() && syncRequested) + || this .deferredLogSyncDisabled) { this .log.sync(txid); } The logic seems to be that deferred flush in HDT overrides WriteGuarantee in Mutation. Is this intended? I think, semantically, it would be most user friendly, if Durability setting in Mutation always takes precedence If no Durability setting in Mutation, the one in HTD takes into affect. This allows us to do sync mutations to otherwise deferred column families, and vice versa. This can be achieved, by defining a Durability setting in HTD (getting rid of current skipwal and deferred stuff), and defaulting it to SYNC_WAL. Mutation will have an optional Durability which defaults to null. Wdyt?
          Hide
          Lars Hofhansl added a comment -

          Enis Soztutar Anoop had the same question above. I do think the column family setting should take precedence, unless we want rethink this completely. For example an option in the WriteGuarantee enum could be USE_DEFAULT, which would do whatever was setup for the CF.

          The CF should probably also have an option to avoid writing the WAL entries... But that's for another jira.

          Show
          Lars Hofhansl added a comment - Enis Soztutar Anoop had the same question above. I do think the column family setting should take precedence, unless we want rethink this completely. For example an option in the WriteGuarantee enum could be USE_DEFAULT, which would do whatever was setup for the CF. The CF should probably also have an option to avoid writing the WAL entries... But that's for another jira.
          Hide
          Lars Hofhansl added a comment -
          • Rebased patch
          • Introduces FSYNC (noop for now, does the same as SYNC)
          • Introduces USE_DEFAULT, to indicate to just use the CF's default setting
          • Renames WriteGuarantee to Durability

          Not tested at all, but it compiles.

          Please have a look. If the API is OK, I'll match that in the 0.94 patch.

          Show
          Lars Hofhansl added a comment - Rebased patch Introduces FSYNC (noop for now, does the same as SYNC) Introduces USE_DEFAULT, to indicate to just use the CF's default setting Renames WriteGuarantee to Durability Not tested at all, but it compiles. Please have a look. If the API is OK, I'll match that in the 0.94 patch.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12577359/7801-0.96-v6.txt
          against trunk revision .

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

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

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

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

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

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

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

          -1 lineLengths. The patch introduces lines longer than 100

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

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.protobuf.TestProtobufUtil

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/5163//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5163//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5163//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5163//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5163//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5163//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5163//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5163//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5163//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/5163//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/12577359/7801-0.96-v6.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 156 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. -1 javadoc . The javadoc tool appears to have generated 1 warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces lines longer than 100 +1 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.protobuf.TestProtobufUtil Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/5163//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5163//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5163//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5163//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5163//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5163//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5163//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5163//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5163//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/5163//console This message is automatically generated.
          Hide
          Lars Hofhansl added a comment -

          Fixed TestProtobufUtil

          Show
          Lars Hofhansl added a comment - Fixed TestProtobufUtil
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12577426/7801-0.96-v7.txt
          against trunk revision .

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

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

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

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

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

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

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

          -1 lineLengths. The patch introduces lines longer than 100

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

          -1 core tests. The patch failed these unit tests:
          org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/5166//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5166//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5166//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5166//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5166//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5166//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5166//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5166//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5166//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/5166//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/12577426/7801-0.96-v7.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 156 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. -1 javadoc . The javadoc tool appears to have generated 1 warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces lines longer than 100 +1 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/5166//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5166//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5166//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5166//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5166//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5166//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5166//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5166//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5166//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/5166//console This message is automatically generated.
          Hide
          Lars Hofhansl added a comment -

          Any comments on the latest patch? It should give us all the functionality with maximum future flexibility.

          Show
          Lars Hofhansl added a comment - Any comments on the latest patch? It should give us all the functionality with maximum future flexibility.
          Hide
          Anoop Sam John added a comment -

          Lars Hofhansl I will see the patch today.

          Show
          Anoop Sam John added a comment - Lars Hofhansl I will see the patch today.
          Hide
          Enis Soztutar added a comment -

          API looks good. A couple of comments:

          • There are still some places referring writeGuarantee, useWriteGuarantee, or wg. Could you please change those to be consistent.
          • We rely on Durability enum ordinals being in ascending sorted order in terms of their semantic guarantees. We should note this in the enum definitions. Does it make sense to allow more room in between? Just thinking out loud, fsync ack from majority of replicas, etc?
          • Durability.java need InterfaceAudience annotations.
          • Should we add some pointers in SYNC_WAL, and FSYNC_WAL javadocs to hflush, hsync semantics? It might be confusing to first time comers. Can do as a follow up.
          • Should BaseRowProcessor#useWriteGuarantee() return USE_DEFAULT? It is not used, but for documentation, and further reference.
          Show
          Enis Soztutar added a comment - API looks good. A couple of comments: There are still some places referring writeGuarantee, useWriteGuarantee, or wg. Could you please change those to be consistent. We rely on Durability enum ordinals being in ascending sorted order in terms of their semantic guarantees. We should note this in the enum definitions. Does it make sense to allow more room in between? Just thinking out loud, fsync ack from majority of replicas, etc? Durability.java need InterfaceAudience annotations. Should we add some pointers in SYNC_WAL, and FSYNC_WAL javadocs to hflush, hsync semantics? It might be confusing to first time comers. Can do as a follow up. Should BaseRowProcessor#useWriteGuarantee() return USE_DEFAULT? It is not used, but for documentation, and further reference.
          Hide
          Lars Hofhansl added a comment -

          Thanks Enis Soztutar. Expect a new patch today.

          Does it make sense to allow more room in between?

          If we use ordinals we should not need to reserve space. The PB enum items are never compared to each other, and for the client side enum only the relationship between the enum items matters, not their absolute positioning.

          Show
          Lars Hofhansl added a comment - Thanks Enis Soztutar . Expect a new patch today. Does it make sense to allow more room in between? If we use ordinals we should not need to reserve space. The PB enum items are never compared to each other, and for the client side enum only the relationship between the enum items matters, not their absolute positioning.
          Hide
          Lars Hofhansl added a comment -

          New patch addressing (hopefully) Enis' comments.

          Show
          Lars Hofhansl added a comment - New patch addressing (hopefully) Enis' comments.
          Hide
          Enis Soztutar added a comment -

          Thanks Lars, this looks better. Could you please address these two as well:

          • Durability.java need InterfaceAudience annotations.
          • Should we add some pointers in SYNC_WAL, and FSYNC_WAL javadocs to hflush, hsync semantics? It might be confusing to first time comers.
          Show
          Enis Soztutar added a comment - Thanks Lars, this looks better. Could you please address these two as well: Durability.java need InterfaceAudience annotations. Should we add some pointers in SYNC_WAL, and FSYNC_WAL javadocs to hflush, hsync semantics? It might be confusing to first time comers.
          Hide
          Lars Hofhansl added a comment -

          I did add the InterfaceAudience... I must have forgotten to save the file before I took the diff, will add again, sorry. (I hope there weren't more changes I didn't save).

          Re: hflush and hsync, can't point to Hadoop Javadoc I think as this changed over the releases (Syncable only has hflush and hsync since hadoop 2.0)

          Show
          Lars Hofhansl added a comment - I did add the InterfaceAudience... I must have forgotten to save the file before I took the diff, will add again, sorry. (I hope there weren't more changes I didn't save). Re: hflush and hsync, can't point to Hadoop Javadoc I think as this changed over the releases (Syncable only has hflush and hsync since hadoop 2.0)
          Hide
          Lars Hofhansl added a comment -

          How about this?

          Show
          Lars Hofhansl added a comment - How about this?
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12577889/7801-0.96-v9.txt
          against trunk revision .

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

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

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

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

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

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

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

          -1 lineLengths. The patch introduces 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/5226//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5226//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5226//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5226//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5226//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5226//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5226//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5226//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5226//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/5226//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/12577889/7801-0.96-v9.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 156 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces 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/5226//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5226//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5226//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5226//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5226//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5226//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5226//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5226//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5226//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/5226//console This message is automatically generated.
          Hide
          Enis Soztutar added a comment -

          +1 from me, the API looks very good now.

          Just a note, below is technically incompatible, but we do not care I guess at this point. Actually using 6 is cleaner.

          -  optional bool writeToWAL = 6 [default = true];
          +  optional Durability durability = 6 [default = USE_DEFAULT];
          
          Show
          Enis Soztutar added a comment - +1 from me, the API looks very good now. Just a note, below is technically incompatible, but we do not care I guess at this point. Actually using 6 is cleaner. - optional bool writeToWAL = 6 [ default = true ]; + optional Durability durability = 6 [ default = USE_DEFAULT];
          Hide
          Anoop Sam John added a comment -

          1.

          -      byte [] row = delete.getRow();
          +      delete.getRow();
          

          We can avoid this line fully right? Good that u make some cleanup also.

          2.

           // do what CF defaults to
           +  if (!isDeferredLogSyncEnabled()) {
          

          We can say do what table defaults to?
          Even here also

          +public enum Durability {
          +  /**
          +   * Use the column family's default setting to determine durability.
          +   * This must remain the first option.
          +   */
          +  USE_DEFAULT,
          

          3.As per code in syncOrDefer() when all the Mutations in the Mini batch says for SKIP_WAL, we should not call the sync.

          +      case SKIP_WAL:
          +        // nothing do to
          +        break;
          

          But see the code in doMiniBatchMutation()

          +      Durability durability = Durability.USE_DEFAULT;
          ...
          +        Durability tmpDur = m.getDurability(); 
          +        if (tmpDur == Durability.SKIP_WAL) {
                     if (m instanceof Put) {
                       recordPutWithoutWal(m.getFamilyMap());
                     }
                     continue;
          +        } else if (tmpDur.ordinal() > durability.ordinal()) {
          +          durability = tmpDur;
                   }
          

          This is correct for not considering one Mutation's SKIP_WAL to affect with the USE_DEFAULT of others. But when all the Mutation say SKIP_WAL, still finally the durability = Durability.USE_DEFAULT and will cause a log sync?

          Excellent work Lars.

          Show
          Anoop Sam John added a comment - 1. - byte [] row = delete.getRow(); + delete.getRow(); We can avoid this line fully right? Good that u make some cleanup also. 2. // do what CF defaults to + if (!isDeferredLogSyncEnabled()) { We can say do what table defaults to? Even here also + public enum Durability { + /** + * Use the column family's default setting to determine durability. + * This must remain the first option. + */ + USE_DEFAULT, 3.As per code in syncOrDefer() when all the Mutations in the Mini batch says for SKIP_WAL, we should not call the sync. + case SKIP_WAL: + // nothing do to + break ; But see the code in doMiniBatchMutation() + Durability durability = Durability.USE_DEFAULT; ... + Durability tmpDur = m.getDurability(); + if (tmpDur == Durability.SKIP_WAL) { if (m instanceof Put) { recordPutWithoutWal(m.getFamilyMap()); } continue ; + } else if (tmpDur.ordinal() > durability.ordinal()) { + durability = tmpDur; } This is correct for not considering one Mutation's SKIP_WAL to affect with the USE_DEFAULT of others. But when all the Mutation say SKIP_WAL, still finally the durability = Durability.USE_DEFAULT and will cause a log sync? Excellent work Lars.
          Hide
          Lars Hofhansl added a comment -

          Thanks Anoop.

          #1 yes
          #2 Oops, you're right. It's per table. Will fix.
          #3 That works, because later in doMiniBatchMutation we only call syncOrDefer if the WALEdit is not empty. But you are right, this can be made safer/easier-to-read if we track the durability even in that case.

          I'm thinking about how I could test this. Would need to something like holding the async flush, and doing that on request in a test. That way one write a bunch, check that it's not in the log, then run the async flush, check again. Maybe for later.

          Show
          Lars Hofhansl added a comment - Thanks Anoop. #1 yes #2 Oops, you're right. It's per table. Will fix. #3 That works, because later in doMiniBatchMutation we only call syncOrDefer if the WALEdit is not empty. But you are right, this can be made safer/easier-to-read if we track the durability even in that case. I'm thinking about how I could test this. Would need to something like holding the async flush, and doing that on request in a test. That way one write a bunch, check that it's not in the log, then run the async flush, check again. Maybe for later.
          Hide
          Anoop Sam John added a comment -

          #3 - Oh yes.. Sorry I missed that.. Ya readability wise it may be better to track this way. Up to you Lars to change or not

          +1 on the patch

          Show
          Anoop Sam John added a comment - #3 - Oh yes.. Sorry I missed that.. Ya readability wise it may be better to track this way. Up to you Lars to change or not +1 on the patch
          Hide
          Lars Hofhansl added a comment -

          Expect a (hopefully) final patch soon.

          Show
          Lars Hofhansl added a comment - Expect a (hopefully) final patch soon.
          Hide
          Enis Soztutar added a comment -

          I'm thinking about how I could test this. Would need to something like holding the async flush, and doing that on request in a test.

          If you plug in a mock hlog implementation, and do no-op in sync(), or do a latch there you might be able to do that. However, I suspect it can still get messy.

          Show
          Enis Soztutar added a comment - I'm thinking about how I could test this. Would need to something like holding the async flush, and doing that on request in a test. If you plug in a mock hlog implementation, and do no-op in sync(), or do a latch there you might be able to do that. However, I suspect it can still get messy.
          Hide
          Lars Hofhansl added a comment -

          Need to rebase the patch again. Grrr.

          Show
          Lars Hofhansl added a comment - Need to rebase the patch again. Grrr.
          Hide
          Lars Hofhansl added a comment -

          I think I ran into the PB 2.5 vs 2.4.1 issue.

          Show
          Lars Hofhansl added a comment - I think I ran into the PB 2.5 vs 2.4.1 issue.
          Hide
          Lars Hofhansl added a comment -

          Addressed Anoop's comment.
          Also added a test (which turned out to be simple, only the optionallogflushinterval needed to increased in order to not interfere)

          Show
          Lars Hofhansl added a comment - Addressed Anoop's comment. Also added a test (which turned out to be simple, only the optionallogflushinterval needed to increased in order to not interfere)
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12578283/7801-0.96-v10.txt
          against trunk revision .

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

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

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

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

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

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

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

          -1 lineLengths. The patch introduces lines longer than 100

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

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

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/5275//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5275//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5275//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5275//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5275//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5275//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5275//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5275//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5275//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/5275//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/12578283/7801-0.96-v10.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 159 new or modified tests. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces lines longer than 100 +1 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.TestFullLogReconstruction Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/5275//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5275//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5275//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5275//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5275//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5275//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5275//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5275//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/5275//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/5275//console This message is automatically generated.
          Hide
          Lars Hofhansl added a comment -

          Enis and Anoop +1'd already (and I have only clarified the code and added a test since then).
          If there are no objections I will commit this tomorrow.

          I would also like to have this client side API in 0.94 (but without the rest of the intrusive changes). Thinking about how to do that is backward and binary compatible way now.

          Show
          Lars Hofhansl added a comment - Enis and Anoop +1'd already (and I have only clarified the code and added a test since then). If there are no objections I will commit this tomorrow. I would also like to have this client side API in 0.94 (but without the rest of the intrusive changes). Thinking about how to do that is backward and binary compatible way now.
          Hide
          Lars Hofhansl added a comment -

          New API for 0.94.
          Caveats:

          • In 0.94 we still have HRegion.put(Put), which does not honor deferred sync, I did not fix that.
          • Because of that checkAndPut does not honor deferred sync either.

          These are existing problems.
          This should be wire and binary compatible. Please have a close look.

          Show
          Lars Hofhansl added a comment - New API for 0.94. Caveats: In 0.94 we still have HRegion.put(Put), which does not honor deferred sync, I did not fix that. Because of that checkAndPut does not honor deferred sync either. These are existing problems. This should be wire and binary compatible. Please have a close look.
          Hide
          Lars Hofhansl added a comment -

          Committed to 0.95 and 0.98. Thanks for the reviews and the patience with me.

          Show
          Lars Hofhansl added a comment - Committed to 0.95 and 0.98. Thanks for the reviews and the patience with me.
          Hide
          Lars Hofhansl added a comment -

          Better Javadocs on Mutation (explaining the interaction of writeToWAL and durability). set/getWriteToWAL are now deprecated - they will be gone in 0.96.

          Show
          Lars Hofhansl added a comment - Better Javadocs on Mutation (explaining the interaction of writeToWAL and durability). set/getWriteToWAL are now deprecated - they will be gone in 0.96.
          Hide
          Lars Hofhansl added a comment -

          Please have a look at the 0.94 patch (stack, Ted Yu, Enis Soztutar, Anoop Sam John).
          I would like to commit this to 0.94.7... If you can't get to it or have concerns that's fine too, I'll move it to 0.94.8, or remove from 0.94.

          Show
          Lars Hofhansl added a comment - Please have a look at the 0.94 patch ( stack , Ted Yu , Enis Soztutar , Anoop Sam John ). I would like to commit this to 0.94.7... If you can't get to it or have concerns that's fine too, I'll move it to 0.94.8, or remove from 0.94.
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK #4058 (See https://builds.apache.org/job/HBase-TRUNK/4058/)
          HBASE-7801 Allow a deferred sync option per Mutation. (Revision 1467357)

          Result = SUCCESS
          larsh :
          Files :

          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Durability.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTablePool.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Put.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
          • /hbase/trunk/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java
          • /hbase/trunk/hbase-protocol/src/main/protobuf/Client.proto
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/ConstraintProcessor.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseRegionObserver.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/MultiTableOutputFormat.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/BaseRowProcessor.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MultiRowMutationProcessor.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RowProcessor.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/rest/client/RemoteHTable.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/thrift2/ThriftUtilities.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/util/Merge.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestCase.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/TimestampTestBase.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/catalog/TestMetaMigrationConvertingToPB.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestCloneSnapshotFromClient.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMultipleTimestamps.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRestoreSnapshotFromClient.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestTimestampsFilter.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/SimpleRegionObserver.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestAggregateProtocol.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestBigDecimalColumnInterpreter.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverBypass.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverStacking.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionServerCoprocessorExceptionWithAbort.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionServerCoprocessorExceptionWithRemove.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestColumnPrefixFilter.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestColumnRangeFilter.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilter.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestMultipleColumnPrefixFilter.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestChangingEncoding.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TsvImporterCustomTestMapper.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterTransitions.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestZKBasedOpenCloseRegion.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/protobuf/TestProtobufUtil.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/OOMERegionServer.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestAtomicOperation.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestColumnSeeking.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestGetClosestAtOrBefore.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestKeepDeletes.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMinVersions.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMultiColumnScanner.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMergeTransaction.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestResettingCounters.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanner.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSeekOptimizations.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestWideScanner.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestDurability.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestScannerResource.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestScannersWithFilters.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestTableResource.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestRestoreFlushSnapshotFromClient.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestCoprocessorScanPolicy.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestMergeTable.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK #4058 (See https://builds.apache.org/job/HBase-TRUNK/4058/ ) HBASE-7801 Allow a deferred sync option per Mutation. (Revision 1467357) Result = SUCCESS larsh : Files : /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Durability.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTablePool.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Put.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java /hbase/trunk/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java /hbase/trunk/hbase-protocol/src/main/protobuf/Client.proto /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/ConstraintProcessor.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseRegionObserver.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/MultiTableOutputFormat.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/BaseRowProcessor.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MultiRowMutationProcessor.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RowProcessor.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/rest/client/RemoteHTable.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/thrift2/ThriftUtilities.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/util/Merge.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestCase.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/TimestampTestBase.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/catalog/TestMetaMigrationConvertingToPB.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestCloneSnapshotFromClient.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMultipleTimestamps.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRestoreSnapshotFromClient.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestTimestampsFilter.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/SimpleRegionObserver.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestAggregateProtocol.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestBigDecimalColumnInterpreter.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverBypass.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverStacking.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionServerCoprocessorExceptionWithAbort.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionServerCoprocessorExceptionWithRemove.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestColumnPrefixFilter.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestColumnRangeFilter.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilter.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestMultipleColumnPrefixFilter.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestChangingEncoding.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TsvImporterCustomTestMapper.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterTransitions.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestZKBasedOpenCloseRegion.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/protobuf/TestProtobufUtil.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/OOMERegionServer.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestAtomicOperation.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestColumnSeeking.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestGetClosestAtOrBefore.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestKeepDeletes.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMinVersions.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMultiColumnScanner.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMergeTransaction.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestResettingCounters.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanner.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSeekOptimizations.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestWideScanner.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestDurability.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestScannerResource.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestScannersWithFilters.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestTableResource.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestRestoreFlushSnapshotFromClient.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestCoprocessorScanPolicy.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestMergeTable.java
          Hide
          Ted Yu added a comment -

          In

          +  public static Durability valueOf(int ordinal) {
          
          +      default: return null;
          

          Should exception be raised above ? Otherwise we would see NPE somewhere else.

          + * Note that the items must be sorted in order of increasing durability
          

          So the durability of SKIP_WAL is higher than that of USE_DEFAULT ?

          +          Durability tmpDur = m.getDurability(); 
          +          if (tmpDur.ordinal() > durability.ordinal()) {
          +            durability = tmpDur;
          +          }
                     if (m.getWriteToWAL()) {
          

          Should the m.getWriteToWAL() call be replaced with m.getDurability() ?

          +      case SYNC_WAL:
          +      case FSYNC_WAL:
          +        // sync the WAL edit (SYNC and FSYNC treated the same for now)
          +        this.log.sync(txid);
          +        break;
          

          Minor: it would be easier to maintain if the above two conditions are separated, with same this.log.sync(txid) call.

          Show
          Ted Yu added a comment - In + public static Durability valueOf( int ordinal) { + default : return null ; Should exception be raised above ? Otherwise we would see NPE somewhere else. + * Note that the items must be sorted in order of increasing durability So the durability of SKIP_WAL is higher than that of USE_DEFAULT ? + Durability tmpDur = m.getDurability(); + if (tmpDur.ordinal() > durability.ordinal()) { + durability = tmpDur; + } if (m.getWriteToWAL()) { Should the m.getWriteToWAL() call be replaced with m.getDurability() ? + case SYNC_WAL: + case FSYNC_WAL: + // sync the WAL edit (SYNC and FSYNC treated the same for now) + this .log.sync(txid); + break ; Minor: it would be easier to maintain if the above two conditions are separated, with same this.log.sync(txid) call.
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #495 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/495/)
          HBASE-7801 Allow a deferred sync option per Mutation. (Revision 1467357)

          Result = FAILURE
          larsh :
          Files :

          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Durability.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTablePool.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Put.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
          • /hbase/trunk/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java
          • /hbase/trunk/hbase-protocol/src/main/protobuf/Client.proto
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/ConstraintProcessor.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseRegionObserver.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/MultiTableOutputFormat.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/BaseRowProcessor.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MultiRowMutationProcessor.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RowProcessor.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/rest/client/RemoteHTable.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/thrift2/ThriftUtilities.java
          • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/util/Merge.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestCase.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/TimestampTestBase.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/catalog/TestMetaMigrationConvertingToPB.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestCloneSnapshotFromClient.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMultipleTimestamps.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRestoreSnapshotFromClient.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestTimestampsFilter.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/SimpleRegionObserver.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestAggregateProtocol.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestBigDecimalColumnInterpreter.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverBypass.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverStacking.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionServerCoprocessorExceptionWithAbort.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionServerCoprocessorExceptionWithRemove.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestColumnPrefixFilter.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestColumnRangeFilter.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilter.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestMultipleColumnPrefixFilter.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestChangingEncoding.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TsvImporterCustomTestMapper.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterTransitions.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestZKBasedOpenCloseRegion.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/protobuf/TestProtobufUtil.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/OOMERegionServer.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestAtomicOperation.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestColumnSeeking.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestGetClosestAtOrBefore.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestKeepDeletes.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMinVersions.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMultiColumnScanner.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMergeTransaction.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestResettingCounters.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanner.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSeekOptimizations.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestWideScanner.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestDurability.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestScannerResource.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestScannersWithFilters.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestTableResource.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestRestoreFlushSnapshotFromClient.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestCoprocessorScanPolicy.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
          • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestMergeTable.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #495 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/495/ ) HBASE-7801 Allow a deferred sync option per Mutation. (Revision 1467357) Result = FAILURE larsh : Files : /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Durability.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTablePool.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Put.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java /hbase/trunk/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java /hbase/trunk/hbase-protocol/src/main/protobuf/Client.proto /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/ConstraintProcessor.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseRegionObserver.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/MultiTableOutputFormat.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/BaseRowProcessor.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MultiRowMutationProcessor.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RowProcessor.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/rest/client/RemoteHTable.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/thrift2/ThriftUtilities.java /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/util/Merge.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestCase.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/TimestampTestBase.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/catalog/TestMetaMigrationConvertingToPB.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestCloneSnapshotFromClient.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMultipleTimestamps.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRestoreSnapshotFromClient.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestTimestampsFilter.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/SimpleRegionObserver.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestAggregateProtocol.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestBigDecimalColumnInterpreter.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverBypass.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverStacking.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionServerCoprocessorExceptionWithAbort.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionServerCoprocessorExceptionWithRemove.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestColumnPrefixFilter.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestColumnRangeFilter.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilter.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestMultipleColumnPrefixFilter.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestChangingEncoding.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TsvImporterCustomTestMapper.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterTransitions.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestZKBasedOpenCloseRegion.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/protobuf/TestProtobufUtil.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/OOMERegionServer.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestAtomicOperation.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestColumnSeeking.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestGetClosestAtOrBefore.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestKeepDeletes.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMinVersions.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMultiColumnScanner.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMergeTransaction.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestResettingCounters.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanner.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSeekOptimizations.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestWideScanner.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestDurability.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestScannerResource.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestScannersWithFilters.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestTableResource.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestRestoreFlushSnapshotFromClient.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestCoprocessorScanPolicy.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestMergeTable.java
          Hide
          Ted Yu added a comment -

          I ran 0.94 test suite with patch and all clear.

          +      case ASYNC_WAL:
          +        // defer the sync, unless we globally can't
          +        if (this.deferredLogSyncDisabled) {
          

          I think we should add to javadoc for ASYNC_WAL that table descriptor setting is not considered.

          Show
          Ted Yu added a comment - I ran 0.94 test suite with patch and all clear. + case ASYNC_WAL: + // defer the sync, unless we globally can't + if ( this .deferredLogSyncDisabled) { I think we should add to javadoc for ASYNC_WAL that table descriptor setting is not considered.
          Hide
          Lars Hofhansl added a comment -

          Ted Yu Thanks for looking.

          Should exception be raised above ? Otherwise we would see NPE somewhere else.

          Yep... I'll change that. What Exception would be best here?

          So the durability of SKIP_WAL is higher than that of USE_DEFAULT ?

          It also says that USE_DEFAULT needs to be the first one, I think the wording is fine, unless you think it should be clarified.

          Should the m.getWriteToWAL() call be replaced with m.getDurability() ?

          Could use tmpDur here. My intend was to keep the patch as small as possible. I'll change that.

          Minor: it would be easier to maintain if the above two conditions are separated, with same this.log.sync(txid) call.

          Personally, I find it more readable this way. It documents clearly in the code that these two are aliases.

          Will post a new patch later today.

          Show
          Lars Hofhansl added a comment - Ted Yu Thanks for looking. Should exception be raised above ? Otherwise we would see NPE somewhere else. Yep... I'll change that. What Exception would be best here? So the durability of SKIP_WAL is higher than that of USE_DEFAULT ? It also says that USE_DEFAULT needs to be the first one, I think the wording is fine, unless you think it should be clarified. Should the m.getWriteToWAL() call be replaced with m.getDurability() ? Could use tmpDur here. My intend was to keep the patch as small as possible. I'll change that. Minor: it would be easier to maintain if the above two conditions are separated, with same this.log.sync(txid) call. Personally, I find it more readable this way. It documents clearly in the code that these two are aliases. Will post a new patch later today.
          Hide
          Ted Yu added a comment -

          What Exception would be best here?

          How about IllegalArgumentException ?

          Show
          Ted Yu added a comment - What Exception would be best here? How about IllegalArgumentException ?
          Hide
          Jeffrey Zhong added a comment -

          Lars Hofhansl Today, I happened to work on Mutation.java and found that Mutation#MUTATION_OVERHEAD is off. The reason that TestHeapSize#testMutations can pass is due to we still leave old

                // writeToWAL
                Bytes.SIZEOF_BOOLEAN
          

          there. The one byte later is round up to 8 bytes which is matching the newly added field durability which is a reference and has 8 bytes in len. Below are detailed dump for the Mutation:

          013-04-12 15:33:40,902 DEBUG [main] util.ClassSize(246): 0 row class [B
          2013-04-12 15:33:40,903 DEBUG [main] util.ClassSize(246): 1 ts long
          2013-04-12 15:33:40,903 DEBUG [main] util.ClassSize(246): 2 durability class org.apache.hadoop.hbase.client.Durability
          2013-04-12 15:33:40,903 DEBUG [main] util.ClassSize(246): 3 familyMap interface java.util.NavigableMap
          2013-04-12 15:33:40,903 DEBUG [main] util.ClassSize(246): 4 attributes interface java.util.Map
          2013-04-12 15:33:40,903 DEBUG [main] util.ClassSize(273): Primitives=8, arrays=1, references(includes 2 for object overhead)=6, refSize 8, size=80, prealign_size=80
          

          I attached an addendum for your references.

          Thanks,
          -Jeffrey

          Show
          Jeffrey Zhong added a comment - Lars Hofhansl Today, I happened to work on Mutation.java and found that Mutation#MUTATION_OVERHEAD is off. The reason that TestHeapSize#testMutations can pass is due to we still leave old // writeToWAL Bytes.SIZEOF_BOOLEAN there. The one byte later is round up to 8 bytes which is matching the newly added field durability which is a reference and has 8 bytes in len. Below are detailed dump for the Mutation: 013-04-12 15:33:40,902 DEBUG [main] util.ClassSize(246): 0 row class [B 2013-04-12 15:33:40,903 DEBUG [main] util.ClassSize(246): 1 ts long 2013-04-12 15:33:40,903 DEBUG [main] util.ClassSize(246): 2 durability class org.apache.hadoop.hbase.client.Durability 2013-04-12 15:33:40,903 DEBUG [main] util.ClassSize(246): 3 familyMap interface java.util.NavigableMap 2013-04-12 15:33:40,903 DEBUG [main] util.ClassSize(246): 4 attributes interface java.util.Map 2013-04-12 15:33:40,903 DEBUG [main] util.ClassSize(273): Primitives=8, arrays=1, references(includes 2 for object overhead)=6, refSize 8, size=80, prealign_size=80 I attached an addendum for your references. Thanks, -Jeffrey
          Hide
          Lars Hofhansl added a comment -

          Oops. Thanks Jeffrey, will commit addendum ASAP.

          Show
          Lars Hofhansl added a comment - Oops. Thanks Jeffrey, will commit addendum ASAP.
          Hide
          Lars Hofhansl added a comment -

          Committed addendum. Thanks again Jeffrey.

          Show
          Lars Hofhansl added a comment - Committed addendum. Thanks again Jeffrey.
          Hide
          Hudson added a comment -

          Integrated in hbase-0.95-on-hadoop2 #67 (See https://builds.apache.org/job/hbase-0.95-on-hadoop2/67/)
          HBASE-7801 Addendum - fix Mutation size (Revision 1467521)
          HBASE-7801 Allow a deferred sync option per Mutation. (Revision 1467356)

          Result = FAILURE
          larsh :
          Files :

          • /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java

          larsh :
          Files :

          • /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java
          • /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Durability.java
          • /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java
          • /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java
          • /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTablePool.java
          • /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java
          • /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Put.java
          • /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
          • /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
          • /hbase/branches/0.95/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java
          • /hbase/branches/0.95/hbase-protocol/src/main/protobuf/Client.proto
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/ConstraintProcessor.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseRegionObserver.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/MultiTableOutputFormat.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/BaseRowProcessor.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MultiRowMutationProcessor.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RowProcessor.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/rest/client/RemoteHTable.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/thrift2/ThriftUtilities.java
          • /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/util/Merge.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestCase.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/TimestampTestBase.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/catalog/TestMetaMigrationConvertingToPB.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestCloneSnapshotFromClient.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMultipleTimestamps.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRestoreSnapshotFromClient.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestTimestampsFilter.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/SimpleRegionObserver.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestAggregateProtocol.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestBigDecimalColumnInterpreter.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverBypass.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverStacking.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionServerCoprocessorExceptionWithAbort.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionServerCoprocessorExceptionWithRemove.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestColumnPrefixFilter.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestColumnRangeFilter.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilter.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestMultipleColumnPrefixFilter.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestChangingEncoding.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TsvImporterCustomTestMapper.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterTransitions.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestZKBasedOpenCloseRegion.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/protobuf/TestProtobufUtil.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/OOMERegionServer.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestAtomicOperation.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestColumnSeeking.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestGetClosestAtOrBefore.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestKeepDeletes.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMinVersions.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMultiColumnScanner.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMergeTransaction.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestResettingCounters.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanner.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSeekOptimizations.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestWideScanner.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestDurability.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestScannerResource.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestScannersWithFilters.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestTableResource.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestRestoreFlushSnapshotFromClient.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestCoprocessorScanPolicy.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
          • /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestMergeTable.java
          Show
          Hudson added a comment - Integrated in hbase-0.95-on-hadoop2 #67 (See https://builds.apache.org/job/hbase-0.95-on-hadoop2/67/ ) HBASE-7801 Addendum - fix Mutation size (Revision 1467521) HBASE-7801 Allow a deferred sync option per Mutation. (Revision 1467356) Result = FAILURE larsh : Files : /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java larsh : Files : /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Durability.java /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTableInterface.java /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTablePool.java /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Put.java /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java /hbase/branches/0.95/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java /hbase/branches/0.95/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java /hbase/branches/0.95/hbase-protocol/src/main/protobuf/Client.proto /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/constraint/ConstraintProcessor.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/BaseRegionObserver.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/MultiTableOutputFormat.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/BaseRowProcessor.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MultiRowMutationProcessor.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RowProcessor.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/rest/client/RemoteHTable.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/thrift2/ThriftUtilities.java /hbase/branches/0.95/hbase-server/src/main/java/org/apache/hadoop/hbase/util/Merge.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestCase.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/TimestampTestBase.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/catalog/TestMetaMigrationConvertingToPB.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestCloneSnapshotFromClient.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMultipleTimestamps.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRestoreSnapshotFromClient.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestTimestampsFilter.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/SimpleRegionObserver.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestAggregateProtocol.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestBigDecimalColumnInterpreter.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverBypass.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverInterface.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverStacking.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionServerCoprocessorExceptionWithAbort.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionServerCoprocessorExceptionWithRemove.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestColumnPrefixFilter.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestColumnRangeFilter.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilter.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestMultipleColumnPrefixFilter.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/io/encoding/TestChangingEncoding.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TestTimeRangeMapRed.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/mapreduce/TsvImporterCustomTestMapper.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterTransitions.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestZKBasedOpenCloseRegion.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/protobuf/TestProtobufUtil.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/OOMERegionServer.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestAtomicOperation.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestBlocksRead.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestColumnSeeking.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestGetClosestAtOrBefore.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestKeepDeletes.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMinVersions.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMultiColumnScanner.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMergeTransaction.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestResettingCounters.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanner.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSeekOptimizations.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestWideScanner.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestDurability.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestMasterReplication.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestScannerResource.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestScannersWithFilters.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/rest/TestTableResource.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestRestoreFlushSnapshotFromClient.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestCoprocessorScanPolicy.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java /hbase/branches/0.95/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestMergeTable.java
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #496 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/496/)
          HBASE-7801 Addendum - fix Mutation size (Revision 1467522)

          Result = FAILURE
          larsh :
          Files :

          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #496 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/496/ ) HBASE-7801 Addendum - fix Mutation size (Revision 1467522) Result = FAILURE larsh : Files : /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java
          Hide
          Lars Hofhansl added a comment -

          I think we should add to javadoc for ASYNC_WAL that table descriptor setting is not considered.

          I think you misunderstand the setting. That's the case that Anoop needs where the deferred log flusher is disabled completely. In the case we also do not honor the table's settings, but flush everything synchronously, since otherwise it will never get flushed. In that case the RegionServer will log a warning on startup.

          Show
          Lars Hofhansl added a comment - I think we should add to javadoc for ASYNC_WAL that table descriptor setting is not considered. I think you misunderstand the setting. That's the case that Anoop needs where the deferred log flusher is disabled completely. In the case we also do not honor the table's settings, but flush everything synchronously, since otherwise it will never get flushed. In that case the RegionServer will log a warning on startup.
          Hide
          Lars Hofhansl added a comment -

          Patch address (some of) Ted's comments.
          I think this should be good to go... If we're OK with putting this into 0.94.

          Show
          Lars Hofhansl added a comment - Patch address (some of) Ted's comments. I think this should be good to go... If we're OK with putting this into 0.94.
          Hide
          Lars Hofhansl added a comment -

          Here's a list of changes I deliberately avoided in the 0.94 patch to keep it small and low risk:

          • Changing the coprocessor signature. The pre/post hooks still get only a boolean indicating whether the entry written to the WAL or not
          • Make HRegion.interalPut honor deferred sync. Currently internalPut does not honor this at all (not even when set on the Table); it is use for checkAndPut. This patch does not fix that.
          • cleanup the Increment/Appand method signatures. These receive an Append (or Increment) and also a writeToWAL boolean, even though the Append/Increment have that boolean already.

          If we wanted to change any of these, it should be a separate jira.

          Show
          Lars Hofhansl added a comment - Here's a list of changes I deliberately avoided in the 0.94 patch to keep it small and low risk: Changing the coprocessor signature. The pre/post hooks still get only a boolean indicating whether the entry written to the WAL or not Make HRegion.interalPut honor deferred sync. Currently internalPut does not honor this at all (not even when set on the Table); it is use for checkAndPut. This patch does not fix that. cleanup the Increment/Appand method signatures. These receive an Append (or Increment) and also a writeToWAL boolean, even though the Append/Increment have that boolean already. If we wanted to change any of these, it should be a separate jira.
          Hide
          Hudson added a comment -

          Integrated in HBase-TRUNK #4060 (See https://builds.apache.org/job/HBase-TRUNK/4060/)
          HBASE-7801 Addendum - fix Mutation size (Revision 1467522)

          Result = SUCCESS
          larsh :
          Files :

          • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java
          Show
          Hudson added a comment - Integrated in HBase-TRUNK #4060 (See https://builds.apache.org/job/HBase-TRUNK/4060/ ) HBASE-7801 Addendum - fix Mutation size (Revision 1467522) Result = SUCCESS larsh : Files : /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java
          Hide
          Ted Yu added a comment -

          @Lars:
          Can you update release notes for this JIRA ?

          Thanks

          Show
          Ted Yu added a comment - @Lars: Can you update release notes for this JIRA ? Thanks
          Hide
          ramkrishna.s.vasudevan added a comment -

          Patch looks good. The changes that you have not done for 0.94 makes sense.

          Show
          ramkrishna.s.vasudevan added a comment - Patch looks good. The changes that you have not done for 0.94 makes sense.
          Hide
          Lars Hofhansl added a comment -

          Going to commit to 0.94 today or tomorrow, unless I hear objections. Since we had discussions about adding feaures to 0.94, please be vocal.

          At Salesforce we need this to do be able to seed a table with data from another system (and still allow that this is eventually replicated to a replication slave) and then allow further incremental updates with synchronous WAL sync.

          Show
          Lars Hofhansl added a comment - Going to commit to 0.94 today or tomorrow, unless I hear objections. Since we had discussions about adding feaures to 0.94, please be vocal. At Salesforce we need this to do be able to seed a table with data from another system (and still allow that this is eventually replicated to a replication slave) and then allow further incremental updates with synchronous WAL sync.
          Hide
          Ted Yu added a comment -
          +  public Durability getDurability() {
          +    byte[] attr = getAttribute(DURABILITY_ID_ATTR);
          +    if (attr == null) {
          +      return writeToWAL ? Durability.USE_DEFAULT : Durability.SKIP_WAL;
          +    }
          +    return Durability.valueOf(Bytes.toInt(attr));
          

          Is it possible that "d.id" has been used by certain user code ? Should exception from Durability.valueOf() be caught so that we can check the value of writeToWAL ?

          Show
          Ted Yu added a comment - + public Durability getDurability() { + byte [] attr = getAttribute(DURABILITY_ID_ATTR); + if (attr == null ) { + return writeToWAL ? Durability.USE_DEFAULT : Durability.SKIP_WAL; + } + return Durability.valueOf(Bytes.toInt(attr)); Is it possible that " d.id " has been used by certain user code ? Should exception from Durability.valueOf() be caught so that we can check the value of writeToWAL ?
          Hide
          Lars Hofhansl added a comment -

          We have a silent agreement that attributes starting with _ are for internal use only (same for the cluster id), I agree that that should have been documented somewhere.

          As for catching the exception... Not sure what we should do in this case, other than throwing an Exception. Or you mean you want to catch and ignore it and then check the value of writeToWAL? Could do that too, but it seems a bit overkill. I'd rather know in the client code if there's something wrong the enum.

          Show
          Lars Hofhansl added a comment - We have a silent agreement that attributes starting with _ are for internal use only (same for the cluster id), I agree that that should have been documented somewhere. As for catching the exception... Not sure what we should do in this case, other than throwing an Exception. Or you mean you want to catch and ignore it and then check the value of writeToWAL? Could do that too, but it seems a bit overkill. I'd rather know in the client code if there's something wrong the enum.
          Hide
          Ted Yu added a comment -

          I'd rather know in the client code if there's something wrong the enum.

          doMiniBatchMutation() is called inside HRegion which is in region server. I think we should be a bit protective.

          Show
          Ted Yu added a comment - I'd rather know in the client code if there's something wrong the enum. doMiniBatchMutation() is called inside HRegion which is in region server. I think we should be a bit protective.
          Hide
          Lars Hofhansl added a comment -

          Hmm... So translate to a DoNotRetryIOException at the server?

          Show
          Lars Hofhansl added a comment - Hmm... So translate to a DoNotRetryIOException at the server?
          Hide
          Ted Yu added a comment -

          We can either throw IOException at the server or, fall back to checking the value of writeToWAL.

          Show
          Ted Yu added a comment - We can either throw IOException at the server or, fall back to checking the value of writeToWAL.
          Hide
          Lars DoNotUse added a comment -

          Neither is ideal. Which would you prefer?

          Show
          Lars DoNotUse added a comment - Neither is ideal. Which would you prefer?
          Hide
          Ted Yu added a comment -

          fall back to checking the value of writeToWAL.

          Before the ideal solution is found, I prefer the above.

          Show
          Ted Yu added a comment - fall back to checking the value of writeToWAL. Before the ideal solution is found, I prefer the above.
          Hide
          Lars DoNotUse added a comment -

          Alright will have a new patch soon.

          Show
          Lars DoNotUse added a comment - Alright will have a new patch soon.
          Hide
          Lars Hofhansl added a comment -

          How about this?
          (Both Durability.valueOf(int) and Bytes.toInt(byte[]) throw IllegalArgumentException.

          Show
          Lars Hofhansl added a comment - How about this? (Both Durability.valueOf(int) and Bytes.toInt(byte[]) throw IllegalArgumentException.
          Hide
          Ted Yu added a comment -

          Looks good to me.

          Thanks for the perseverance, Lars.

          Show
          Ted Yu added a comment - Looks good to me. Thanks for the perseverance, Lars.
          Hide
          Lars DoNotUse added a comment -

          Cool. Committed to 0.94 as well.
          Thanks for the reviews!

          Show
          Lars DoNotUse added a comment - Cool. Committed to 0.94 as well. Thanks for the reviews!
          Hide
          Hudson added a comment -

          Integrated in HBase-0.94 #961 (See https://builds.apache.org/job/HBase-0.94/961/)
          HBASE-7801 Allow a deferred sync option per Mutation. (Revision 1468179)

          Result = FAILURE
          larsh :
          Files :

          • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/client/Durability.java
          • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/client/Mutation.java
          • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
          • /hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestDurability.java
          Show
          Hudson added a comment - Integrated in HBase-0.94 #961 (See https://builds.apache.org/job/HBase-0.94/961/ ) HBASE-7801 Allow a deferred sync option per Mutation. (Revision 1468179) Result = FAILURE larsh : Files : /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/client/Durability.java /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/client/Mutation.java /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java /hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestDurability.java
          Hide
          Hudson added a comment -

          Integrated in HBase-0.94-security #137 (See https://builds.apache.org/job/HBase-0.94-security/137/)
          HBASE-7801 Allow a deferred sync option per Mutation. (Revision 1468179)

          Result = SUCCESS
          larsh :
          Files :

          • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/client/Durability.java
          • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/client/Mutation.java
          • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
          • /hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestDurability.java
          Show
          Hudson added a comment - Integrated in HBase-0.94-security #137 (See https://builds.apache.org/job/HBase-0.94-security/137/ ) HBASE-7801 Allow a deferred sync option per Mutation. (Revision 1468179) Result = SUCCESS larsh : Files : /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/client/Durability.java /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/client/Mutation.java /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java /hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestDurability.java

            People

            • Assignee:
              Lars Hofhansl
              Reporter:
              Lars Hofhansl
            • Votes:
              0 Vote for this issue
              Watchers:
              12 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development