Skip to content

HBASE-28192 Master should recover if meta region state is inconsistent #5513

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
virajjasani wants to merge 2 commits into
base: master
Choose a base branch
from
Open

HBASE-28192 Master should recover if meta region state is inconsistent #5513

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8e0494b
HBASE-28192 Master should recover if meta region state is inconsistent
virajjasani Nov 9, 2023
f3eb95d
addendum
virajjasani Nov 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1399,23 +1399,46 @@ private void createMissingCFsInMetaDuringUpgrade(TableDescriptor metaDescriptor)
* Check hbase:meta is up and ready for reading. For use during Master startup only.
* @return True if meta is UP and online and startup can progress. Otherwise, meta is not online
* and we will hold here until operator intervention.
* @throws IOException If the master restart is required.
*/
@InterfaceAudience.Private
public boolean waitForMetaOnline() {
public boolean waitForMetaOnline() throws IOException {
return isRegionOnline(RegionInfoBuilder.FIRST_META_REGIONINFO);
}

/**
* Wait until the region is reported online on a live regionserver.
* @param ri Region info.
* @return True if region is online and scannable else false if an error or shutdown (Otherwise we
* just block in here holding up all forward-progess).
* @throws IOException If the master restart is required.
*/
private boolean isRegionOnline(RegionInfo ri) {
private boolean isRegionOnline(RegionInfo ri) throws IOException {
RetryCounter rc = null;
while (!isStopped()) {
RegionState rs = this.assignmentManager.getRegionStates().getRegionState(ri);
if (rs != null && rs.isOpened()) {
if (this.getServerManager().isServerOnline(rs.getServerName())) {
return true;
} else {
LOG.info("{} has state {} but the server {} is not online, scheduling recovery.",
ri.getRegionNameAsString(), rs, rs.getServerName());
this.getServerManager().expireServer(rs.getServerName(), true);
// If already many SCPs are scheduled, but they are not progressing because of
// meta's unavailability, the best action item is to throw PleaseRestartMasterException
// and let new active master init take care of on-lining meta and process all other
// pending SCPs. It's worth waiting for ~20s before arriving at the conclusion, rather

@apurtell apurtell Nov 9, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this true? Have we really thought about prioritizing execution of the procedures related to meta and namespace before the others? I agree this could lead to complex code.

@virajjasani virajjasani Nov 9, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added proc state based waiting

// than looping through procedures to figure out how/when/why they are able to or not
// able to make any progress and eventually abort master initialization anyway.
Threads.sleep(20000);

@apurtell apurtell Nov 9, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interrupt is not handled here.
But, more generally, do not unconditionally wait for so long. Use the Waiter pattern. Why 20 seconds and not some other value? Why not make this configurable?

@virajjasani virajjasani Nov 9, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

introduced 2 configs to improve the retry logic

rs = this.assignmentManager.getRegionStates().getRegionState(ri);
if (rs != null && rs.isOpened()) {
if (this.getServerManager().isServerOnline(rs.getServerName())) {
return true;
} else {
throw new PleaseRestartMasterException("meta is still not online on live server yet");

@apurtell apurtell Nov 9, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What you check here, for this particular region, is if it is supposed to be online on a server, that the server is considered online. This does not seem to check anything specifically pertaining to the meta table. If you want to break out of this loop because meta is not available you should make an explicit check of the region state for the meta regions.

Also, this is the same logic as if we would just loop around again, so why not do that? D.R.Y.

Finally, do we really need to restart the master here or is there some other way to address this?

@virajjasani virajjasani Nov 9, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not seem to check anything specifically pertaining to the meta table.

This utility is used by both meta and namespace regions only.

Finally, do we really need to restart the master here or is there some other way to address this?

That's what will guarantee prioritizing SCP of the server where meta was hosted. So we schedule SCP of the server hosting meta and abort master, new master failover is guaranteed to process SCP of that server, and then continue processing other SCPs that were stuck due to unavailable meta (due to inconsistent meta state).

@virajjasani virajjasani Nov 9, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried improving this overall in the latest revision.

}
}
}
}
// Region is not OPEN.
Expand Down
4 changes: 2 additions & 2 deletions hbase-server/src/test/java/org/apache/hadoop/hbase/TestMetaTableAccessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,13 @@ public static void afterClass() throws Exception {
}

@Test
public void testIsMetaWhenAllHealthy() throws InterruptedException {
public void testIsMetaWhenAllHealthy() throws IOException {
HMaster m = UTIL.getMiniHBaseCluster().getMaster();
assertTrue(m.waitForMetaOnline());
}

@Test
public void testIsMetaWhenMetaGoesOffline() throws InterruptedException {
public void testIsMetaWhenMetaGoesOffline() throws IOException {
HMaster m = UTIL.getMiniHBaseCluster().getMaster();
int index = UTIL.getMiniHBaseCluster().getServerWithMeta();
HRegionServer rsWithMeta = UTIL.getMiniHBaseCluster().getRegionServer(index);
Expand Down