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 60175137ad2c..fab8436ab12c 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 @@ -1423,12 +1423,13 @@ public static ClientProtos.Result toResult(final Result result) { */ public static ClientProtos.Result toResult(final Result result, boolean encodeTags) { if (result.getExists() != null) { - return toResult(result.getExists(), result.isStale()); + return toResult(result.getExists(), result.isStale(), result.getMetrics()); } ExtendedCell[] cells = ClientInternalHelper.getExtendedRawCells(result); if (cells == null || cells.length == 0) { - return result.isStale() ? EMPTY_RESULT_PB_STALE : EMPTY_RESULT_PB; + return withMetrics(result.isStale() ? EMPTY_RESULT_PB_STALE : EMPTY_RESULT_PB, + result.getMetrics()); } ClientProtos.Result.Builder builder = ClientProtos.Result.newBuilder(); @@ -1459,6 +1460,24 @@ public static ClientProtos.Result toResult(final boolean existence, boolean stal } } + /** + * Convert a client Result to a protocol buffer Result + * @param existence the client existence to send + * @param stale whether the result is stale + * @param metrics query metrics associated with the result + * @return the converted protocol buffer Result + */ + public static ClientProtos.Result toResult(final boolean existence, boolean stale, + QueryMetrics metrics) { + return withMetrics(toResult(existence, stale), metrics); + } + + private static ClientProtos.Result withMetrics(ClientProtos.Result result, QueryMetrics metrics) { + return metrics == null + ? result + : result.toBuilder().setMetrics(toQueryMetrics(metrics)).build(); + } + /** * Convert a client Result to a protocol buffer Result. The pb Result does not include the Cell * data. That is for transport otherwise. @@ -1466,9 +1485,14 @@ public static ClientProtos.Result toResult(final boolean existence, boolean stal * @return the converted protocol buffer Result */ public static ClientProtos.Result toResultNoData(final Result result) { - if (result.getExists() != null) return toResult(result.getExists(), result.isStale()); + if (result.getExists() != null) { + return toResult(result.getExists(), result.isStale(), result.getMetrics()); + } int size = result.size(); - if (size == 0) return result.isStale() ? EMPTY_RESULT_PB_STALE : EMPTY_RESULT_PB; + if (size == 0) { + return withMetrics(result.isStale() ? EMPTY_RESULT_PB_STALE : EMPTY_RESULT_PB, + result.getMetrics()); + } ClientProtos.Result.Builder builder = ClientProtos.Result.newBuilder(); builder.setAssociatedCellCount(size); builder.setStale(result.isStale()); @@ -1499,6 +1523,11 @@ public static Result toResult(final ClientProtos.Result proto) { */ public static Result toResult(final ClientProtos.Result proto, boolean decodeTags) { if (proto.hasExists()) { + if (proto.hasMetrics()) { + Result result = Result.create((Cell[]) null, proto.getExists(), proto.getStale()); + result.setMetrics(toQueryMetrics(proto.getMetrics())); + return result; + } if (proto.getStale()) { return proto.getExists() ? EMPTY_RESULT_EXISTS_TRUE_STALE : EMPTY_RESULT_EXISTS_FALSE_STALE; } @@ -1506,7 +1535,7 @@ public static Result toResult(final ClientProtos.Result proto, boolean decodeTag } List values = proto.getCellList(); - if (values.isEmpty()) { + if (values.isEmpty() && !proto.hasMetrics()) { return proto.getStale() ? EMPTY_RESULT_STALE : EMPTY_RESULT; } @@ -1539,10 +1568,7 @@ public static Result toResult(final ClientProtos.Result proto, final CellScanner ) { throw new IllegalArgumentException("bad proto: exists with cells is no allowed " + proto); } - if (proto.getStale()) { - return proto.getExists() ? EMPTY_RESULT_EXISTS_TRUE_STALE : EMPTY_RESULT_EXISTS_FALSE_STALE; - } - return proto.getExists() ? EMPTY_RESULT_EXISTS_TRUE : EMPTY_RESULT_EXISTS_FALSE; + return toResult(proto); } // TODO: Unit test that has some Cells in scanner and some in the proto. @@ -1564,9 +1590,14 @@ public static Result toResult(final ClientProtos.Result proto, final CellScanner } } - Result r = (cells == null || cells.isEmpty()) - ? (proto.getStale() ? EMPTY_RESULT_STALE : EMPTY_RESULT) - : Result.create(cells, null, proto.getStale()); + Result r; + if (cells == null || cells.isEmpty()) { + r = proto.hasMetrics() + ? Result.create(EMPTY_CELL_ARRAY, null, proto.getStale()) + : (proto.getStale() ? EMPTY_RESULT_STALE : EMPTY_RESULT); + } else { + r = Result.create(cells, null, proto.getStale()); + } if (proto.hasMetrics()) { r.setMetrics(toQueryMetrics(proto.getMetrics())); 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 6326bf7fd77e..4bad7faf1a0c 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 @@ -19,11 +19,13 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; import java.lang.reflect.Method; import java.nio.ByteBuffer; +import java.util.Arrays; import java.util.Collections; import java.util.List; import org.apache.hadoop.hbase.ArrayBackedTag; @@ -43,6 +45,8 @@ import org.apache.hadoop.hbase.client.Get; import org.apache.hadoop.hbase.client.Increment; import org.apache.hadoop.hbase.client.Put; +import org.apache.hadoop.hbase.client.QueryMetrics; +import org.apache.hadoop.hbase.client.Result; import org.apache.hadoop.hbase.client.SlowLogParams; import org.apache.hadoop.hbase.io.TimeRange; import org.apache.hadoop.hbase.testclassification.SmallTests; @@ -132,6 +136,40 @@ public void testGet() throws IOException { assertEquals(getBuilder.build(), ProtobufUtil.toGet(get)); } + @Test + public void testEmptyResultWithQueryMetrics() throws IOException { + long blockBytesScanned = 123L; + for (Boolean exists : Arrays.asList(null, false, true)) { + Result result = Result.create(Collections.emptyList(), exists); + result.setMetrics(new QueryMetrics(blockBytesScanned)); + + for (ClientProtos.Result proto : List.of(ProtobufUtil.toResult(result), + ProtobufUtil.toResultNoData(result))) { + assertEquals(exists, proto.hasExists() ? proto.getExists() : null); + assertTrue(proto.hasMetrics()); + assertEquals(blockBytesScanned, proto.getMetrics().getBlockBytesScanned()); + + Result roundTrip = ProtobufUtil.toResult(proto); + assertEquals(exists, roundTrip.getExists()); + assertNotNull(roundTrip.getMetrics()); + assertEquals(blockBytesScanned, roundTrip.getMetrics().getBlockBytesScanned()); + + roundTrip = ProtobufUtil.toResult(proto, + PrivateCellUtil.createExtendedCellScanner(Collections. emptyList())); + assertEquals(exists, roundTrip.getExists()); + assertNotNull(roundTrip.getMetrics()); + assertEquals(blockBytesScanned, roundTrip.getMetrics().getBlockBytesScanned()); + } + } + + ClientProtos.Result emptyProto = ClientProtos.Result.getDefaultInstance(); + assertNull(ProtobufUtil.toResult(emptyProto).getMetrics()); + assertNull(ProtobufUtil + .toResult(emptyProto, + PrivateCellUtil.createExtendedCellScanner(Collections. emptyList())) + .getMetrics()); + } + /** * Test Delete Mutate conversions. * @throws IOException if the conversion to a {@link Delete} or a diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java index ba838e2f16ca..39968843c4ab 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java @@ -2535,8 +2535,8 @@ public GetResponse get(final RpcController controller, final GetRequest request) } } if (existence != null) { - ClientProtos.Result pbr = - ProtobufUtil.toResult(existence, region.getRegionInfo().getReplicaId() != 0); + ClientProtos.Result pbr = ProtobufUtil.toResult(existence, + region.getRegionInfo().getReplicaId() != 0, r != null ? r.getMetrics() : null); builder.setResult(pbr); } else if (r != null) { ClientProtos.Result pbr; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableQueryMetrics.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableQueryMetrics.java index 72591c19d79b..7d79d94fe53f 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableQueryMetrics.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncTableQueryMetrics.java @@ -108,6 +108,25 @@ public void itTestsGets() throws Exception { } assertEquals(getClusterBlockBytesScanned(), bbs); + + g1.setCheckExistenceOnly(true); + g2.setCheckExistenceOnly(true); + g3.setCheckExistenceOnly(true); + + result = CONN.getTable(TABLE_NAME).get(g1).get(); + assertEquals(Boolean.TRUE, result.getExists()); + assertNotNull(result.getMetrics()); + bbs += result.getMetrics().getBlockBytesScanned(); + assertEquals(getClusterBlockBytesScanned(), bbs); + + futures = CONN.getTable(TABLE_NAME).get(List.of(g1, g2, g3)); + for (CompletableFuture future : futures) { + result = future.join(); + assertEquals(Boolean.TRUE, result.getExists()); + assertNotNull(result.getMetrics()); + bbs += result.getMetrics().getBlockBytesScanned(); + } + assertEquals(getClusterBlockBytesScanned(), bbs); } @Test