saintstack commented on a change in pull request #2640:
URL: https://github.com/apache/hbase/pull/2640#discussion_r520701344
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java
##########
@@ -332,19 +338,59 @@ public void newDead(ServerName sn) {
close();
throw e;
}
+
+ // Get the region locator's meta replica mode.
+ this.metaReplicaMode =
CatalogReplicaMode.fromString(conf.get(LOCATOR_META_REPLICAS_MODE,
+ CatalogReplicaMode.NONE.toString()));
+
+ switch (this.metaReplicaMode) {
+ case LOAD_BALANCE:
+ String replicaSelectorClass = conf.get(
+ RegionLocator.LOCATOR_META_REPLICAS_MODE_LOADBALANCE_SELECTOR,
+ CatalogReplicaLoadBalanceSimpleSelector.class.getName());
+
+ this.metaReplicaSelector =
CatalogReplicaLoadBalanceSelectorFactory.createSelector(
+ replicaSelectorClass, META_TABLE_NAME, getChoreService(), () -> {
+ int numOfReplicas = 1;
+ try {
+ RegionLocations metaLocations =
registry.getMetaRegionLocations().get(
+ connectionConfig.getReadRpcTimeout(), TimeUnit.MILLISECONDS);
+ numOfReplicas = metaLocations.size();
+ } catch (Exception e) {
+ LOG.error("Failed to get table {}'s region replication, ",
META_TABLE_NAME, e);
+ }
+ return numOfReplicas;
+ });
+ break;
+ case NONE:
+ // If user does not configure LOCATOR_META_REPLICAS_MODE, let's check
the legacy config.
+
+ boolean useMetaReplicas = conf.getBoolean(USE_META_REPLICAS,
+ DEFAULT_USE_META_REPLICAS);
+ if (useMetaReplicas) {
+ this.metaReplicaMode = CatalogReplicaMode.HEDGED_READ;
+ }
+ break;
+ default:
+ // Doing nothing
+ }
Review comment:
Is this code duplicated? Does it have to be?
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java
##########
@@ -841,8 +898,23 @@ private RegionLocations locateRegionInMeta(TableName
tableName, byte[] row, bool
Scan s = new Scan().withStartRow(metaStartKey).withStopRow(metaStopKey,
true)
.addFamily(HConstants.CATALOG_FAMILY).setReversed(true).setCaching(5)
.setReadType(ReadType.PREAD);
- if (this.useMetaReplicas) {
- s.setConsistency(Consistency.TIMELINE);
+
+ switch (this.metaReplicaMode) {
+ case LOAD_BALANCE:
+ int metaReplicaId = this.metaReplicaSelector.select(tableName, row,
+ RegionLocateType.CURRENT);
+ if (metaReplicaId != RegionInfo.DEFAULT_REPLICA_ID) {
+ // If the selector gives a non-primary meta replica region, then go
with it.
+ // Otherwise, just go to primary in non-hedgedRead mode.
+ s.setConsistency(Consistency.TIMELINE);
+ s.setReplicaId(metaReplicaId);
+ }
+ break;
+ case HEDGED_READ:
+ s.setConsistency(Consistency.TIMELINE);
+ break;
+ default:
+ // do nothing
Review comment:
Yeah, this a dupe too?
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java
##########
@@ -332,19 +338,59 @@ public void newDead(ServerName sn) {
close();
throw e;
}
+
+ // Get the region locator's meta replica mode.
+ this.metaReplicaMode =
CatalogReplicaMode.fromString(conf.get(LOCATOR_META_REPLICAS_MODE,
+ CatalogReplicaMode.NONE.toString()));
+
+ switch (this.metaReplicaMode) {
+ case LOAD_BALANCE:
+ String replicaSelectorClass = conf.get(
+ RegionLocator.LOCATOR_META_REPLICAS_MODE_LOADBALANCE_SELECTOR,
+ CatalogReplicaLoadBalanceSimpleSelector.class.getName());
+
+ this.metaReplicaSelector =
CatalogReplicaLoadBalanceSelectorFactory.createSelector(
+ replicaSelectorClass, META_TABLE_NAME, getChoreService(), () -> {
+ int numOfReplicas = 1;
+ try {
+ RegionLocations metaLocations =
registry.getMetaRegionLocations().get(
+ connectionConfig.getReadRpcTimeout(), TimeUnit.MILLISECONDS);
+ numOfReplicas = metaLocations.size();
+ } catch (Exception e) {
+ LOG.error("Failed to get table {}'s region replication, ",
META_TABLE_NAME, e);
+ }
+ return numOfReplicas;
+ });
+ break;
+ case NONE:
+ // If user does not configure LOCATOR_META_REPLICAS_MODE, let's check
the legacy config.
+
+ boolean useMetaReplicas = conf.getBoolean(USE_META_REPLICAS,
+ DEFAULT_USE_META_REPLICAS);
+ if (useMetaReplicas) {
+ this.metaReplicaMode = CatalogReplicaMode.HEDGED_READ;
+ }
+ break;
+ default:
+ // Doing nothing
+ }
}
private void spawnRenewalChore(final UserGroupInformation user) {
- authService = new ChoreService("Relogin service");
- authService.scheduleChore(AuthUtil.getAuthRenewalChore(user));
+ ChoreService service = getChoreService();
+ service.scheduleChore(AuthUtil.getAuthRenewalChore(user));
}
/**
* @param useMetaReplicas
*/
@VisibleForTesting
void setUseMetaReplicas(final boolean useMetaReplicas) {
- this.useMetaReplicas = useMetaReplicas;
Review comment:
I was wondering about the @Apache9 comment above on scope. I was
thinking this a @private class so we should be able to change internals but I
see now this method exposes our internal useMetaReplicas.
Its a weird method though. Package protected for testing only? The test
shouldn't do this? Should change config rather than this method? Probably ok to
add @deprecated.... to be replaced by nothing.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
