[jira] [Commented] (HBASE-21401) Sanity check when constructing the KeyValue
[
https://issues.apache.org/jira/browse/HBASE-21401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16824033#comment-16824033
]
Hudson commented on HBASE-21401:
Results for branch master
[build #954 on
builds.a.o|https://builds.apache.org/job/HBase%20Nightly/job/master/954/]: (x)
*{color:red}-1 overall{color}*
details (if available):
(x) {color:red}-1 general checks{color}
-- For more information [see general
report|https://builds.apache.org/job/HBase%20Nightly/job/master/954//General_Nightly_Build_Report/]
(x) {color:red}-1 jdk8 hadoop2 checks{color}
-- For more information [see jdk8 (hadoop2)
report|https://builds.apache.org/job/HBase%20Nightly/job/master/954//JDK8_Nightly_Build_Report_(Hadoop2)/]
(x) {color:red}-1 jdk8 hadoop3 checks{color}
-- For more information [see jdk8 (hadoop3)
report|https://builds.apache.org/job/HBase%20Nightly/job/master/954//JDK8_Nightly_Build_Report_(Hadoop3)/]
(/) {color:green}+1 source release artifact{color}
-- See build output for details.
(/) {color:green}+1 client integration test{color}
> Sanity check when constructing the KeyValue
> ---
>
> Key: HBASE-21401
> URL: https://issues.apache.org/jira/browse/HBASE-21401
> Project: HBase
> Issue Type: Sub-task
> Components: regionserver
>Reporter: Zheng Hu
>Assignee: Zheng Hu
>Priority: Critical
> Fix For: 3.0.0, 2.2.0, 2.1.2, 2.0.4
>
> Attachments: HBASE-21401.v1.patch, HBASE-21401.v2.patch,
> HBASE-21401.v3.patch, HBASE-21401.v4.patch, HBASE-21401.v4.patch,
> HBASE-21401.v5.patch, HBASE-21401.v6.patch, HBASE-21401.v7.patch
>
>
> In KeyValueDecoder & ByteBuffKeyValueDecoder, we pass a byte buffer to
> initialize the Cell without a sanity check (check each field's offset
> exceed the byte buffer or not), so ArrayIndexOutOfBoundsException may happen
> when read the cell's fields, such as HBASE-21379, it's hard to debug this
> kind of bug.
> An earlier check will help to find such kind of bugs.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-21401) Sanity check when constructing the KeyValue
[
https://issues.apache.org/jira/browse/HBASE-21401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16798736#comment-16798736
]
Hudson commented on HBASE-21401:
Results for branch branch-1
[build #732 on
builds.a.o|https://builds.apache.org/job/HBase%20Nightly/job/branch-1/732/]:
(x) *{color:red}-1 overall{color}*
details (if available):
(x) {color:red}-1 general checks{color}
-- For more information [see general
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-1/732//General_Nightly_Build_Report/]
(x) {color:red}-1 jdk7 checks{color}
-- For more information [see jdk7
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-1/732//JDK7_Nightly_Build_Report/]
(x) {color:red}-1 jdk8 hadoop2 checks{color}
-- For more information [see jdk8 (hadoop2)
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-1/732//JDK8_Nightly_Build_Report_(Hadoop2)/]
(x) {color:red}-1 source release artifact{color}
-- See build output for details.
> Sanity check when constructing the KeyValue
> ---
>
> Key: HBASE-21401
> URL: https://issues.apache.org/jira/browse/HBASE-21401
> Project: HBase
> Issue Type: Sub-task
> Components: regionserver
>Reporter: Zheng Hu
>Assignee: Zheng Hu
>Priority: Critical
> Fix For: 3.0.0, 2.2.0, 2.1.2, 2.0.4
>
> Attachments: HBASE-21401.v1.patch, HBASE-21401.v2.patch,
> HBASE-21401.v3.patch, HBASE-21401.v4.patch, HBASE-21401.v4.patch,
> HBASE-21401.v5.patch, HBASE-21401.v6.patch, HBASE-21401.v7.patch
>
>
> In KeyValueDecoder & ByteBuffKeyValueDecoder, we pass a byte buffer to
> initialize the Cell without a sanity check (check each field's offset
> exceed the byte buffer or not), so ArrayIndexOutOfBoundsException may happen
> when read the cell's fields, such as HBASE-21379, it's hard to debug this
> kind of bug.
> An earlier check will help to find such kind of bugs.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-21401) Sanity check when constructing the KeyValue
[
https://issues.apache.org/jira/browse/HBASE-21401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16727172#comment-16727172
]
Hudson commented on HBASE-21401:
Results for branch branch-2
[build #1571 on
builds.a.o|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/1571/]:
(x) *{color:red}-1 overall{color}*
details (if available):
(/) {color:green}+1 general checks{color}
-- For more information [see general
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/1571//General_Nightly_Build_Report/]
(x) {color:red}-1 jdk8 hadoop2 checks{color}
-- For more information [see jdk8 (hadoop2)
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/1571//JDK8_Nightly_Build_Report_(Hadoop2)/]
(x) {color:red}-1 jdk8 hadoop3 checks{color}
-- For more information [see jdk8 (hadoop3)
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2/1571//JDK8_Nightly_Build_Report_(Hadoop3)/]
(/) {color:green}+1 source release artifact{color}
-- See build output for details.
(/) {color:green}+1 client integration test{color}
> Sanity check when constructing the KeyValue
> ---
>
> Key: HBASE-21401
> URL: https://issues.apache.org/jira/browse/HBASE-21401
> Project: HBase
> Issue Type: Sub-task
> Components: regionserver
>Reporter: Zheng Hu
>Assignee: Zheng Hu
>Priority: Critical
> Fix For: 3.0.0, 2.2.0, 2.1.2, 2.0.4
>
> Attachments: HBASE-21401.v1.patch, HBASE-21401.v2.patch,
> HBASE-21401.v3.patch, HBASE-21401.v4.patch, HBASE-21401.v4.patch,
> HBASE-21401.v5.patch, HBASE-21401.v6.patch, HBASE-21401.v7.patch
>
>
> In KeyValueDecoder & ByteBuffKeyValueDecoder, we pass a byte buffer to
> initialize the Cell without a sanity check (check each field's offset
> exceed the byte buffer or not), so ArrayIndexOutOfBoundsException may happen
> when read the cell's fields, such as HBASE-21379, it's hard to debug this
> kind of bug.
> An earlier check will help to find such kind of bugs.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-21401) Sanity check when constructing the KeyValue
[
https://issues.apache.org/jira/browse/HBASE-21401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16726896#comment-16726896
]
Hudson commented on HBASE-21401:
Results for branch branch-2.0
[build #1184 on
builds.a.o|https://builds.apache.org/job/HBase%20Nightly/job/branch-2.0/1184/]:
(x) *{color:red}-1 overall{color}*
details (if available):
(/) {color:green}+1 general checks{color}
-- For more information [see general
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2.0/1184//General_Nightly_Build_Report/]
(x) {color:red}-1 jdk8 hadoop2 checks{color}
-- Something went wrong running this stage, please [check relevant console
output|https://builds.apache.org/job/HBase%20Nightly/job/branch-2.0/1184//console].
(x) {color:red}-1 jdk8 hadoop3 checks{color}
-- For more information [see jdk8 (hadoop3)
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2.0/1184//JDK8_Nightly_Build_Report_(Hadoop3)/]
(/) {color:green}+1 source release artifact{color}
-- See build output for details.
> Sanity check when constructing the KeyValue
> ---
>
> Key: HBASE-21401
> URL: https://issues.apache.org/jira/browse/HBASE-21401
> Project: HBase
> Issue Type: Sub-task
> Components: regionserver
>Reporter: Zheng Hu
>Assignee: Zheng Hu
>Priority: Critical
> Fix For: 3.0.0, 2.2.0, 2.1.2, 2.0.4
>
> Attachments: HBASE-21401.v1.patch, HBASE-21401.v2.patch,
> HBASE-21401.v3.patch, HBASE-21401.v4.patch, HBASE-21401.v4.patch,
> HBASE-21401.v5.patch, HBASE-21401.v6.patch, HBASE-21401.v7.patch
>
>
> In KeyValueDecoder & ByteBuffKeyValueDecoder, we pass a byte buffer to
> initialize the Cell without a sanity check (check each field's offset
> exceed the byte buffer or not), so ArrayIndexOutOfBoundsException may happen
> when read the cell's fields, such as HBASE-21379, it's hard to debug this
> kind of bug.
> An earlier check will help to find such kind of bugs.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-21401) Sanity check when constructing the KeyValue
[
https://issues.apache.org/jira/browse/HBASE-21401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16726892#comment-16726892
]
Hudson commented on HBASE-21401:
Results for branch branch-2.1
[build #702 on
builds.a.o|https://builds.apache.org/job/HBase%20Nightly/job/branch-2.1/702/]:
(x) *{color:red}-1 overall{color}*
details (if available):
(/) {color:green}+1 general checks{color}
-- For more information [see general
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2.1/702//General_Nightly_Build_Report/]
(x) {color:red}-1 jdk8 hadoop2 checks{color}
-- For more information [see jdk8 (hadoop2)
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2.1/702//JDK8_Nightly_Build_Report_(Hadoop2)/]
(x) {color:red}-1 jdk8 hadoop3 checks{color}
-- For more information [see jdk8 (hadoop3)
report|https://builds.apache.org/job/HBase%20Nightly/job/branch-2.1/702//JDK8_Nightly_Build_Report_(Hadoop3)/]
(/) {color:green}+1 source release artifact{color}
-- See build output for details.
(/) {color:green}+1 client integration test{color}
> Sanity check when constructing the KeyValue
> ---
>
> Key: HBASE-21401
> URL: https://issues.apache.org/jira/browse/HBASE-21401
> Project: HBase
> Issue Type: Sub-task
> Components: regionserver
>Reporter: Zheng Hu
>Assignee: Zheng Hu
>Priority: Critical
> Fix For: 3.0.0, 2.2.0, 2.1.2, 2.0.4
>
> Attachments: HBASE-21401.v1.patch, HBASE-21401.v2.patch,
> HBASE-21401.v3.patch, HBASE-21401.v4.patch, HBASE-21401.v4.patch,
> HBASE-21401.v5.patch, HBASE-21401.v6.patch, HBASE-21401.v7.patch
>
>
> In KeyValueDecoder & ByteBuffKeyValueDecoder, we pass a byte buffer to
> initialize the Cell without a sanity check (check each field's offset
> exceed the byte buffer or not), so ArrayIndexOutOfBoundsException may happen
> when read the cell's fields, such as HBASE-21379, it's hard to debug this
> kind of bug.
> An earlier check will help to find such kind of bugs.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (HBASE-21401) Sanity check when constructing the KeyValue
[ https://issues.apache.org/jira/browse/HBASE-21401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16726452#comment-16726452 ] Zheng Hu commented on HBASE-21401: -- bq. Please fix checkstyle on commit. Add a release note too saying what this does but that it has some cost we need to address. Yes, sir. Thank you. > Sanity check when constructing the KeyValue > --- > > Key: HBASE-21401 > URL: https://issues.apache.org/jira/browse/HBASE-21401 > Project: HBase > Issue Type: Sub-task > Components: regionserver >Reporter: Zheng Hu >Assignee: Zheng Hu >Priority: Critical > Fix For: 3.0.0, 2.2.0, 2.1.3, 2.0.5 > > Attachments: HBASE-21401.v1.patch, HBASE-21401.v2.patch, > HBASE-21401.v3.patch, HBASE-21401.v4.patch, HBASE-21401.v4.patch, > HBASE-21401.v5.patch, HBASE-21401.v6.patch, HBASE-21401.v7.patch > > > In KeyValueDecoder & ByteBuffKeyValueDecoder, we pass a byte buffer to > initialize the Cell without a sanity check (check each field's offset > exceed the byte buffer or not), so ArrayIndexOutOfBoundsException may happen > when read the cell's fields, such as HBASE-21379, it's hard to debug this > kind of bug. > An earlier check will help to find such kind of bugs. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21401) Sanity check when constructing the KeyValue
[ https://issues.apache.org/jira/browse/HBASE-21401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16726430#comment-16726430 ] stack commented on HBASE-21401: --- +1 let safety win. Let’s address the overparsing in other follow on issues > Sanity check when constructing the KeyValue > --- > > Key: HBASE-21401 > URL: https://issues.apache.org/jira/browse/HBASE-21401 > Project: HBase > Issue Type: Sub-task > Components: regionserver >Reporter: Zheng Hu >Assignee: Zheng Hu >Priority: Critical > Fix For: 3.0.0, 2.2.0, 2.1.3, 2.0.5 > > Attachments: HBASE-21401.v1.patch, HBASE-21401.v2.patch, > HBASE-21401.v3.patch, HBASE-21401.v4.patch, HBASE-21401.v4.patch, > HBASE-21401.v5.patch, HBASE-21401.v6.patch, HBASE-21401.v7.patch > > > In KeyValueDecoder & ByteBuffKeyValueDecoder, we pass a byte buffer to > initialize the Cell without a sanity check (check each field's offset > exceed the byte buffer or not), so ArrayIndexOutOfBoundsException may happen > when read the cell's fields, such as HBASE-21379, it's hard to debug this > kind of bug. > An earlier check will help to find such kind of bugs. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21401) Sanity check when constructing the KeyValue
[ https://issues.apache.org/jira/browse/HBASE-21401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16726432#comment-16726432 ] stack commented on HBASE-21401: --- Please fix checkstyle on commit. Add a release note too saying what this does but that it has some cost we need to address. Thank you [~openinx] > Sanity check when constructing the KeyValue > --- > > Key: HBASE-21401 > URL: https://issues.apache.org/jira/browse/HBASE-21401 > Project: HBase > Issue Type: Sub-task > Components: regionserver >Reporter: Zheng Hu >Assignee: Zheng Hu >Priority: Critical > Fix For: 3.0.0, 2.2.0, 2.1.3, 2.0.5 > > Attachments: HBASE-21401.v1.patch, HBASE-21401.v2.patch, > HBASE-21401.v3.patch, HBASE-21401.v4.patch, HBASE-21401.v4.patch, > HBASE-21401.v5.patch, HBASE-21401.v6.patch, HBASE-21401.v7.patch > > > In KeyValueDecoder & ByteBuffKeyValueDecoder, we pass a byte buffer to > initialize the Cell without a sanity check (check each field's offset > exceed the byte buffer or not), so ArrayIndexOutOfBoundsException may happen > when read the cell's fields, such as HBASE-21379, it's hard to debug this > kind of bug. > An earlier check will help to find such kind of bugs. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21401) Sanity check when constructing the KeyValue
[ https://issues.apache.org/jira/browse/HBASE-21401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16726398#comment-16726398 ] Zheng Hu commented on HBASE-21401: -- bq. I don't like that we are adding friction to KV making because it will impinge on perf. Yeah, I understand you consideration. Our HBASE-21459 indicated that there's extremely small performance loss for diff kinds of keyvalue, so I think should be OK. bq. What happens if we do not commit this? For blocks coming from hdfs (hfile and/or WAL), they should be checksummed so read should fail if bits are corrupted between writing and re-reading. Is the reading from socket when we read from client? That seems like a good time to do this extensive check. The sanity check isn't designed for discovering the bits corruption in network transferring or disk IO. It is designed to detect bugs inside HBase in advance. Let see an example: 1. client mis-encoded a KeyValue in some rare case; 2. client made a checksum and sent to RS; 3. RS checked the checksum (checksum was OK), and wrapped the byte[] as cell (KeyValue constructioin) 4. RS tried to get a family but found that the offset overflow. The problem is, we cannot ensure bug was caused by RegionServer's KeyValue encoding or mis-encoding by client ? If we had the sanity check, in step.3 , we would got a exception, it mis-encoded definitly by client . The HFile reading or WAL reading (replication) will also benefit from the sanity check too. Thanks. > Sanity check when constructing the KeyValue > --- > > Key: HBASE-21401 > URL: https://issues.apache.org/jira/browse/HBASE-21401 > Project: HBase > Issue Type: Sub-task > Components: regionserver >Reporter: Zheng Hu >Assignee: Zheng Hu >Priority: Critical > Fix For: 3.0.0, 2.2.0, 2.1.3, 2.0.5 > > Attachments: HBASE-21401.v1.patch, HBASE-21401.v2.patch, > HBASE-21401.v3.patch, HBASE-21401.v4.patch, HBASE-21401.v4.patch, > HBASE-21401.v5.patch, HBASE-21401.v6.patch, HBASE-21401.v7.patch > > > In KeyValueDecoder & ByteBuffKeyValueDecoder, we pass a byte buffer to > initialize the Cell without a sanity check (check each field's offset > exceed the byte buffer or not), so ArrayIndexOutOfBoundsException may happen > when read the cell's fields, such as HBASE-21379, it's hard to debug this > kind of bug. > An earlier check will help to find such kind of bugs. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21401) Sanity check when constructing the KeyValue
[ https://issues.apache.org/jira/browse/HBASE-21401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16726378#comment-16726378 ] stack commented on HBASE-21401: --- [~openinx] As you can tell, I don't like that we are adding friction to KV making because it will impinge on perf. It is good that there is no overlapping of checks. I've not done the review to see if it possible to avoid this check. What happens if we do not commit this? For blocks coming from hdfs (hfile and/or WAL), they should be checksummed so read should fail if bits are corrupted between writing and re-reading. Is the reading from socket when we read from client? That seems like a good time to do this extensive check. > Sanity check when constructing the KeyValue > --- > > Key: HBASE-21401 > URL: https://issues.apache.org/jira/browse/HBASE-21401 > Project: HBase > Issue Type: Sub-task > Components: regionserver >Reporter: Zheng Hu >Assignee: Zheng Hu >Priority: Critical > Fix For: 3.0.0, 2.2.0, 2.1.3, 2.0.5 > > Attachments: HBASE-21401.v1.patch, HBASE-21401.v2.patch, > HBASE-21401.v3.patch, HBASE-21401.v4.patch, HBASE-21401.v4.patch, > HBASE-21401.v5.patch, HBASE-21401.v6.patch, HBASE-21401.v7.patch > > > In KeyValueDecoder & ByteBuffKeyValueDecoder, we pass a byte buffer to > initialize the Cell without a sanity check (check each field's offset > exceed the byte buffer or not), so ArrayIndexOutOfBoundsException may happen > when read the cell's fields, such as HBASE-21379, it's hard to debug this > kind of bug. > An earlier check will help to find such kind of bugs. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21401) Sanity check when constructing the KeyValue
[ https://issues.apache.org/jira/browse/HBASE-21401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16725524#comment-16725524 ] Zheng Hu commented on HBASE-21401: -- Sorry for pinging again, [~stack] boss. I'm trying to push this issue as fast as possible, so we can make this related issue finished. > Sanity check when constructing the KeyValue > --- > > Key: HBASE-21401 > URL: https://issues.apache.org/jira/browse/HBASE-21401 > Project: HBase > Issue Type: Sub-task > Components: regionserver >Reporter: Zheng Hu >Assignee: Zheng Hu >Priority: Critical > Fix For: 3.0.0, 2.2.0, 2.1.3, 2.0.5 > > Attachments: HBASE-21401.v1.patch, HBASE-21401.v2.patch, > HBASE-21401.v3.patch, HBASE-21401.v4.patch, HBASE-21401.v4.patch, > HBASE-21401.v5.patch, HBASE-21401.v6.patch, HBASE-21401.v7.patch > > > In KeyValueDecoder & ByteBuffKeyValueDecoder, we pass a byte buffer to > initialize the Cell without a sanity check (check each field's offset > exceed the byte buffer or not), so ArrayIndexOutOfBoundsException may happen > when read the cell's fields, such as HBASE-21379, it's hard to debug this > kind of bug. > An earlier check will help to find such kind of bugs. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21401) Sanity check when constructing the KeyValue
[ https://issues.apache.org/jira/browse/HBASE-21401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16724706#comment-16724706 ] Zheng Hu commented on HBASE-21401: -- bq. KeyValue does checks of lengths to make sure they are not insane (e.g. KeyValue#checkParameters). How much overlap between your check and these? Will we be doing double checking? Thanks. Good question, I checked the lengths checking, such as KeyValue#createEmptyByteArray, KeyValue#checkParameters ... No overlap between my check and those checks, I think. becase my sanity check only works when we construct an KeyValue from an complete byte[] (no other params), we use this kind of constructor because we read bytes from socket or wal or hfile. the existed length check only works when we construct kv from the given row, cf, cq, ts, type and so on. we use this because we have known the cf/cq/ts/type etc (not read byte stream from IO), and construct kv to do further work (such as optimized seek in sever side or construct the client side request). So, in theory, no overlap between them. Thanks > Sanity check when constructing the KeyValue > --- > > Key: HBASE-21401 > URL: https://issues.apache.org/jira/browse/HBASE-21401 > Project: HBase > Issue Type: Sub-task > Components: regionserver >Reporter: Zheng Hu >Assignee: Zheng Hu >Priority: Critical > Fix For: 3.0.0, 2.2.0, 2.1.3, 2.0.5 > > Attachments: HBASE-21401.v1.patch, HBASE-21401.v2.patch, > HBASE-21401.v3.patch, HBASE-21401.v4.patch, HBASE-21401.v4.patch, > HBASE-21401.v5.patch, HBASE-21401.v6.patch, HBASE-21401.v7.patch > > > In KeyValueDecoder & ByteBuffKeyValueDecoder, we pass a byte buffer to > initialize the Cell without a sanity check (check each field's offset > exceed the byte buffer or not), so ArrayIndexOutOfBoundsException may happen > when read the cell's fields, such as HBASE-21379, it's hard to debug this > kind of bug. > An earlier check will help to find such kind of bugs. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21401) Sanity check when constructing the KeyValue
[ https://issues.apache.org/jira/browse/HBASE-21401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16724687#comment-16724687 ] stack commented on HBASE-21401: --- You have a point [~openinx] that doing checks inline will bulk up regular processing some. The basis though is that we are obscene when it comes to how many times we do offset and length calculations (We should fix this! Smile). KeyValue does checks of lengths to make sure they are not insane (e.g. KeyValue#checkParameters). How much overlap between your check and these? Will we be doing double checking? Thanks. > Sanity check when constructing the KeyValue > --- > > Key: HBASE-21401 > URL: https://issues.apache.org/jira/browse/HBASE-21401 > Project: HBase > Issue Type: Sub-task > Components: regionserver >Reporter: Zheng Hu >Assignee: Zheng Hu >Priority: Critical > Fix For: 3.0.0, 2.2.0, 2.1.3, 2.0.5 > > Attachments: HBASE-21401.v1.patch, HBASE-21401.v2.patch, > HBASE-21401.v3.patch, HBASE-21401.v4.patch, HBASE-21401.v4.patch, > HBASE-21401.v5.patch, HBASE-21401.v6.patch, HBASE-21401.v7.patch > > > In KeyValueDecoder & ByteBuffKeyValueDecoder, we pass a byte buffer to > initialize the Cell without a sanity check (check each field's offset > exceed the byte buffer or not), so ArrayIndexOutOfBoundsException may happen > when read the cell's fields, such as HBASE-21379, it's hard to debug this > kind of bug. > An earlier check will help to find such kind of bugs. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21401) Sanity check when constructing the KeyValue
[ https://issues.apache.org/jira/browse/HBASE-21401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16724637#comment-16724637 ] Zheng Hu commented on HBASE-21401: -- bq. I looked at the patch and I still see double-parse, no? (Once to check byte array contains a wholesome KV and then the usual parse that happens as part of KV usage?). Was thinking we could check wholesomeness inline with use? Yes, it's double-parse now, once to check the wholesome KV, then parse the specific fields such as row/family/qualifler/ts/type and so on. I did not move the check wholesomeness inline with use, because I found that in the upper layer, the cell.getRowOffset() and cell.getRowLength() will be called many times. take the scan processing as an example: step.1 load block from hfile, and let the cell to ref to the block; step.2 compare the row part with given startRow or stopRow in scan, call the cell.getRowOffset() and cell.getRowOffset(); step.3 Merge with other hfiles, still need compare the row part . call the cell.getRowOffset() and cell.getRowOffset() ; step.4 filters ... compare the row/family/qulifier/value. step.3 Merge with other stores, compare the row part ... I mean the getRowOffset() and getRowOffset() (or getFamilyOffset/getFamilyLength() ... ) will be used in the uppler layer so many times. If we move the row sanity check in getRowOffset() and getRowOffset(), move the family sanity check in getFamilyOffset() and getFamilyOffset the sanity check will parse the relative fields so many times too ? the cost even large than the double-check, so i think the double-parse will be better in our case. Please correct me if I mis-understood something or missed something. > Sanity check when constructing the KeyValue > --- > > Key: HBASE-21401 > URL: https://issues.apache.org/jira/browse/HBASE-21401 > Project: HBase > Issue Type: Sub-task > Components: regionserver >Reporter: Zheng Hu >Assignee: Zheng Hu >Priority: Critical > Fix For: 3.0.0, 2.2.0, 2.1.3, 2.0.5 > > Attachments: HBASE-21401.v1.patch, HBASE-21401.v2.patch, > HBASE-21401.v3.patch, HBASE-21401.v4.patch, HBASE-21401.v4.patch, > HBASE-21401.v5.patch, HBASE-21401.v6.patch, HBASE-21401.v7.patch > > > In KeyValueDecoder & ByteBuffKeyValueDecoder, we pass a byte buffer to > initialize the Cell without a sanity check (check each field's offset > exceed the byte buffer or not), so ArrayIndexOutOfBoundsException may happen > when read the cell's fields, such as HBASE-21379, it's hard to debug this > kind of bug. > An earlier check will help to find such kind of bugs. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21401) Sanity check when constructing the KeyValue
[ https://issues.apache.org/jira/browse/HBASE-21401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16724291#comment-16724291 ] stack commented on HBASE-21401: --- Pardon me [~openinx] I missed this. So, I looked at the patch and I still see double-parse, no? (Once to check byte array contains a wholesome KV and then the usual parse that happens as part of KV usage?). Was thinking we could check wholesomeness inline with use? (Pardon me if I'm not following along the conversation -- I've switched out the context here). > Sanity check when constructing the KeyValue > --- > > Key: HBASE-21401 > URL: https://issues.apache.org/jira/browse/HBASE-21401 > Project: HBase > Issue Type: Sub-task > Components: regionserver >Reporter: Zheng Hu >Assignee: Zheng Hu >Priority: Critical > Fix For: 3.0.0, 2.2.0, 2.1.3, 2.0.5 > > Attachments: HBASE-21401.v1.patch, HBASE-21401.v2.patch, > HBASE-21401.v3.patch, HBASE-21401.v4.patch, HBASE-21401.v4.patch, > HBASE-21401.v5.patch, HBASE-21401.v6.patch, HBASE-21401.v7.patch > > > In KeyValueDecoder & ByteBuffKeyValueDecoder, we pass a byte buffer to > initialize the Cell without a sanity check (check each field's offset > exceed the byte buffer or not), so ArrayIndexOutOfBoundsException may happen > when read the cell's fields, such as HBASE-21379, it's hard to debug this > kind of bug. > An earlier check will help to find such kind of bugs. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21401) Sanity check when constructing the KeyValue
[ https://issues.apache.org/jira/browse/HBASE-21401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16723799#comment-16723799 ] Zheng Hu commented on HBASE-21401: -- Ping [~stack], [~Apache9], [~anoop.hbase]. > Sanity check when constructing the KeyValue > --- > > Key: HBASE-21401 > URL: https://issues.apache.org/jira/browse/HBASE-21401 > Project: HBase > Issue Type: Sub-task > Components: regionserver >Reporter: Zheng Hu >Assignee: Zheng Hu >Priority: Critical > Fix For: 3.0.0, 2.2.0, 2.1.3, 2.0.5 > > Attachments: HBASE-21401.v1.patch, HBASE-21401.v2.patch, > HBASE-21401.v3.patch, HBASE-21401.v4.patch, HBASE-21401.v4.patch, > HBASE-21401.v5.patch, HBASE-21401.v6.patch, HBASE-21401.v7.patch > > > In KeyValueDecoder & ByteBuffKeyValueDecoder, we pass a byte buffer to > initialize the Cell without a sanity check (check each field's offset > exceed the byte buffer or not), so ArrayIndexOutOfBoundsException may happen > when read the cell's fields, such as HBASE-21379, it's hard to debug this > kind of bug. > An earlier check will help to find such kind of bugs. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21401) Sanity check when constructing the KeyValue
[ https://issues.apache.org/jira/browse/HBASE-21401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16708703#comment-16708703 ] Zheng Hu commented on HBASE-21401: -- [~stack], any other concern ? > Sanity check when constructing the KeyValue > --- > > Key: HBASE-21401 > URL: https://issues.apache.org/jira/browse/HBASE-21401 > Project: HBase > Issue Type: Sub-task > Components: regionserver >Reporter: Zheng Hu >Assignee: Zheng Hu >Priority: Critical > Fix For: 3.0.0, 2.2.0, 2.1.2, 2.0.4 > > Attachments: HBASE-21401.v1.patch, HBASE-21401.v2.patch, > HBASE-21401.v3.patch, HBASE-21401.v4.patch, HBASE-21401.v4.patch, > HBASE-21401.v5.patch, HBASE-21401.v6.patch, HBASE-21401.v7.patch > > > In KeyValueDecoder & ByteBuffKeyValueDecoder, we pass a byte buffer to > initialize the Cell without a sanity check (check each field's offset > exceed the byte buffer or not), so ArrayIndexOutOfBoundsException may happen > when read the cell's fields, such as HBASE-21379, it's hard to debug this > kind of bug. > An earlier check will help to find such kind of bugs. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21401) Sanity check when constructing the KeyValue
[
https://issues.apache.org/jira/browse/HBASE-21401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16708276#comment-16708276
]
Hadoop QA commented on HBASE-21401:
---
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
21s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} hbaseanti {color} | {color:green} 0m
0s{color} | {color:green} Patch does not have any anti-patterns. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 4 new or modified test
files. {color} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
31s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 4m
42s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m
26s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 1m
28s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green} 3m
53s{color} | {color:green} branch has no errors when building our shaded
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 2m
30s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
51s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
13s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 4m
10s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m
13s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 2m
13s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 0m
26s{color} | {color:red} hbase-common: The patch generated 21 new + 148
unchanged - 1 fixed = 169 total (was 149) {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 1m
6s{color} | {color:green} hbase-server: The patch generated 0 new + 46
unchanged - 1 fixed = 46 total (was 47) {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green} 3m
53s{color} | {color:green} patch has no errors when building our shaded
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} hadoopcheck {color} | {color:green}
8m 27s{color} | {color:green} Patch does not cause any errors with Hadoop 2.7.4
or 3.0.0. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 2m
47s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
47s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 2m
46s{color} | {color:green} hbase-common in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green}129m
37s{color} | {color:green} hbase-server in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
47s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black}174m 35s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:b002b0b |
| JIRA Issue | HBASE-21401 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12950493/HBASE-21401.v7.patch |
| Optional Tests | dupname asflicense javac javadoc unit findbugs
shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux f5f3d448770c 4.4.0-138-generic #164-Ubuntu SMP Tue Oct 2
17:16:02 UTC 2018 x86_64 GNU/Linux |
| Build tool | maven |
| Personality |
[jira] [Commented] (HBASE-21401) Sanity check when constructing the KeyValue
[ https://issues.apache.org/jira/browse/HBASE-21401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16708155#comment-16708155 ] Zheng Hu commented on HBASE-21401: -- Patch.v7 Fixed all the failed UT > Sanity check when constructing the KeyValue > --- > > Key: HBASE-21401 > URL: https://issues.apache.org/jira/browse/HBASE-21401 > Project: HBase > Issue Type: Sub-task > Components: regionserver >Reporter: Zheng Hu >Assignee: Zheng Hu >Priority: Critical > Fix For: 3.0.0, 2.2.0, 2.1.2, 2.0.4 > > Attachments: HBASE-21401.v1.patch, HBASE-21401.v2.patch, > HBASE-21401.v3.patch, HBASE-21401.v4.patch, HBASE-21401.v4.patch, > HBASE-21401.v5.patch, HBASE-21401.v6.patch, HBASE-21401.v7.patch > > > In KeyValueDecoder & ByteBuffKeyValueDecoder, we pass a byte buffer to > initialize the Cell without a sanity check (check each field's offset > exceed the byte buffer or not), so ArrayIndexOutOfBoundsException may happen > when read the cell's fields, such as HBASE-21379, it's hard to debug this > kind of bug. > An earlier check will help to find such kind of bugs. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HBASE-21401) Sanity check when constructing the KeyValue
[
https://issues.apache.org/jira/browse/HBASE-21401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16703289#comment-16703289
]
Hadoop QA commented on HBASE-21401:
---
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
24s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} hbaseanti {color} | {color:green} 0m
0s{color} | {color:green} Patch does not have any anti-patterns. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 1 new or modified test
files. {color} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
28s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 4m
32s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m
22s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 1m
31s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green} 3m
51s{color} | {color:green} branch has no errors when building our shaded
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 2m
36s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
43s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
14s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 3m
57s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m
16s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 2m
16s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 0m
24s{color} | {color:red} hbase-common: The patch generated 21 new + 148
unchanged - 1 fixed = 169 total (was 149) {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} shadedjars {color} | {color:green} 3m
49s{color} | {color:green} patch has no errors when building our shaded
downstream artifacts. {color} |
| {color:green}+1{color} | {color:green} hadoopcheck {color} | {color:green}
8m 41s{color} | {color:green} Patch does not cause any errors with Hadoop 2.7.4
or 3.0.0. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 2m
51s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
47s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 2m 34s{color}
| {color:red} hbase-common in the patch failed. {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red}248m 19s{color}
| {color:red} hbase-server in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
41s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black}292m 52s{color} |
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests | hadoop.hbase.TestKeyValue |
| | hadoop.hbase.master.TestAssignmentManagerMetrics |
| | hadoop.hbase.io.hfile.TestCacheOnWrite |
| | hadoop.hbase.io.encoding.TestDataBlockEncoders |
| | hadoop.hbase.io.hfile.TestHFileSeek |
| | hadoop.hbase.client.TestAdmin1 |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:b002b0b |
| JIRA Issue | HBASE-21401 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12949984/HBASE-21401.v6.patch |
| Optional Tests | dupname asflicense javac javadoc unit findbugs
shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux ab114c9e23ba 4.4.0-138-generic #164-Ubuntu SMP Tue Oct 2
[jira] [Commented] (HBASE-21401) Sanity check when constructing the KeyValue
[ https://issues.apache.org/jira/browse/HBASE-21401?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16702938#comment-16702938 ] Zheng Hu commented on HBASE-21401: -- Moved the sanity check into KV constructor, Let's see if hadoop QA say something. > Sanity check when constructing the KeyValue > --- > > Key: HBASE-21401 > URL: https://issues.apache.org/jira/browse/HBASE-21401 > Project: HBase > Issue Type: Sub-task > Components: regionserver >Reporter: Zheng Hu >Assignee: Zheng Hu >Priority: Critical > Fix For: 3.0.0, 2.2.0, 2.0.3, 2.1.2 > > Attachments: HBASE-21401.v1.patch, HBASE-21401.v2.patch, > HBASE-21401.v3.patch, HBASE-21401.v4.patch, HBASE-21401.v4.patch, > HBASE-21401.v5.patch, HBASE-21401.v6.patch > > > In KeyValueDecoder & ByteBuffKeyValueDecoder, we pass a byte buffer to > initialize the Cell without a sanity check (check each field's offset > exceed the byte buffer or not), so ArrayIndexOutOfBoundsException may happen > when read the cell's fields, such as HBASE-21379, it's hard to debug this > kind of bug. > An earlier check will help to find such kind of bugs. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
