Uploaded image for project: 'HBase'
  1. HBase
  2. HBASE-17138

Backport read-path offheap (HBASE-11425) to branch-1

    Details

    • Type: Improvement
    • Status: Open
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: regionserver
    • Labels:
      None

      Description

      From the thread of sharing our experience and performance data of read-path offheap usage in Alibaba search, we could see people are positive to have HBASE-11425 in branch-1, so I'd like to create a JIRA and move the discussion and decision making here.

      Echoing some comments from the mail thread:

      Bryan:
      Is the backported patch available anywhere? If it ends up not getting officially backported to branch-1 due to 2.0 around the corner, some of us who build our own deploy may want to integrate into our builds

      Andrew:
      Yes, please, the patches will be useful to the community even if we decide not to backport into an official 1.x release.

      Enis:
      I don't see any reason why we cannot backport to branch-1.

      Ted:
      Opening a JIRA would be fine. This makes it easier for people to obtain the patch(es)

      Nick:
      From the DISCUSS thread re: EOL of 1.1, it seems we'll continue to
      support 1.x releases for some time... I would guess these will be
      maintained until 2.2 at least. Therefore, offheap patches that have seen
      production exposure seem like a reasonable candidate for backport, perhaps in a 1.4 or 1.5 release timeframe.

      Anoop:
      Because of some compatibility issues, we decide that this will be done in 2.0 only.. Ya as Andy said, it would be great to share the 1.x backported patches.

      The following is all the jira ids we have back ported:
      HBASE-10930 Change Filters and GetClosestRowBeforeTracker to work with Cells (Ram)
      HBASE-13373 Squash HFileReaderV3 together with HFileReaderV2 and AbstractHFileReader; ditto for Scanners and BlockReader, etc.
      HBASE-13429 Remove deprecated seek/reseek methods from HFileScanner.
      HBASE-13450 - Purge RawBytescomparator from the writers and readers for HBASE-10800 (Ram)
      HBASE-13501 - Deprecate/Remove getComparator() in HRegionInfo.
      HBASE-12048 Remove deprecated APIs from Filter.
      HBASE-10800 - Use CellComparator instead of KVComparator (Ram)
      HBASE-13679 Change ColumnTracker and SQM to deal with Cell instead of byte[], int, int.
      HBASE-13642 Deprecate RegionObserver#postScannerFilterRow CP hook with byte[],int,int args in favor of taking Cell arg.
      HBASE-13641 Deperecate Filter#filterRowKey(byte[] buffer, int offset, int length) in favor of filterRowKey(Cell firstRowCell).
      HBASE-13827 Delayed scanner close in KeyValueHeap and StoreScanner.
      HBASE-13871 Change RegionScannerImpl to deal with Cell instead of byte[], int, int.

      HBASE-11911 Break up tests into more fine grained categories (Alex Newman)
      HBASE-12059 Create hbase-annotations module
      HBASE-12106 Move test annotations to test artifact (Enis Soztutar)

      HBASE-13916 Create MultiByteBuffer an aggregation of ByteBuffers.
      HBASE-15679 Assertion on wrong variable in TestReplicationThrottler#testThrottling
      HBASE-13931 Move Unsafe based operations to UnsafeAccess.
      HBASE-12345 Unsafe based ByteBuffer Comparator.
      HBASE-13998 Remove CellComparator#compareRows(byte[] left, int loffset, int llength, byte[] right, int roffset, int rlength).
      HBASE-13998 Remove CellComparator#compareRows()- Addendum to fix javadoc warn
      HBASE-13579 Avoid isCellTTLExpired() for NO-TAG cases (partially backport this patch)
      HBASE-13448 New Cell implementation with cached component offsets/lengths.
      HBASE-13387 Add ByteBufferedCell an extension to Cell.
      HBASE-13387 Add ByteBufferedCell an extension to Cell - addendum.
      HBASE-12650 Move ServerName to hbase-common module (partially backport this patch)
      HBASE-12296 Filters should work with ByteBufferedCell.
      HBASE-14120 ByteBufferUtils#compareTo small optimization.
      HBASE-13510 - Purge ByteBloomFilter (Ram)
      HBASE-13451 - Make the HFileBlockIndex blockKeys to Cells so that it could be easy to use in the CellComparators (Ram)
      HBASE-13614 - Avoid temp KeyOnlyKeyValue temp objects creations in read hot path (Ram)
      HBASE-13939 - Make HFileReaderImpl.getFirstKeyInBlock() to return a Cell (Ram)
      HBASE-13307 Making methods under ScannerV2#next inlineable, faster
      HBASE-14020 Unsafe based optimized write in ByteBufferOutputStream.
      HBASE-13977 - Convert getKey and related APIs to Cell (Ram)
      HBASE-11927 Use Native Hadoop Library for HFile checksum. (Apekshit)
      HBASE-12213 HFileBlock backed by Array of ByteBuffers (Ram)
      HBASE-12084 Remove deprecated APIs from Result.
      HBASE-12084 Remove deprecated APIs from Result - shell addendum
      HBASE-13754 Allow non KeyValue Cell types also to oswrite.
      HBASE-14047 - Cleanup deprecated APIs from Cell class (Ashish Singhi)
      HBASE-13817 ByteBufferOuputStream - add writeInt support.
      HBASE-12374 Change DBEs to work with new BB based cell.
      HBASE-14116 Change ByteBuff.getXXXStrictlyForward to relative position based reads
      HBASE-14073 TestRemoteTable.testDelete failed in the latest trunk code.(Jingcheng)
      HBASE-13926 Close the scanner only after Call#setResponse.
      HBASE-12295 Prevent block eviction under us if reads are in progress from the BBs (Ram)
      HBASE-12295 - Addendum for multiGets to add the call back(Ram)
      HBASE-14001 Optimize write(OutputStream out, boolean withTags) for SizeCachedNoTagsKeyValue.
      HBASE-14063 Use BufferBackedCell in read path after HBASE-12213 and HBASE-12295 (ram)
      HBASE-14202 Reduce garbage we create.
      HBASE-14099 StoreFile.passesKeyRangeFilter need not create Cells from the Scan's start and stop Row (Ram)
      HBASE-14144 - Bloomfilter path to work with Byte buffered cells (Ram)
      HBASE-14395 Short circuit last byte check in CellUtil#matchingXXX methods for ByteBufferedCells.
      HBASE-12298 Support BB usage in PrefixTree (Ram)
      HBASE-14398 - Create the fake keys required in the scan path to avoid copy to byte[] (Ram)
      HBASE-14480 Small optimization in SingleByteBuff.
      HBASE-14590 Shorten ByteBufferedCell#getXXXPositionInByteBuffer method name (Anoop Sam John)
      HBASE-14636 Clear HFileScannerImpl#prevBlocks in between Compaction flow.
      HBASE-14832 Ensure write paths work with ByteBufferedCells in case of compaction (Ram)
      HBASE-14524 Short-circuit comparison of rows in CellComparator. (Lars Francke)
      HBASE-14188 - Read path optimizations after HBASE-11425 profiling (Ram)
      HBASE-14188- Read path optimizations after HBASE-11425 profiling-Addendum(Ram)
      HBASE-12593 Tags to work with ByteBuffer.
      HBASE-15077 Support OffheapKV write in compaction with out copying data on heap.
      HBASE-14660 AssertionError found when using offheap BucketCache with assertion enabled (ram)
      HBASE-15076 Add getScanner(Scan scan, List<KeyValueScanner> additionalScanners) API into Region interface (Anoop Sam John
      HBASE-15735(also particial HBASE-13893) Tightening of the CP contract.
      HBASE-15785 Unnecessary lock in ByteBufferArray.
      HBASE-15760 TestBlockEvictionFromClient#testParallelGetsAndScanWithWrappedRegionScanner fails in master branch (Ram)
      HBASE-15379 Fake cells created in read path not implementing SettableSequenceId
      HBASE-16609 Fake cells EmptyByteBufferedCell created in read path not implementing SettableSequenceId
      HBASE-14940 Make our unsafe based ops more safe
      HBASE-15063 Bug in MultiByteBuf#toBytes. (deepankar)
      HBASE-15064 BufferUnderflowException after last Cell fetched from an HFile Block served from L2 offheap cache.
      HBASE-15064 BufferUnderflowException after last Cell fetched from an HFile Block served from L2 offheap cache - Addendum.
      HBASE-15253 Small bug in CellUtil.matchingRow(Cell, byte[]) (Ram)
      HBASE-16704 Scan will be broken while working with DBE and KeyValueCodecWithTags

      the above jira ids are listed in chronological order I have backport,and there are also some patch i dont list, such as:
      1. keep Cell api compatible with our existing code.
      2. hfile format related compatibility issues.
      3. client compatibility issue

        Activity

        Hide
        carp84 Yu Li added a comment -

        So here I'd like we make a consensus on whether to 1) backport HBASE-11425 to branch-1 or 2) upload our patches for our customized 1.1.2 here. The former is more official and means community will maintain the backported feature, but as Anoop Sam John mentioned there'll be some more efforts to keep compatibility so more than only HBASE-11425, and the latter is easier for us but harder for others to use due to code-base diverge.

        Maybe some kind of voting in need here? +1 for backport and -1 for only uploading existing patches? Let me know your thoughts please, thanks. Anoop Sam John stack ramkrishna.s.vasudevan Andrew Purtell Enis Soztutar Nick Dimiduk Duo Zhang Sean Busbey Mikhail Antonov Matteo Bertozzi

        And comments from others are also welcome. Thanks.

        Show
        carp84 Yu Li added a comment - So here I'd like we make a consensus on whether to 1) backport HBASE-11425 to branch-1 or 2) upload our patches for our customized 1.1.2 here. The former is more official and means community will maintain the backported feature, but as Anoop Sam John mentioned there'll be some more efforts to keep compatibility so more than only HBASE-11425 , and the latter is easier for us but harder for others to use due to code-base diverge. Maybe some kind of voting in need here? +1 for backport and -1 for only uploading existing patches? Let me know your thoughts please, thanks. Anoop Sam John stack ramkrishna.s.vasudevan Andrew Purtell Enis Soztutar Nick Dimiduk Duo Zhang Sean Busbey Mikhail Antonov Matteo Bertozzi And comments from others are also welcome. Thanks.
        Hide
        anoop.hbase Anoop Sam John added a comment -

        U guys have list of all jiras backported right. There are some building blocks jiras not under HBASE-11425.. Can u pls put all such jira ids here.. I forgot which all were compatibility related changes.. I can also help to check them... So let us do that analysis first and list down all such issues (if any).

        Show
        anoop.hbase Anoop Sam John added a comment - U guys have list of all jiras backported right. There are some building blocks jiras not under HBASE-11425 .. Can u pls put all such jira ids here.. I forgot which all were compatibility related changes.. I can also help to check them... So let us do that analysis first and list down all such issues (if any).
        Hide
        Apache9 Duo Zhang added a comment -

        I'm a +0. I suggest you give a try first to see if it is easy to address the compatibiliy issues. I think the option of the one who decide to do the backport work(you or other people who want this feature on branch-1) is most important.

        Thanks.

        Show
        Apache9 Duo Zhang added a comment - I'm a +0. I suggest you give a try first to see if it is easy to address the compatibiliy issues. I think the option of the one who decide to do the backport work(you or other people who want this feature on branch-1) is most important. Thanks.
        Hide
        carp84 Yu Li added a comment -

        Assigning to Yu Sun who is my workmate and did most of the backporting work, and leaving the questions to him. Sorry that he is on vacation this week (worth a rest after going through 11/11) and let's wait a little while.

        Show
        carp84 Yu Li added a comment - Assigning to Yu Sun who is my workmate and did most of the backporting work, and leaving the questions to him. Sorry that he is on vacation this week (worth a rest after going through 11/11) and let's wait a little while.
        Hide
        ram_krish ramkrishna.s.vasudevan added a comment -

        I think some of the building blocks may be a bigger change and may end up in problems which is not easy to back port. I agree with Duo Zhang here.
        First we see what compatability issues we got and then based on that see if it is easier to back port.
        Even i too don't remember all the compatability issues but I don't think there is anything in terms of USer APIs.

        Show
        ram_krish ramkrishna.s.vasudevan added a comment - I think some of the building blocks may be a bigger change and may end up in problems which is not easy to back port. I agree with Duo Zhang here. First we see what compatability issues we got and then based on that see if it is easier to back port. Even i too don't remember all the compatability issues but I don't think there is anything in terms of USer APIs.
        Hide
        haoran Yu Sun added a comment -

        yes, Anoop Sam John is right, there are some building blocks jira not under HBASE-11425, To resolve the merge conflicts I have totally backport about 77 patches to our customized branch, should I list the all the jira id I have backported as sub-task here for you and ramkrishna.s.vasudevan to check?

        Show
        haoran Yu Sun added a comment - yes, Anoop Sam John is right, there are some building blocks jira not under HBASE-11425 , To resolve the merge conflicts I have totally backport about 77 patches to our customized branch, should I list the all the jira id I have backported as sub-task here for you and ramkrishna.s.vasudevan to check?
        Hide
        anoop.hbase Anoop Sam John added a comment -

        Yes pls list all the jira ids.. Will have a look at them and can help in analyzing the compatibility issues if any.

        Show
        anoop.hbase Anoop Sam John added a comment - Yes pls list all the jira ids.. Will have a look at them and can help in analyzing the compatibility issues if any.
        Hide
        ram_krish ramkrishna.s.vasudevan added a comment -

        Pls prepare the list. We can figure it out I think from that list if there are some critical changes related to compatibility.

        Show
        ram_krish ramkrishna.s.vasudevan added a comment - Pls prepare the list. We can figure it out I think from that list if there are some critical changes related to compatibility.
        Hide
        haoran Yu Sun added a comment -

        done, you and Anoop Sam John please see the Description

        Show
        haoran Yu Sun added a comment - done, you and Anoop Sam John please see the Description
        Hide
        ram_krish ramkrishna.s.vasudevan added a comment -

        Checking this. Will revert back tomorrow.

        Show
        ram_krish ramkrishna.s.vasudevan added a comment - Checking this. Will revert back tomorrow.
        Hide
        ram_krish ramkrishna.s.vasudevan added a comment -

        I think

        https://issues.apache.org/jira/browse/HBASE-16189 - Rolling upgrade issue
        https://issues.apache.org/jira/browse/HBASE-12084 - remove deprecated APIs from Result - incompatible change
        https://issues.apache.org/jira/browse/HBASE-13641 - will break the feature by doing copy in filters
        https://issues.apache.org/jira/browse/HBASE-13642 - will break the feature by doing copy in CPs
        https://issues.apache.org/jira/browse/HBASE-12048 - already an incompatible change
        https://issues.apache.org/jira/browse/HBASE-13450 - Purge rawbytescomparator
        https://issues.apache.org/jira/browse/HBASE-10800
        

        The last one is pretty important.
        https://issues.apache.org/jira/browse/HBASE-16189 was already raised.
        Some JIRAs are like we have maintained BC but if the Filters/CP are not upgraded then we don't get the full benefit of the feature.
        Purging RawBytesComparator could have same impact as reported in HBASE-16189? Need to check that.

        Show
        ram_krish ramkrishna.s.vasudevan added a comment - I think https: //issues.apache.org/jira/browse/HBASE-16189 - Rolling upgrade issue https: //issues.apache.org/jira/browse/HBASE-12084 - remove deprecated APIs from Result - incompatible change https: //issues.apache.org/jira/browse/HBASE-13641 - will break the feature by doing copy in filters https: //issues.apache.org/jira/browse/HBASE-13642 - will break the feature by doing copy in CPs https: //issues.apache.org/jira/browse/HBASE-12048 - already an incompatible change https: //issues.apache.org/jira/browse/HBASE-13450 - Purge rawbytescomparator https: //issues.apache.org/jira/browse/HBASE-10800 The last one is pretty important. https://issues.apache.org/jira/browse/HBASE-16189 was already raised. Some JIRAs are like we have maintained BC but if the Filters/CP are not upgraded then we don't get the full benefit of the feature. Purging RawBytesComparator could have same impact as reported in HBASE-16189 ? Need to check that.
        Hide
        anoop.hbase Anoop Sam John added a comment -

        Thanks for the analysis Ram. See my thinking on these
        HBASE-12084 Removed APIs were deprecated since 0.96.. We can keep the old APIs if needed BC. Need to make sure within server, we use only cell based APIs. Client end any way we deal with KVs only. (No off heaping there)
        HBASE-13641 Ya. But not using the new filter method but continue with old one in custom filters will cause perf issue in 2.0 also. We can make sure HBase filters use the new ones not the deprecated one
        HBASE-13642 Same as above. I feel that the usage of the CP methods mentioned here in custom CPs might be very rare.
        HBASE-10800 HBASE-16189 already fix this issue in branch-1
        HBASE-12048 Again deprecated since 0.96 - Can this be removed? Need to see how/whether we can do workaround for BC just like CP way
        HBASE-13450 HBASE-16189 seems handle this also. Should not be a big concern IMO

        Show
        anoop.hbase Anoop Sam John added a comment - Thanks for the analysis Ram. See my thinking on these HBASE-12084 Removed APIs were deprecated since 0.96.. We can keep the old APIs if needed BC. Need to make sure within server, we use only cell based APIs. Client end any way we deal with KVs only. (No off heaping there) HBASE-13641 Ya. But not using the new filter method but continue with old one in custom filters will cause perf issue in 2.0 also. We can make sure HBase filters use the new ones not the deprecated one HBASE-13642 Same as above. I feel that the usage of the CP methods mentioned here in custom CPs might be very rare. HBASE-10800 HBASE-16189 already fix this issue in branch-1 HBASE-12048 Again deprecated since 0.96 - Can this be removed? Need to see how/whether we can do workaround for BC just like CP way HBASE-13450 HBASE-16189 seems handle this also. Should not be a big concern IMO
        Hide
        ram_krish ramkrishna.s.vasudevan added a comment -

        That is true. But I was just thinking on the impact for the users. So unless they do this change then the feature is not useful. How do we ensure that? So who needs this feature will need to upgrade the server?

        1. keep Cell api compatible with our existing code.
        2. hfile format related compatibility issues.
        3. client compatibility issue
        

        You already know these list? Particularly hfile format related issues?

        Show
        ram_krish ramkrishna.s.vasudevan added a comment - That is true. But I was just thinking on the impact for the users. So unless they do this change then the feature is not useful. How do we ensure that? So who needs this feature will need to upgrade the server? 1. keep Cell api compatible with our existing code. 2. hfile format related compatibility issues. 3. client compatibility issue You already know these list? Particularly hfile format related issues?
        Hide
        anoop.hbase Anoop Sam John added a comment -

        Sorry am not getting ur Qs fully.

        So who needs this feature will need to upgrade the server?

        U mean update their server side code extensions ie. Filters/CPs?.. If so - yes

        What is the HFile format issue? Ya the Comparator class name issue is there but that is solved already in branch-1

        What are client compatibility issues? U mean we removed some of the deprecated APIs from Cell. U say keep it as is? I mean Apis like getRow(). Ya that should be ok. After all client side every thing is KVs

        Show
        anoop.hbase Anoop Sam John added a comment - Sorry am not getting ur Qs fully. So who needs this feature will need to upgrade the server? U mean update their server side code extensions ie. Filters/CPs?.. If so - yes What is the HFile format issue? Ya the Comparator class name issue is there but that is solved already in branch-1 What are client compatibility issues? U mean we removed some of the deprecated APIs from Cell. U say keep it as is? I mean Apis like getRow(). Ya that should be ok. After all client side every thing is KVs
        Hide
        haoran Yu Sun added a comment -

        yes, we have three patches to resolve the issue, but not list above.
        1. we still use Cell.getFamily(),Cell.getQualilfier and Cell.getRow() api in our existing code. this issue mainly introduced by HBASE-14047.
        2. we should ensure offheap hfile format and old branch 1.1.2(without HBASE-16189) hfile format keep compatible with each other for fallback purpose. so change the offheap hfile format the same as our branch-1.1.2, and change some code while backport offheap to process this.
        3. introduced by HBASE-12084,HBASE-13641,HBASE-12048.

        so i think we should at least remove HBASE-14047,HBASE-12084,HBASE-13641,HBASE-12048 from backport list.

        Show
        haoran Yu Sun added a comment - yes, we have three patches to resolve the issue, but not list above. 1. we still use Cell.getFamily(),Cell.getQualilfier and Cell.getRow() api in our existing code. this issue mainly introduced by HBASE-14047 . 2. we should ensure offheap hfile format and old branch 1.1.2(without HBASE-16189 ) hfile format keep compatible with each other for fallback purpose. so change the offheap hfile format the same as our branch-1.1.2, and change some code while backport offheap to process this. 3. introduced by HBASE-12084 , HBASE-13641 , HBASE-12048 . so i think we should at least remove HBASE-14047 , HBASE-12084 , HBASE-13641 , HBASE-12048 from backport list.
        Hide
        anoop.hbase Anoop Sam John added a comment -

        Mind attaching the 3 patches here so as to get what code level changes were needed.
        So what is ur feeling abt backport and possible BS issue? I feel some how that we can backport with out any BC issue. Ya it wont be exactly same as 2.0 based patches. As u said some to be avoided and some to changed a bit. still overall possible is what my feeling now after seeing the list. wdyt? cc Ted Yu

        Show
        anoop.hbase Anoop Sam John added a comment - Mind attaching the 3 patches here so as to get what code level changes were needed. So what is ur feeling abt backport and possible BS issue? I feel some how that we can backport with out any BC issue. Ya it wont be exactly same as 2.0 based patches. As u said some to be avoided and some to changed a bit. still overall possible is what my feeling now after seeing the list. wdyt? cc Ted Yu
        Hide
        ram_krish ramkrishna.s.vasudevan added a comment -

        so i think we should at least remove HBASE-14047,HBASE-12084,HBASE-13641,HBASE-12048 from backport list.

        Makes sense.
        I too think we can backport. Some patches needs to be avoided or changed. We can try backporting by creating sub-jiras under this?

        Show
        ram_krish ramkrishna.s.vasudevan added a comment - so i think we should at least remove HBASE-14047 , HBASE-12084 , HBASE-13641 , HBASE-12048 from backport list. Makes sense. I too think we can backport. Some patches needs to be avoided or changed. We can try backporting by creating sub-jiras under this?
        Hide
        haoran Yu Sun added a comment -

        done

        Show
        haoran Yu Sun added a comment - done
        Hide
        haoran Yu Sun added a comment -

        yes, I think so, almost all of patches need to be changed due to merge conflict.so this will need some time if we decide to backport.

        Show
        haoran Yu Sun added a comment - yes, I think so, almost all of patches need to be changed due to merge conflict.so this will need some time if we decide to backport.
        Hide
        carp84 Yu Li added a comment -

        Ok, we get a long list to backport and some new changes to keep compatibility, good start (smile).

        So what's the better way for next move? Fork and merge? Or simply a big patch here for review? No doubt it's time costing to make sure of compatibility and stability no matter which way to take, but worth the efforts.

        Show
        carp84 Yu Li added a comment - Ok, we get a long list to backport and some new changes to keep compatibility, good start (smile). So what's the better way for next move? Fork and merge? Or simply a big patch here for review? No doubt it's time costing to make sure of compatibility and stability no matter which way to take, but worth the efforts.
        Hide
        anoop.hbase Anoop Sam John added a comment - - edited

        With all these list and analysis, lets start a vote in dev@. Then we can decide based on the voting pattern there. Ya a new branch based on branch-1 might be needed with smaller patches getting in and then finally a merge back to branch-1

        Show
        anoop.hbase Anoop Sam John added a comment - - edited With all these list and analysis, lets start a vote in dev@. Then we can decide based on the voting pattern there. Ya a new branch based on branch-1 might be needed with smaller patches getting in and then finally a merge back to branch-1
        Hide
        anoop.hbase Anoop Sam John added a comment -

        Thanks for the patches.. Ya all make sense..
        On Filter, so we have Cell based APIs from 0.96 and deprecated KV based APIs there. But 1.x version contain both. So in branch-1 how is it used now? The core code flow calls cell based new APIs right? And FilterBase def impl of new Cell based APIs try to call KV based old APIs. CPs follow this way. I did not see branch-1 code base. I think ur attached patch is having some issue wrt maintain BC. Separate topic , we can discuss.

        So the end result is that we can conclude that the backport of the feature is possible with BC maintain. Good ..

        Show
        anoop.hbase Anoop Sam John added a comment - Thanks for the patches.. Ya all make sense.. On Filter, so we have Cell based APIs from 0.96 and deprecated KV based APIs there. But 1.x version contain both. So in branch-1 how is it used now? The core code flow calls cell based new APIs right? And FilterBase def impl of new Cell based APIs try to call KV based old APIs. CPs follow this way. I did not see branch-1 code base. I think ur attached patch is having some issue wrt maintain BC. Separate topic , we can discuss. So the end result is that we can conclude that the backport of the feature is possible with BC maintain. Good ..
        Hide
        yuzhihong@gmail.com Ted Yu added a comment -

        Considerable testing effort is needed on the final artifact with all the backports. The effort involves:

        1. showing the backports have the same level of performance boost as seen within Alibaba
        2. rolling restart works starting with the build artifact without the backports.
        3. ITBLL works with backports (maybe tricky since we don't know whether ITBLL passes for the tip of branch-1 and this is a moving target)

        Yu Sun
        Do you have bandwidth for the above actions ?

        If the answer is yea, you can start the discussion on dev@hbase to get community opinion.

        Show
        yuzhihong@gmail.com Ted Yu added a comment - Considerable testing effort is needed on the final artifact with all the backports. The effort involves: 1. showing the backports have the same level of performance boost as seen within Alibaba 2. rolling restart works starting with the build artifact without the backports. 3. ITBLL works with backports (maybe tricky since we don't know whether ITBLL passes for the tip of branch-1 and this is a moving target) Yu Sun Do you have bandwidth for the above actions ? If the answer is yea, you can start the discussion on dev@hbase to get community opinion.
        Hide
        haoran Yu Sun added a comment -

        Ted Yu As you have listed, the backport needs lots of effort and i think it will need weeks to complete this.
        I am afraid I dont have enouth time to do this and cause the backport to be delayed.
        so if any others who have interesting in doing backport, I would like to provide any necessary help if needed.

        Show
        haoran Yu Sun added a comment - Ted Yu As you have listed, the backport needs lots of effort and i think it will need weeks to complete this. I am afraid I dont have enouth time to do this and cause the backport to be delayed. so if any others who have interesting in doing backport, I would like to provide any necessary help if needed.
        Hide
        tychang Tianying Chang added a comment -

        Yu Sun Yu Li This improvement looks super useful to us too. We are currently on HBase 1.2.1. Will be very helpful if this can be backport. Anyone currently working on it?

        Show
        tychang Tianying Chang added a comment - Yu Sun Yu Li This improvement looks super useful to us too. We are currently on HBase 1.2.1. Will be very helpful if this can be backport. Anyone currently working on it?
        Hide
        anoop.hbase Anoop Sam John added a comment -

        Not as such.. The effort for backport seems more.. Ya Sunyu did all the backport to their custom version but on branch-1 it wont be a direct fit.. And then effort of testing.. Seems Yu Li was not having that much bandwidth.. Is there some interest for collaboration guys? Let us know.. We can do any help as needed.

        Show
        anoop.hbase Anoop Sam John added a comment - Not as such.. The effort for backport seems more.. Ya Sunyu did all the backport to their custom version but on branch-1 it wont be a direct fit.. And then effort of testing.. Seems Yu Li was not having that much bandwidth.. Is there some interest for collaboration guys? Let us know.. We can do any help as needed.
        Hide
        tychang Tianying Chang added a comment - - edited

        Anoop Sam John Yes, we are interested in getting this patch in our production online facint cluster which is running on 1.2.1, any help from you will be great!

        It seems the customized version in Alibaba was forked from 1.1 (although on top of it with many of their private patches). If so I thought the effort fro backport to 1.2 should be similar? Do you have a high level sense what else need to be done besides those 70+ patches?

        Thanks

        Show
        tychang Tianying Chang added a comment - - edited Anoop Sam John Yes, we are interested in getting this patch in our production online facint cluster which is running on 1.2.1, any help from you will be great! It seems the customized version in Alibaba was forked from 1.1 (although on top of it with many of their private patches). If so I thought the effort fro backport to 1.2 should be similar? Do you have a high level sense what else need to be done besides those 70+ patches? Thanks
        Hide
        anoop.hbase Anoop Sam John added a comment -

        Besides that 70+ patches nothing more.. There were some changes in Filter/CP APIs as needed by this work.. So if ur cluster is having custom CP/Filter, u might have to change the impl code (new API implementation) and recompile and use that new jar. I believe Alibaba also had similar case. That is not a big deal at all.. Main thing is backport of the patches. But ya rather than working on trunk patches backport, Alibaba custom backported versions would be better.. (That will be more closer to 1.2.1)..
        cc Yu Li,

        Show
        anoop.hbase Anoop Sam John added a comment - Besides that 70+ patches nothing more.. There were some changes in Filter/CP APIs as needed by this work.. So if ur cluster is having custom CP/Filter, u might have to change the impl code (new API implementation) and recompile and use that new jar. I believe Alibaba also had similar case. That is not a big deal at all.. Main thing is backport of the patches. But ya rather than working on trunk patches backport, Alibaba custom backported versions would be better.. (That will be more closer to 1.2.1).. cc Yu Li ,
        Hide
        tychang Tianying Chang added a comment -

        thanks Anoop Sam John what do you mean by trunk patches backport? Is this Alibaba's patch list for backporting from trunk/2.0 to 1.1? So the patch list above theoretically can be applied one by one on 1.2.1 open source version?

        Show
        tychang Tianying Chang added a comment - thanks Anoop Sam John what do you mean by trunk patches backport? Is this Alibaba's patch list for backporting from trunk/2.0 to 1.1? So the patch list above theoretically can be applied one by one on 1.2.1 open source version?
        Hide
        tychang Tianying Chang added a comment -

        thanks Anoop Sam John what do you mean by trunk patches backport? Is this Alibaba's patch list for backporting from trunk/2.0 to 1.1? So the patch list above theoretically can be applied one by one on 1.2.1 open source version?

        Show
        tychang Tianying Chang added a comment - thanks Anoop Sam John what do you mean by trunk patches backport? Is this Alibaba's patch list for backporting from trunk/2.0 to 1.1? So the patch list above theoretically can be applied one by one on 1.2.1 open source version?
        Hide
        anoop.hbase Anoop Sam John added a comment -

        The list is for backporting from 2.0 to 1.1 (Base version for Alibaba). What I mean is if Alibaba can share their backported patches (Each of it) which they applied on their 1.1 based version, that might be more useful than working 2.0 patches which are there in these jira attachments. Because 1.2.1 will be more closer to 1.1. than 2.0. Any way is fine.. Just trying to help.

        Show
        anoop.hbase Anoop Sam John added a comment - The list is for backporting from 2.0 to 1.1 (Base version for Alibaba). What I mean is if Alibaba can share their backported patches (Each of it) which they applied on their 1.1 based version, that might be more useful than working 2.0 patches which are there in these jira attachments. Because 1.2.1 will be more closer to 1.1. than 2.0. Any way is fine.. Just trying to help.
        Hide
        carp84 Yu Li added a comment -

        So if ur cluster is having custom CP/Filter, u might have to change the impl code (new API implementation) and recompile and use that new jar. I believe Alibaba also had similar case.

        True story, some more work like HBASE-16626 and cp changes, case by case though.

        1.2.1 will be more closer to 1.1 than 2.0

        Probably, but it's almost impossible for us to fully upload our code base to github (legal stuff you know...), and if we share the patches one by one, there'll be 70+ to generate and w/o git (cherry-pick/rebase) I'm afraid they're not easy to apply.

        Sorry that we're short of hand to complete this backport to branch-1 since we have a tight schedule to meet challenges of 2017 Singles' Day (while the work listed here finished in Sep. last year), but still we hope listing out the JIRAs and giving some proof that a backport can work well in production could help others who want this feature ahead of time (will be officially released in 2.0).

        OTOH, I'd strongly suggest to spend enough time on the backport – read and understand the purpose of each JIRA listed – to make sure of the quality and capability of handling problems if any.

        Show
        carp84 Yu Li added a comment - So if ur cluster is having custom CP/Filter, u might have to change the impl code (new API implementation) and recompile and use that new jar. I believe Alibaba also had similar case. True story, some more work like HBASE-16626 and cp changes, case by case though. 1.2.1 will be more closer to 1.1 than 2.0 Probably, but it's almost impossible for us to fully upload our code base to github (legal stuff you know...), and if we share the patches one by one, there'll be 70+ to generate and w/o git (cherry-pick/rebase) I'm afraid they're not easy to apply. Sorry that we're short of hand to complete this backport to branch-1 since we have a tight schedule to meet challenges of 2017 Singles' Day (while the work listed here finished in Sep. last year), but still we hope listing out the JIRAs and giving some proof that a backport can work well in production could help others who want this feature ahead of time (will be officially released in 2.0). OTOH, I'd strongly suggest to spend enough time on the backport – read and understand the purpose of each JIRA listed – to make sure of the quality and capability of handling problems if any.
        Hide
        tychang Tianying Chang added a comment -

        Yu Li I totally agree that should spend enough time to understand all the JIRA listed here, so have the quality and capability to handle problems in production if any.

        I am wondering if you can just post your patches against 1.1 here, just as a reference purpose? (no need to worry about clean apply-able) This way, for people doing back port, they can leverage your knowledge/lessons/experience, and have the extra information to debug if needed. This way, the back port can be done with less time.

        Show
        tychang Tianying Chang added a comment - Yu Li I totally agree that should spend enough time to understand all the JIRA listed here, so have the quality and capability to handle problems in production if any. I am wondering if you can just post your patches against 1.1 here, just as a reference purpose? (no need to worry about clean apply-able) This way, for people doing back port, they can leverage your knowledge/lessons/experience, and have the extra information to debug if needed. This way, the back port can be done with less time.
        Hide
        anoop.hbase Anoop Sam John added a comment -

        Yu Li Hope u had individual patches for those 70+ issues (Based on ur custom version) and not a mega patch. If so, it would be great if u share those patches. Share via some way so that others can just refer that. If it is a mega patch, then I would say better is master patches in these jiras.

        Show
        anoop.hbase Anoop Sam John added a comment - Yu Li Hope u had individual patches for those 70+ issues (Based on ur custom version) and not a mega patch. If so, it would be great if u share those patches. Share via some way so that others can just refer that. If it is a mega patch, then I would say better is master patches in these jiras.
        Hide
        carp84 Yu Li added a comment -

        It's almost impossible for us to fully upload our code base to github (legal stuff you know...), and if we share the patches one by one, there'll be 70+ to generate and w/o git (cherry-pick/rebase) I'm afraid they're not easy to apply.

        Tianying Chang Anoop Sam John Please allow me to quote my comment before. We have separate commits before merging into our own master (squashed), but not necessarily a one-one mapping between our commits and JIRAs. And you will need to apply them manually, which I'm not sure whether is easier to resolve the conflicts than using master version (where you could try cherry-pick). But yes, if you think this could help, we could generate and upload the patches (which requires some additional efforts than simply "git show", especially the mapping between commits and JIRAs for a good reference, we did the work half a year ago, so hopefully we could recall it correctly). Let me know your thoughts.

        Show
        carp84 Yu Li added a comment - It's almost impossible for us to fully upload our code base to github (legal stuff you know...), and if we share the patches one by one, there'll be 70+ to generate and w/o git (cherry-pick/rebase) I'm afraid they're not easy to apply. Tianying Chang Anoop Sam John Please allow me to quote my comment before. We have separate commits before merging into our own master (squashed), but not necessarily a one-one mapping between our commits and JIRAs. And you will need to apply them manually, which I'm not sure whether is easier to resolve the conflicts than using master version (where you could try cherry-pick). But yes, if you think this could help, we could generate and upload the patches (which requires some additional efforts than simply "git show", especially the mapping between commits and JIRAs for a good reference, we did the work half a year ago, so hopefully we could recall it correctly). Let me know your thoughts.
        Hide
        tychang Tianying Chang added a comment -

        > the above jira ids are listed in chronological order I have backport,and there are also some patch i dont list, such as:
        > 1. keep Cell api compatible with our existing code.
        > 2. hfile format related compatibility issues.
        > 3. client compatibility issue

        Yu Sun Yu Li Just to want to clarify do you have the above related jira attached in the list?

        how much benefit does HBASE-15756 contribute for the Singles day performane boost on top of this offheap feature?

        Show
        tychang Tianying Chang added a comment - > the above jira ids are listed in chronological order I have backport,and there are also some patch i dont list, such as: > 1. keep Cell api compatible with our existing code. > 2. hfile format related compatibility issues. > 3. client compatibility issue Yu Sun Yu Li Just to want to clarify do you have the above related jira attached in the list? how much benefit does HBASE-15756 contribute for the Singles day performane boost on top of this offheap feature?
        Hide
        carp84 Yu Li added a comment -

        how much benefit does HBASE-15756 contribute for the Singles day performane boost on top of this offheap feature?

        Please refer to this comment in HBASE-15756.

        For the other question, I think the answer is "No", but better with Yu Sun's confirmation.

        Show
        carp84 Yu Li added a comment - how much benefit does HBASE-15756 contribute for the Singles day performane boost on top of this offheap feature? Please refer to this comment in HBASE-15756 . For the other question, I think the answer is "No", but better with Yu Sun 's confirmation.
        Hide
        jingcheng.du Jingcheng Du added a comment - - edited

        The changes in RegionScanner in master branch is neither binary compatible nor source compatible for branch-1, and changes in RegionCoprocessorHost might be only binary compatible. If users move to a new 1.x version which includes the off-heap read path, they have to change the customized region scanners and coprocessors.
        Do we allow this kind of backward compatibility break in minor release? Sean Busbey Anoop Sam John Duo Zhang

        According to the compatibility matrix in HBase (https://hbase.apache.org/book.html#hbase.versioning), such changes are Server-Side Limited API compatibility, but the changes in RegionScanner break both binary and source compatibility.

        Show
        jingcheng.du Jingcheng Du added a comment - - edited The changes in RegionScanner in master branch is neither binary compatible nor source compatible for branch-1, and changes in RegionCoprocessorHost might be only binary compatible. If users move to a new 1.x version which includes the off-heap read path, they have to change the customized region scanners and coprocessors. Do we allow this kind of backward compatibility break in minor release? Sean Busbey Anoop Sam John Duo Zhang According to the compatibility matrix in HBase ( https://hbase.apache.org/book.html#hbase.versioning ), such changes are Server-Side Limited API compatibility, but the changes in RegionScanner break both binary and source compatibility.
        Hide
        busbey Sean Busbey added a comment -

        RegionScanner is marked LimitedPrivate / Evolving, which we do list as allowed to break on minor version. If you'd like some reassurance, it would be best to provide a single list of all the classes that break along with what their annotation is for Audience and Stability.

        We can then use that list to gauge impact on downstream. Phoenix is the most obvious community to check with, since we know they extensively make use of LimitedPrivate APIs.

        Show
        busbey Sean Busbey added a comment - RegionScanner is marked LimitedPrivate / Evolving, which we do list as allowed to break on minor version. If you'd like some reassurance, it would be best to provide a single list of all the classes that break along with what their annotation is for Audience and Stability. We can then use that list to gauge impact on downstream. Phoenix is the most obvious community to check with, since we know they extensively make use of LimitedPrivate APIs.

          People

          • Assignee:
            haoran Yu Sun
            Reporter:
            carp84 Yu Li
          • Votes:
            0 Vote for this issue
            Watchers:
            24 Start watching this issue

            Dates

            • Created:
              Updated:

              Development