huaxiangsun commented on a change in pull request #4014:
URL: https://github.com/apache/hbase/pull/4014#discussion_r781611698
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/HRegionLocator.java
##########
@@ -62,52 +70,57 @@ public void close() throws IOException {
@Override
public HRegionLocation getRegionLocation(byte[] row, int replicaId, boolean
reload)
throws IOException {
- return TraceUtil.trace(() -> connection.locateRegion(tableName, row,
!reload, true, replicaId)
- .getRegionLocation(replicaId), () -> TraceUtil
- .createTableSpan(getClass().getSimpleName() + ".getRegionLocation",
tableName));
+ final Supplier<Span> supplier = new TableSpanBuilder(connection)
+ .setName("HRegionLocator.getRegionLocation")
+ .setTableName(tableName);
+ return tracedLocationFuture(
+ () -> connection.locateRegion(tableName, row, !reload, true, replicaId)
+ .getRegionLocation(replicaId),
+ AsyncRegionLocator::getRegionNames,
+ supplier);
}
@Override
public List<HRegionLocation> getRegionLocations(byte[] row, boolean reload)
throws IOException {
- return TraceUtil.trace(() -> {
- RegionLocations locs =
- connection.locateRegion(tableName, row, !reload, true,
RegionInfo.DEFAULT_REPLICA_ID);
- return Arrays.asList(locs.getRegionLocations());
- }, () -> TraceUtil
- .createTableSpan(getClass().getSimpleName() + ".getRegionLocations",
tableName));
+ final Supplier<Span> supplier = new TableSpanBuilder(connection)
+ .setName("HRegionLocator.getRegionLocations")
+ .setTableName(tableName);
+ final RegionLocations locs = tracedLocationFuture(
+ () -> connection.locateRegion(tableName, row, !reload, true,
+ RegionInfo.DEFAULT_REPLICA_ID),
+ AsyncRegionLocator::getRegionNames, supplier);
+ return Arrays.asList(locs.getRegionLocations());
}
@Override
public List<HRegionLocation> getAllRegionLocations() throws IOException {
- return TraceUtil.trace(() -> {
- ArrayList<HRegionLocation> regions = new ArrayList<>();
- for (RegionLocations locations : listRegionLocations()) {
- for (HRegionLocation location : locations.getRegionLocations()) {
- regions.add(location);
- }
- connection.cacheLocation(tableName, locations);
- }
- return regions;
- }, () -> TraceUtil
- .createTableSpan(getClass().getSimpleName() + ".getAllRegionLocations",
tableName));
+ final Supplier<Span> supplier = new TableSpanBuilder(connection)
+ .setName("HRegionLocator.getAllRegionLocations")
+ .setTableName(tableName);
+ final RegionLocations locs = tracedLocationFuture(() -> {
+ final RegionLocations rlocs = listRegionLocations();
+ connection.cacheLocation(tableName, rlocs);
+ return rlocs;
+ }, AsyncRegionLocator::getRegionNames, supplier);
+ return Arrays.asList(locs.getRegionLocations());
}
@Override
public void clearRegionLocationCache() {
- TraceUtil.trace(() ->
- connection.clearRegionCache(tableName), () -> TraceUtil
- .createTableSpan(this.getClass().getSimpleName() +
".clearRegionLocationCache", tableName));
+ final Supplier<Span> supplier = new TableSpanBuilder(connection)
+ .setName("HRegionLocator.clearRegionLocationCache")
+ .setTableName(tableName);
+ TraceUtil.trace(() -> connection.clearRegionCache(tableName), supplier);
}
@Override
public TableName getName() {
return this.tableName;
}
- private List<RegionLocations> listRegionLocations() throws IOException {
Review comment:
This change does not seem logically correct. RegionLocations is for one
region, there is a start key within it so it will be used as part of the key in
meta cache. HRegionLocation[] inside RegionLocations is for replica regions.
For an example, if a table is defined to have 3 replicas (replica_id 0, 1, 2),
HRegionLocation[] is array with 3 elements, each maps replica's location.
--
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]
