This is an automated email from the ASF dual-hosted git repository.
charlesconnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/master by this push:
new c9fa892afbd HBASE-30332 Preserve QueryMetrics for empty results (#8555)
c9fa892afbd is described below
commit c9fa892afbd24683640f59afa4aa05bb701482bf
Author: Ma Zhengxuan <[email protected]>
AuthorDate: Tue Aug 25 06:47:56 2026 +0800
HBASE-30332 Preserve QueryMetrics for empty results (#8555)
Co-authored-by: mazhengxuan <[email protected]>
Signed-off by: Peng Lu <[email protected]>
Signed-off by: Charles Connell <[email protected]>
---
.../hadoop/hbase/shaded/protobuf/ProtobufUtil.java | 55 +++++++++++++++++-----
.../hbase/shaded/protobuf/TestProtobufUtil.java | 38 +++++++++++++++
.../hadoop/hbase/regionserver/RSRpcServices.java | 4 +-
.../hbase/client/TestAsyncTableQueryMetrics.java | 19 ++++++++
4 files changed, 102 insertions(+), 14 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 60175137ad2..fab8436ab12 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 final class ProtobufUtil {
*/
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 final class ProtobufUtil {
}
}
+ /**
+ * 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 final class ProtobufUtil {
* @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 final class ProtobufUtil {
*/
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 final class ProtobufUtil {
}
List<CellProtos.Cell> values = proto.getCellList();
- if (values.isEmpty()) {
+ if (values.isEmpty() && !proto.hasMetrics()) {
return proto.getStale() ? EMPTY_RESULT_STALE : EMPTY_RESULT;
}
@@ -1539,10 +1568,7 @@ public final class ProtobufUtil {
) {
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 final class ProtobufUtil {
}
}
- 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 6326bf7fd77..4bad7faf1a0 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 @@ package org.apache.hadoop.hbase.shaded.protobuf;
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.Delete;
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 class TestProtobufUtil {
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.<ExtendedCell>
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.<ExtendedCell>
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 ba838e2f16c..39968843c4a 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 class RSRpcServices extends
HBaseRpcServicesBase<HRegionServer>
}
}
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 72591c19d79..7d79d94fe53 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 class TestAsyncTableQueryMetrics {
}
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<Result> future : futures) {
+ result = future.join();
+ assertEquals(Boolean.TRUE, result.getExists());
+ assertNotNull(result.getMetrics());
+ bbs += result.getMetrics().getBlockBytesScanned();
+ }
+ assertEquals(getClusterBlockBytesScanned(), bbs);
}
@Test
