jbewing commented on code in PR #5576:
URL: https://github.com/apache/hbase/pull/5576#discussion_r1433090992
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValueUtil.java:
##########
@@ -226,7 +226,7 @@ public static KeyValue nextShallowCopy(final ByteBuffer bb,
final boolean includ
int kvLength = (int) KeyValue.getKeyValueDataStructureSize(keyLength,
valueLength, tagsLength);
KeyValue keyValue = new KeyValue(bb.array(), underlyingArrayOffset,
kvLength);
if (includesMvccVersion) {
- long mvccVersion = ByteBufferUtils.readVLong(bb);
+ long mvccVersion = ByteBufferUtils.readVLongTimestamp(bb);
Review Comment:
Yeah it looks like mvcc, memstoreTs, and sequenceId are all pretty much used
interchangeably from a quick read. I've dug up a bit of literature on the topic
in [HBASE-13389](https://issues.apache.org/jira/browse/HBASE-13389). To
summarize: MVCC values stick around for a while and the performance hit
associated with that is known. They're not timestamps, but they're also not
likely to be super small.
This was a known issue and the performance impact was previously solved with
[HBASE-14861](https://issues.apache.org/jira/browse/HBASE-14186). That patch
affects only HFiles without a data block encoding applied and works by
attempting to eagerly read 4 bytes, then 2 bytes, and then falling back to
single byte reads as opposed to the approach taken in this PR where we attempt
to eagerly read 8 bytes. It's hard to say which approach is better, but I'm
thinking we should at least apply similar optimizations to data block
encodings.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
