Copilot commented on code in PR #8416:
URL: https://github.com/apache/hbase/pull/8416#discussion_r3475884439
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestMetaRegionReplicaReplication.java:
##########
@@ -430,38 +449,43 @@ public void testHBaseMetaReplicaGets() throws Exception {
}
}
}
+ }
- getMetaReplicaReadRequests(metaRegions, readReqsForMetaReplicas);
+ getMetaReplicaReadRequests(metaRegions, readReqsForMetaReplicas);
- Configuration c = new Configuration(HTU.getConfiguration());
- c.setBoolean(HConstants.USE_META_REPLICAS, true);
- c.set(LOCATOR_META_REPLICAS_MODE, "LoadBalance");
- Connection connection = ConnectionFactory.createConnection(c);
+ Configuration c = new Configuration(HTU.getConfiguration());
+ c.setBoolean(HConstants.USE_META_REPLICAS, true);
+ c.set(LOCATOR_META_REPLICAS_MODE, "LoadBalance");
+ try (Connection connection = ConnectionFactory.createConnection(c);
Table tableForGet = connection.getTable(tn);
+ RegionLocator locator = tableForGet.getRegionLocator()) {
byte[][] getRows = new byte[HBaseTestingUtil.KEYS.length][];
-
- int i = 0;
- for (byte[] key : HBaseTestingUtil.KEYS) {
- getRows[i] = key;
- i++;
+ for (int i = 1; i < HBaseTestingUtil.KEYS.length; i++) {
+ getRows[i] = HBaseTestingUtil.KEYS[i];
}
getRows[0] = Bytes.toBytes("aaa");
- doNGets(tableForGet, getRows);
-
- getMetaReplicaReadRequests(metaRegions, readReqsForMetaReplicasAfterGet);
// There are more reads against all meta replica regions, including the
primary region.
- primaryIncreaseReplicaIncrease(readReqsForMetaReplicas,
readReqsForMetaReplicasAfterGet);
-
- RegionLocator locator = tableForGet.getRegionLocator();
-
- for (int j = 0; j < numOfMetaReplica * 3; j++) {
- locator.getAllRegionLocations();
- }
-
- getMetaReplicaReadRequests(metaRegions,
readReqsForMetaReplicasAfterGetAllLocations);
- primaryIncreaseReplicaIncrease(readReqsForMetaReplicasAfterGet,
- readReqsForMetaReplicasAfterGetAllLocations);
+ // Since in load balance mode, we will randomly select region replicas,
it is possible that we
+ // missed read some replicas, so here we run it at most 3 times to make
it more stable
+ runAtMostNTimes(3, () -> {
+ locator.clearRegionLocationCache();
+ doNGets(tableForGet, getRows);
+ getMetaReplicaReadRequests(metaRegions,
readReqsForMetaReplicasAfterGet);
+ return null;
+ }, () -> primaryIncreaseReplicaIncrease(readReqsForMetaReplicas,
+ readReqsForMetaReplicasAfterGet));
+
+ // Same as above, we need to run it multiple times to make it stable.
And numOfMetaRelicas is
+ // much less than HBaseTestingUtil.KEYS.length, so here we need to run
more times.
Review Comment:
Typo in comment: "numOfMetaRelicas" should be "numOfMetaReplicas".
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java:
##########
@@ -3344,7 +3344,7 @@ private void addToMovedRegions(String encodedName,
ServerName destination, long
movedRegionInfoCache.put(encodedName, new MovedRegionInfo(destination,
closeSeqNum));
}
- void removeFromMovedRegions(String encodedName) {
+ public void removeFromMovedRegions(String encodedName) {
movedRegionInfoCache.invalidate(encodedName);
}
Review Comment:
removeFromMovedRegions was made public but is not annotated as
internal-only. Adding @InterfaceAudience.Private (like getMovedRegion) helps
prevent treating this as supported API surface.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestMetaRegionReplicaReplication.java:
##########
@@ -398,6 +401,22 @@ private void getMetaReplicaReadRequests(final Region[]
metaRegions, final long[]
}
}
+ private void runAtMostNTimes(int times, Callable<?> action, Runnable
assertion) throws Exception {
+ for (int i = 0; i < times - 1; i++) {
+ action.call();
+ try {
+ assertion.run();
+ // return if the assertion pass, otherwise try again
Review Comment:
Grammar in comment: "pass" should be "passes".
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestMetaRegionReplicaReplication.java:
##########
@@ -398,6 +401,22 @@ private void getMetaReplicaReadRequests(final Region[]
metaRegions, final long[]
}
}
+ private void runAtMostNTimes(int times, Callable<?> action, Runnable
assertion) throws Exception {
+ for (int i = 0; i < times - 1; i++) {
+ action.call();
Review Comment:
runAtMostNTimes does not validate that times is positive, so passing 0 or a
negative value would still execute the action once, contradicting the method
name/contract.
--
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]
