virajjasani commented on a change in pull request #1869:
URL: https://github.com/apache/hbase/pull/1869#discussion_r439246562
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##########
@@ -1285,7 +1285,9 @@ private boolean waitForNamespaceOnline() throws
InterruptedException, IOExceptio
}
// Else there are namespace regions up in meta. Ensure they are assigned
before we go on.
for (RegionInfo ri : ris) {
- isRegionOnline(ri);
+ if (!isRegionOnline(ri)) {
Review comment:
Oh I see, for infinite retries, it is not a problem but then ideally we
should change the signature of `isRegionOnline()` because boolean is of no use.
Also, it is being used in similar manner for meta region also, and that's why I
wanted to make namespace region open logic look same.
e.g
```
public boolean waitForMetaOnline() throws InterruptedException {
return isRegionOnline(RegionInfoBuilder.FIRST_META_REGIONINFO);
}
```
However, now I understand the main purpose of `isRegionOnline()` which is to
keep retrying infinite times and only when server is going down, return false.
And with the current logic, `waitForNamespaceOnline()` returns true no matter
what `isRegionOnline()` returns and hence, instead of returning from HMaster
active initializations, it proceeds further.
```
if (!waitForNamespaceOnline()) {
return;
}
status.setStatus("Starting cluster schema service");
initClusterSchemaService();
....
....
....
```
We should return from the above if condition ideally. Good to maintain same
logic for namespace and meta regions initializations. Thought?
----------------------------------------------------------------
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]
