[jira] [Commented] (HBASE-16624) MVCC DeSerialization bug in the HFileScannerImpl
[ https://issues.apache.org/jira/browse/HBASE-16624?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15494608#comment-15494608 ] Hudson commented on HBASE-16624: FAILURE: Integrated in Jenkins build HBase-Trunk_matrix #1608 (See [https://builds.apache.org/job/HBase-Trunk_matrix/1608/]) HBASE-16624 Fix MVCC DeSerialization bug in the HFileScannerImpl (stack: rev 8c4b09dfbaf53fd770fe3963df6095fc690f2ef5) * (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderImpl.java > MVCC DeSerialization bug in the HFileScannerImpl > > > Key: HBASE-16624 > URL: https://issues.apache.org/jira/browse/HBASE-16624 > Project: HBase > Issue Type: Bug > Components: HFile >Affects Versions: 2.0.0 >Reporter: deepankar >Assignee: deepankar >Priority: Blocker > Fix For: 2.0.0 > > Attachments: HBASE-16624.patch > > > My colleague [~naggarwal] found a bug in the deserialization of mvcc from > HFile, As a part of the optimization of deserialization of VLong, we read a > int at once but we forgot to convert it to unsigned one. > This would cause issues because once we cross the integer threshold in > sequenceId and a compaction happens we would write MAX_MEMSTORE_TS in the > trailer as 0 (because we will be reading negative values from the file that > got flushed with sequenceId > Integer.MAX_VALUE). And once we have > MAX_MEMSTORE_TS as 0, and there are sequenceId values present alongside with > KeyValues the regionserver will now start failing to read the compacted file > and thus corruption. > Interestingly this would happen only on the tables that don't have > DataBlockEncoding enabled and unfortunately in our case that turned out to be > META and a another small table. > Fix is small (~20 chars) and attached -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-16624) MVCC DeSerialization bug in the HFileScannerImpl
[ https://issues.apache.org/jira/browse/HBASE-16624?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15487602#comment-15487602 ] stack commented on HBASE-16624: --- +1 on the fix. Good find. Agree a test would be good but rather, we should have a bigger project that looks for all places where we do this sort of dodgy convertion ...I'd think this sort of practice, from int to long, would flag in findbugs or other static analysis tools. > MVCC DeSerialization bug in the HFileScannerImpl > > > Key: HBASE-16624 > URL: https://issues.apache.org/jira/browse/HBASE-16624 > Project: HBase > Issue Type: Bug > Components: HFile >Affects Versions: 2.0.0 >Reporter: deepankar >Assignee: deepankar >Priority: Blocker > Attachments: HBASE-16624.patch > > > My colleague [~naggarwal] found a bug in the deserialization of mvcc from > HFile, As a part of the optimization of deserialization of VLong, we read a > int at once but we forgot to convert it to unsigned one. > This would cause issues because once we cross the integer threshold in > sequenceId and a compaction happens we would write MAX_MEMSTORE_TS in the > trailer as 0 (because we will be reading negative values from the file that > got flushed with sequenceId > Integer.MAX_VALUE). And once we have > MAX_MEMSTORE_TS as 0, and there are sequenceId values present alongside with > KeyValues the regionserver will now start failing to read the compacted file > and thus corruption. > Interestingly this would happen only on the tables that don't have > DataBlockEncoding enabled and unfortunately in our case that turned out to be > META and a another small table. > Fix is small (~20 chars) and attached -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-16624) MVCC DeSerialization bug in the HFileScannerImpl
[ https://issues.apache.org/jira/browse/HBASE-16624?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15487595#comment-15487595 ] stack commented on HBASE-16624: --- I think this a good supposition but method looks too big to inline... > MVCC DeSerialization bug in the HFileScannerImpl > > > Key: HBASE-16624 > URL: https://issues.apache.org/jira/browse/HBASE-16624 > Project: HBase > Issue Type: Bug > Components: HFile >Affects Versions: 2.0.0 >Reporter: deepankar >Assignee: deepankar >Priority: Blocker > Attachments: HBASE-16624.patch > > > My colleague [~naggarwal] found a bug in the deserialization of mvcc from > HFile, As a part of the optimization of deserialization of VLong, we read a > int at once but we forgot to convert it to unsigned one. > This would cause issues because once we cross the integer threshold in > sequenceId and a compaction happens we would write MAX_MEMSTORE_TS in the > trailer as 0 (because we will be reading negative values from the file that > got flushed with sequenceId > Integer.MAX_VALUE). And once we have > MAX_MEMSTORE_TS as 0, and there are sequenceId values present alongside with > KeyValues the regionserver will now start failing to read the compacted file > and thus corruption. > Interestingly this would happen only on the tables that don't have > DataBlockEncoding enabled and unfortunately in our case that turned out to be > META and a another small table. > Fix is small (~20 chars) and attached -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-16624) MVCC DeSerialization bug in the HFileScannerImpl
[ https://issues.apache.org/jira/browse/HBASE-16624?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15486672#comment-15486672 ] Anoop Sam John commented on HBASE-16624: +1. Lets get this in. Will see how/whether we can make this as a util kind of method or so, so that we can UT this. Lets get this in first. This is an imp fix. Similar read of mvcc vlong is done in 1.x versions also I believe. May be 1.2+ versions. cc [~saint@gmail.com] > MVCC DeSerialization bug in the HFileScannerImpl > > > Key: HBASE-16624 > URL: https://issues.apache.org/jira/browse/HBASE-16624 > Project: HBase > Issue Type: Bug > Components: HFile >Affects Versions: 2.0.0 >Reporter: deepankar >Assignee: deepankar >Priority: Blocker > Attachments: HBASE-16624.patch > > > My colleague [~naggarwal] found a bug in the deserialization of mvcc from > HFile, As a part of the optimization of deserialization of VLong, we read a > int at once but we forgot to convert it to unsigned one. > This would cause issues because once we cross the integer threshold in > sequenceId and a compaction happens we would write MAX_MEMSTORE_TS in the > trailer as 0 (because we will be reading negative values from the file that > got flushed with sequenceId > Integer.MAX_VALUE). And once we have > MAX_MEMSTORE_TS as 0, and there are sequenceId values present alongside with > KeyValues the regionserver will now start failing to read the compacted file > and thus corruption. > Interestingly this would happen only on the tables that don't have > DataBlockEncoding enabled and unfortunately in our case that turned out to be > META and a another small table. > Fix is small (~20 chars) and attached -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-16624) MVCC DeSerialization bug in the HFileScannerImpl
[ https://issues.apache.org/jira/browse/HBASE-16624?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15486470#comment-15486470 ] deepankar commented on HBASE-16624: --- Inorder to test we may need to refactor and isolate this function and the reason I think this issue came up was because this is a very hot method and was optimized to make sure there are unnecessary methdo redirections to make sure jit works and inlines it. > MVCC DeSerialization bug in the HFileScannerImpl > > > Key: HBASE-16624 > URL: https://issues.apache.org/jira/browse/HBASE-16624 > Project: HBase > Issue Type: Bug > Components: HFile >Affects Versions: 2.0.0 >Reporter: deepankar >Assignee: deepankar >Priority: Blocker > Attachments: HBASE-16624.patch > > > My colleague [~naggarwal] found a bug in the deserialization of mvcc from > HFile, As a part of the optimization of deserialization of VLong, we read a > int at once but we forgot to convert it to unsigned one. > This would cause issues because once we cross the integer threshold in > sequenceId and a compaction happens we would write MAX_MEMSTORE_TS in the > trailer as 0 (because we will be reading negative values from the file that > got flushed with sequenceId > Integer.MAX_VALUE). And once we have > MAX_MEMSTORE_TS as 0, and there are sequenceId values present alongside with > KeyValues the regionserver will now start failing to read the compacted file > and thus corruption. > Interestingly this would happen only on the tables that don't have > DataBlockEncoding enabled and unfortunately in our case that turned out to be > META and a another small table. > Fix is small (~20 chars) and attached -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-16624) MVCC DeSerialization bug in the HFileScannerImpl
[
https://issues.apache.org/jira/browse/HBASE-16624?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15486459#comment-15486459
]
deepankar commented on HBASE-16624:
---
Function is nested deep inside the HFileScannerImpl and it was hard to come up
with a UT. But if you can cross the sequenceId above the Integer.MAX_VALUE you
would immediately see the issue, to confirm the issue I have changed the
function
{code}
/**
* Actually do the mvcc read. Does no checks.
* @param offsetFromPos
*/
private long _readMvccVersion(ByteBuff blockBuffer) {
long currMemstoreTS = 0;
int offsetFromPos = 0;
// This is Bytes#bytesToVint inlined so can save a few instructions in
this hot method; i.e.
// previous if one-byte vint, we'd redo the vint call to find int size.
// Also the method is kept small so can be inlined.
byte firstByte = blockBuffer.getByteAfterPosition(offsetFromPos);
int len = WritableUtils.decodeVIntSize(firstByte);
if (len == 1) {
currMemstoreTS = firstByte;
} else {
int remaining = len -1;
long i = 0;
offsetFromPos++;
if (remaining >= Bytes.SIZEOF_INT) {
// The int read has to be converted to unsigned long so the & op
i = (blockBuffer.getIntAfterPosition(offsetFromPos) &
0xL);
remaining -= Bytes.SIZEOF_INT;
offsetFromPos += Bytes.SIZEOF_INT;
}
if (remaining >= Bytes.SIZEOF_SHORT) {
short s = blockBuffer.getShortAfterPosition(offsetFromPos);
i = i << 16;
i = i | (s & 0x);
remaining -= Bytes.SIZEOF_SHORT;
offsetFromPos += Bytes.SIZEOF_SHORT;
}
for (int idx = 0; idx < remaining; idx++) {
byte b = blockBuffer.getByteAfterPosition(offsetFromPos + idx);
i = i << 8;
i = i | (b & 0xFF);
}
currMemstoreTS = (WritableUtils.isNegativeVInt(firstByte) ? ~i : i);
}
return currMemstoreTS;
}
{code}
And I passed the byteBuff I got from
{code}
ByteArrayDataOutput out = ByteStreams.newDataOutput();
WritableUtils.writeVLong(out, 3085788160L);
new SingleByteBuff(ByteBuffer.wrap(out.toByteArray()))
{code}
> MVCC DeSerialization bug in the HFileScannerImpl
>
>
> Key: HBASE-16624
> URL: https://issues.apache.org/jira/browse/HBASE-16624
> Project: HBase
> Issue Type: Bug
> Components: HFile
>Affects Versions: 2.0.0
>Reporter: deepankar
>Assignee: deepankar
>Priority: Blocker
> Attachments: HBASE-16624.patch
>
>
> My colleague [~naggarwal] found a bug in the deserialization of mvcc from
> HFile, As a part of the optimization of deserialization of VLong, we read a
> int at once but we forgot to convert it to unsigned one.
> This would cause issues because once we cross the integer threshold in
> sequenceId and a compaction happens we would write MAX_MEMSTORE_TS in the
> trailer as 0 (because we will be reading negative values from the file that
> got flushed with sequenceId > Integer.MAX_VALUE). And once we have
> MAX_MEMSTORE_TS as 0, and there are sequenceId values present alongside with
> KeyValues the regionserver will now start failing to read the compacted file
> and thus corruption.
> Interestingly this would happen only on the tables that don't have
> DataBlockEncoding enabled and unfortunately in our case that turned out to be
> META and a another small table.
> Fix is small (~20 chars) and attached
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (HBASE-16624) MVCC DeSerialization bug in the HFileScannerImpl
[ https://issues.apache.org/jira/browse/HBASE-16624?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15486425#comment-15486425 ] Duo Zhang commented on HBASE-16624: --- +1 on the fix. Can we prepare a testcase for this? Signed/unsigned cast is always an error-prone part, we should make sure this won't happen again... Thanks. > MVCC DeSerialization bug in the HFileScannerImpl > > > Key: HBASE-16624 > URL: https://issues.apache.org/jira/browse/HBASE-16624 > Project: HBase > Issue Type: Bug > Components: HFile >Affects Versions: 2.0.0 >Reporter: deepankar >Assignee: deepankar >Priority: Blocker > Attachments: HBASE-16624.patch > > > My colleague [~naggarwal] found a bug in the deserialization of mvcc from > HFile, As a part of the optimization of deserialization of VLong, we read a > int at once but we forgot to convert it to unsigned one. > This would cause issues because once we cross the integer threshold in > sequenceId and a compaction happens we would write MAX_MEMSTORE_TS in the > trailer as 0 (because we will be reading negative values from the file that > got flushed with sequenceId > Integer.MAX_VALUE). And once we have > MAX_MEMSTORE_TS as 0, and there are sequenceId values present alongside with > KeyValues the regionserver will now start failing to read the compacted file > and thus corruption. > Interestingly this would happen only on the tables that don't have > DataBlockEncoding enabled and unfortunately in our case that turned out to be > META and a another small table. > Fix is small (~20 chars) and attached -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (HBASE-16624) MVCC DeSerialization bug in the HFileScannerImpl
[ https://issues.apache.org/jira/browse/HBASE-16624?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15486411#comment-15486411 ] Anoop Sam John commented on HBASE-16624: A small UT to show the issue and fix pls? > MVCC DeSerialization bug in the HFileScannerImpl > > > Key: HBASE-16624 > URL: https://issues.apache.org/jira/browse/HBASE-16624 > Project: HBase > Issue Type: Bug > Components: HFile >Affects Versions: 2.0.0 >Reporter: deepankar >Assignee: deepankar >Priority: Blocker > Attachments: HBASE-16624.patch > > > My colleague [~naggarwal] found a bug in the deserialization of mvcc from > HFile, As a part of the optimization of deserialization of VLong, we read a > int at once but we forgot to convert it to unsigned one. > This would cause issues because once we cross the integer threshold in > sequenceId and a compaction happens we would write MAX_MEMSTORE_TS in the > trailer as 0 (because we will be reading negative values from the file that > got flushed with sequenceId > Integer.MAX_VALUE). And once we have > MAX_MEMSTORE_TS as 0, and there are sequenceId values present alongside with > KeyValues the regionserver will now start failing to read the compacted file > and thus corruption. > Interestingly this would happen only on the tables that don't have > DataBlockEncoding enabled and unfortunately in our case that turned out to be > META and a another small table. > Fix is small (~20 chars) and attached -- This message was sent by Atlassian JIRA (v6.3.4#6332)
