hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Hadoop QA (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-15063) Bug in MultiByteBuf#toBytes
Date Fri, 01 Jan 2016 18:16:39 GMT

    [ https://issues.apache.org/jira/browse/HBASE-15063?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15076349#comment-15076349
] 

Hadoop QA commented on HBASE-15063:
-----------------------------------

{color:green}+1 overall{color}.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12780190/HBASE-15063-v2.patch
  against master branch at commit 92abf8ac5743849d4c32d6abf8fd33f161bab249.
  ATTACHMENT ID: 12780190

    {color:green}+1 @author{color}.  The patch does not contain any @author tags.

    {color:green}+1 tests included{color}.  The patch appears to include 4 new or modified
tests.

    {color:green}+1 hadoop versions{color}. The patch compiles with all supported hadoop versions
(2.4.0 2.4.1 2.5.0 2.5.1 2.5.2 2.6.0 2.6.1 2.7.0 2.7.1)

    {color:green}+1 javac{color}.  The applied patch does not increase the total number of
javac compiler warnings.

    {color:green}+1 protoc{color}.  The applied patch does not increase the total number of
protoc compiler warnings.

    {color:green}+1 javadoc{color}.  The javadoc tool did not generate any warning messages.

    {color:green}+1 checkstyle{color}. The applied patch does not generate new checkstyle
errors.

    {color:green}+1 findbugs{color}.  The patch does not introduce any  new Findbugs (version
2.0.3) warnings.

    {color:green}+1 release audit{color}.  The applied patch does not increase the total number
of release audit warnings.

    {color:green}+1 lineLengths{color}.  The patch does not introduce lines longer than 100

    {color:green}+1 site{color}.  The mvn post-site goal succeeds with this patch.

    {color:green}+1 core tests{color}.  The patch passed unit tests in .

    {color:green}+1 zombies{color}. No zombie tests found running at the end of the build.

Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/17099//testReport/
Release Findbugs (version 2.0.3) 	warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/17099//artifact/patchprocess/newFindbugsWarnings.html
Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/17099//artifact/patchprocess/checkstyle-aggregate.html

  Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/17099//console

This message is automatically generated.

> Bug in MultiByteBuf#toBytes
> ---------------------------
>
>                 Key: HBASE-15063
>                 URL: https://issues.apache.org/jira/browse/HBASE-15063
>             Project: HBase
>          Issue Type: Bug
>          Components: io, Performance
>    Affects Versions: 2.0.0
>            Reporter: deepankar
>            Assignee: deepankar
>            Priority: Critical
>             Fix For: 2.0.0
>
>         Attachments: HBASE-15063-v1.patch, HBASE-15063-v2.patch
>
>
> In MutliByteBuff there are couple of inconsistencies, one is in the toBytes function
where the offset for copying in the initial element is not being reset with respect to the
element
> {code}
>  public byte[] toBytes(int offset, int length) {
>      byte[] output = new byte[length];
>      int itemIndex = getItemIndex(offset);
>      ByteBuffer item = this.items[itemIndex];
>      // the offset has to be reset here to the items offset 
>      // should be offset = offset - itemBeingPos[itemIndex]
>      int toRead = item.limit() - offset;
>      int destinationOffset = 0;
>     .  .   .   .
> {code}
> Since there is already an existing function get to copy to an byte array it is better
we reuse the function here, I attached a patch with a corresponding unit test. (HBASE-XXXX.patch)
> Another inconsistency I noticed is that there is lack of some consistency in using the
position marker of the bytebuffers passed, In the constructor we noting the beginning offsets
of each bytebuffer in the {{itemBeginPos}} array we are using these position markers in most
of the absolute index functions such as {{get(index)}}, {{put(index, byte)}}, {{toBytes}},
{{get(int,byte[],int,int)}} to find the current item to access. There are two problems with
this
> # The array {{itemBeginPos}} is not being updated whenever there are some writes to internal
bytebuffers (remember {{itemBeginPos}} depends on the {{bytebuffer.position()}} of the internal
bytebuffers and the position marker is changed whenever some writes happen to bytebuffers)
> # Also the position marker is not being used in any of the index functions for example
here in the get function
>    {code}
>  @Override
>   public byte get(int index) {
>     int itemIndex = getItemIndex(index);
>     return ByteBufferUtils.toByte(this.items[itemIndex], index - this.itemBeginPos[itemIndex]);
>   } 
>    {code}
> where the the index I think should be {{index - this.itemBeginPos\[itemIndex\] + items\[itemIndex\].position()}}
because we are using the position marker to calculate the offsets.
> There are two solutions I think for this 
> # The simple solution I feel for this should be probably ignoring the position marker
while calculating the {{itemBeginPos}} array which will unify the semantics 
> # Not use this array at all and iterate over bytebuffers every time for the {{getItemIndex}}
function and also use the position marker before calling the {{ByteBufferUtils}} functions
> I can put up a patch for the second part if we decide which way to go.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message