From eb2862ed92588fb95353600594f0a68ad989e806 Mon Sep 17 00:00:00 2001 From: Bryan Beaudreault Date: Thu, 27 Jan 2022 09:33:11 -0500 Subject: [PATCH] HBASE-26713 Default to LATEST_TIMESTAMP if no timestamp sent along on Increment/Append --- .../hbase/shaded/protobuf/ProtobufUtil.java | 15 ++- .../shaded/protobuf/TestProtobufUtil.java | 103 +++++++++++++----- 2 files changed, 83 insertions(+), 35 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java index 03e2923b114a..9d853d0e4bac 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java @@ -836,10 +836,7 @@ public static Delete toDelete(final MutationProto proto, final CellScanner cellS if (qv.hasQualifier()) { qualifier = qv.getQualifier().toByteArray(); } - long ts = HConstants.LATEST_TIMESTAMP; - if (qv.hasTimestamp()) { - ts = qv.getTimestamp(); - } + long ts = cellTimestampOrLatest(qv); if (deleteType == DeleteType.DELETE_ONE_VERSION) { delete.addColumn(family, qualifier, ts); } else if (deleteType == DeleteType.DELETE_MULTIPLE_VERSIONS) { @@ -906,7 +903,7 @@ private static T toDelta(Function supplier, Consu .setRow(mutation.getRow()) .setFamily(family) .setQualifier(qualifier) - .setTimestamp(qv.getTimestamp()) + .setTimestamp(cellTimestampOrLatest(qv)) .setType(KeyValue.Type.Put.getCode()) .setValue(value) .setTags(tags) @@ -921,6 +918,14 @@ private static T toDelta(Function supplier, Consu return mutation; } + private static long cellTimestampOrLatest(QualifierValue cell) { + if (cell.hasTimestamp()) { + return cell.getTimestamp(); + } else { + return HConstants.LATEST_TIMESTAMP; + } + } + /** * Convert a protocol buffer Mutate to an Append * @param cellScanner diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/shaded/protobuf/TestProtobufUtil.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/shaded/protobuf/TestProtobufUtil.java index c47150b04858..317dff9efebc 100644 --- a/hbase-client/src/test/java/org/apache/hadoop/hbase/shaded/protobuf/TestProtobufUtil.java +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/shaded/protobuf/TestProtobufUtil.java @@ -33,6 +33,7 @@ import org.apache.hadoop.hbase.ExtendedCellBuilder; import org.apache.hadoop.hbase.ExtendedCellBuilderFactory; import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.KeyValue; import org.apache.hadoop.hbase.PrivateCellUtil; import org.apache.hadoop.hbase.Tag; @@ -294,7 +295,22 @@ public void testToCell() { */ @Test public void testIncrement() throws IOException { - long timeStamp = 111111; + + MutationProto proto = getIncrementMutation(111111L); + // default fields + assertEquals(MutationProto.Durability.USE_DEFAULT, proto.getDurability()); + + // set the default value for equal comparison + MutationProto.Builder mutateBuilder = MutationProto.newBuilder(proto); + mutateBuilder.setDurability(MutationProto.Durability.USE_DEFAULT); + + Increment increment = ProtobufUtil.toIncrement(proto, null); + mutateBuilder.setTimestamp(increment.getTimestamp()); + mutateBuilder.setTimeRange(ProtobufUtil.toTimeRange(increment.getTimeRange())); + assertEquals(mutateBuilder.build(), ProtobufUtil.toMutation(MutationType.INCREMENT, increment)); + } + + private MutationProto getIncrementMutation(Long timestamp) { MutationProto.Builder mutateBuilder = MutationProto.newBuilder(); mutateBuilder.setRow(ByteString.copyFromUtf8("row")); mutateBuilder.setMutateType(MutationProto.MutationType.INCREMENT); @@ -303,66 +319,93 @@ public void testIncrement() throws IOException { QualifierValue.Builder qualifierBuilder = QualifierValue.newBuilder(); qualifierBuilder.setQualifier(ByteString.copyFromUtf8("c1")); qualifierBuilder.setValue(ByteString.copyFrom(Bytes.toBytes(11L))); - qualifierBuilder.setTimestamp(timeStamp); + + if (timestamp != null) { + qualifierBuilder.setTimestamp(timestamp); + } + valueBuilder.addQualifierValue(qualifierBuilder.build()); qualifierBuilder.setQualifier(ByteString.copyFromUtf8("c2")); qualifierBuilder.setValue(ByteString.copyFrom(Bytes.toBytes(22L))); valueBuilder.addQualifierValue(qualifierBuilder.build()); mutateBuilder.addColumnValue(valueBuilder.build()); - MutationProto proto = mutateBuilder.build(); + return mutateBuilder.build(); + } + + /** + * Older clients may not send along a timestamp in the MutationProto. Check that we + * default correctly. + */ + @Test + public void testIncrementNoTimestamp() throws IOException { + MutationProto mutation = getIncrementMutation(null); + Increment increment = ProtobufUtil.toIncrement(mutation, null); + assertEquals(HConstants.LATEST_TIMESTAMP, increment.getTimestamp()); + increment.getFamilyCellMap().values() + .forEach(cells -> + cells.forEach(cell -> + assertEquals(HConstants.LATEST_TIMESTAMP, cell.getTimestamp()))); + } + + /** + * Test Append Mutate conversions. + * + * @throws IOException if converting to an {@link Append} fails + */ + @Test + public void testAppend() throws IOException { + MutationProto proto = getAppendMutation(111111L); // default fields assertEquals(MutationProto.Durability.USE_DEFAULT, proto.getDurability()); // set the default value for equal comparison - mutateBuilder = MutationProto.newBuilder(proto); + MutationProto.Builder mutateBuilder = MutationProto.newBuilder(proto); mutateBuilder.setDurability(MutationProto.Durability.USE_DEFAULT); - Increment increment = ProtobufUtil.toIncrement(proto, null); - mutateBuilder.setTimestamp(increment.getTimestamp()); - mutateBuilder.setTimeRange(ProtobufUtil.toTimeRange(increment.getTimeRange())); - assertEquals(mutateBuilder.build(), ProtobufUtil.toMutation(MutationType.INCREMENT, increment)); + Append append = ProtobufUtil.toAppend(proto, null); + + // append always use the latest timestamp, + // reset the timestamp to the original mutate + mutateBuilder.setTimestamp(append.getTimestamp()); + mutateBuilder.setTimeRange(ProtobufUtil.toTimeRange(append.getTimeRange())); + assertEquals(mutateBuilder.build(), ProtobufUtil.toMutation(MutationType.APPEND, append)); } /** - * Test Append Mutate conversions. - * - * @throws IOException if converting to an {@link Append} fails + * Older clients may not send along a timestamp in the MutationProto. Check that we + * default correctly. */ @Test - public void testAppend() throws IOException { - long timeStamp = 111111; + public void testAppendNoTimestamp() throws IOException { + MutationProto mutation = getAppendMutation(null); + Append append = ProtobufUtil.toAppend(mutation, null); + assertEquals(HConstants.LATEST_TIMESTAMP, append.getTimestamp()); + append.getFamilyCellMap().values().forEach(cells -> cells.forEach(cell -> assertEquals(HConstants.LATEST_TIMESTAMP, cell.getTimestamp()))); + } + + private MutationProto getAppendMutation(Long timestamp) { MutationProto.Builder mutateBuilder = MutationProto.newBuilder(); mutateBuilder.setRow(ByteString.copyFromUtf8("row")); mutateBuilder.setMutateType(MutationType.APPEND); - mutateBuilder.setTimestamp(timeStamp); + if (timestamp != null) { + mutateBuilder.setTimestamp(timestamp); + } ColumnValue.Builder valueBuilder = ColumnValue.newBuilder(); valueBuilder.setFamily(ByteString.copyFromUtf8("f1")); QualifierValue.Builder qualifierBuilder = QualifierValue.newBuilder(); qualifierBuilder.setQualifier(ByteString.copyFromUtf8("c1")); qualifierBuilder.setValue(ByteString.copyFromUtf8("v1")); - qualifierBuilder.setTimestamp(timeStamp); + if (timestamp != null) { + qualifierBuilder.setTimestamp(timestamp); + } valueBuilder.addQualifierValue(qualifierBuilder.build()); qualifierBuilder.setQualifier(ByteString.copyFromUtf8("c2")); qualifierBuilder.setValue(ByteString.copyFromUtf8("v2")); valueBuilder.addQualifierValue(qualifierBuilder.build()); mutateBuilder.addColumnValue(valueBuilder.build()); - MutationProto proto = mutateBuilder.build(); - // default fields - assertEquals(MutationProto.Durability.USE_DEFAULT, proto.getDurability()); - - // set the default value for equal comparison - mutateBuilder = MutationProto.newBuilder(proto); - mutateBuilder.setDurability(MutationProto.Durability.USE_DEFAULT); - - Append append = ProtobufUtil.toAppend(proto, null); - - // append always use the latest timestamp, - // reset the timestamp to the original mutate - mutateBuilder.setTimestamp(append.getTimestamp()); - mutateBuilder.setTimeRange(ProtobufUtil.toTimeRange(append.getTimeRange())); - assertEquals(mutateBuilder.build(), ProtobufUtil.toMutation(MutationType.APPEND, append)); + return mutateBuilder.build(); } private static ProcedureProtos.Procedure.Builder createProcedureBuilder(long procId) {