sanjeet006py commented on code in PR #7340:
URL: https://github.com/apache/hbase/pull/7340#discussion_r2377620917
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionScannerImpl.java:
##########
@@ -148,16 +145,7 @@ private static boolean hasNonce(HRegion region, long
nonce) {
} finally {
region.smallestReadPointCalcLock.unlock(ReadPointCalculationLock.LockType.RECORDING_LOCK);
}
- boolean isScanMetricsEnabled = scan.isScanMetricsEnabled();
-
ThreadLocalServerSideScanMetrics.setScanMetricsEnabled(isScanMetricsEnabled);
- if (isScanMetricsEnabled) {
- this.scannerInitMetrics = new ServerSideScanMetrics();
- ThreadLocalServerSideScanMetrics.reset();
- }
Review Comment:
The removed code in RegionScannerImpl was doing two things:
- Capturing server side scan metrics even when RegionScanner is initialized
and passing it to the `next()` call in `RegionScannerImpl` to update
`ServerSideScanMetrics` from `ScannerContext`.
- Resetting the state of thread local variable to start fresh for a scan.
For the first point given the call to `RegionScannerImpl` constructor and
`next()` happens from same thread and its the same thread which is handling
`RSRpcServices#scan()` call so, we can avoid passing around
`ServerSideScanMetrics` from constructor to `next()` call if we populate
`ServerSideScanMetrics` in `RSRpcServices` as done for some other scan metrics
already.
For second point, I am taking care of resetting the thread local variables
for each call in `RSRpcServices#scan()` call and its much cleaner also.
So, we are still doing same thing in `RSRpcServices` which we used to do in
`RegionScannerImpl` it's just that its more concise and cleaner now. Please let
me know if I am missing something. Thanks
--
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]
