bbeaudreault commented on code in PR #5228:
URL: https://github.com/apache/hbase/pull/5228#discussion_r1293383761
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/RequestConverter.java:
##########
@@ -346,10 +346,9 @@ public static RegionAction.Builder
getRegionActionBuilderWithRegion(
public static ScanRequest buildScanRequest(byte[] regionName, Scan scan, int
numberOfRows,
boolean closeScanner) throws IOException {
ScanRequest.Builder builder = ScanRequest.newBuilder();
- RegionSpecifier region =
buildRegionSpecifier(RegionSpecifierType.REGION_NAME, regionName);
+ builder.setRegion(buildRegionSpecifier(RegionSpecifierType.REGION_NAME,
regionName));
Review Comment:
Thanks for explaining.
I'm not sure we should modify the RPC protocol just for the sake of these
client side metrics. It might be better to use HBaseRpcController to shuttle
the table name into the `onCallFinished` method (where `updateRpc` is called).
As an example, you could modify AsyncScanSingleRegionRpcRetryingCaller.call
method:
```diff
- resetController(controller, callTimeoutNs, priority);
+ resetController(controller, callTimeoutNs, priority,
loc.getRegion().getTable());
ScanRequest req = RequestConverter.buildScanRequest(scannerId,
scan.getCaching(), false,
nextCallSeq, scan.isScanMetricsEnabled(), false, scan.getLimit());
final Context context = Context.current();
stub.scan(controller, req, resp -> {
try (Scope ignored = context.makeCurrent()) {
onComplete(controller, resp);
}
});
```
This requires a bit more changes, but at least it keeps our RPC protocol
unchanged.
You'll need to add a setTableName and getTableName to HBaseRpcController and
DelegatingHBaseRpcController. Then update that
`ConnectionUtils.resetController` method (and callers), as well as
RegionServerCallable once on branch-2.
@Apache9 if you have time, do you agree with this advice?
--
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]
