anmolnar commented on code in PR #7474:
URL: https://github.com/apache/hbase/pull/7474#discussion_r2611855209
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -3116,16 +3116,9 @@ public ClusterMetrics
getClusterMetricsWithoutCoprocessor(EnumSet<Option> option
if (isActiveMaster() && isInitialized() && assignmentManager !=
null) {
try {
Map<TableName, RegionStatesCount> tableRegionStatesCountMap =
new HashMap<>();
- Map<String, TableDescriptor> tableDescriptorMap =
getTableDescriptors().getAll();
- for (TableDescriptor tableDescriptor :
tableDescriptorMap.values()) {
+ List<TableDescriptor> tableDescriptors =
listTableDescriptors(null, null, null, true);
+ for (TableDescriptor tableDescriptor : tableDescriptors) {
Review Comment:
To my understanding this logic has been changed to work only with tables
which are present in replica cluster's meta table.
Previously it used `HBaseServerBase.getTableDescriptors().getAll()` which
eventually calls `FSTableDescriptors.getAll()` method which loads all table
from the filesystem. (only once, because it caches the result)
Now, in this patch, we call `HMaster.listTableDescriptors()` which ends up
calling the same `FSTableDescriptors.getAll()`, but before returning the list
it, filters for the tablenames which are present in replica cluster's meta
table.
```java
for (TableDescriptor desc : allHtds) {
if (
tableStateManager.isTablePresent(desc.getTableName())
&& (includeSysTables || !desc.getTableName().isSystemTable())
) {
htds.add(desc);
}
}
```
I wonder why don't we just get the list directly from meta, but either way
the patch looks good to me.
--
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]
