HBASE-28951 rename the wal before retrying the wal-split with another worker #6534

Umeshkumar9414 · 2024-12-12T18:55:48Z

Added an workerchange counter so that each time we can have a new name, that is needed in case the supposed dead RS starts to process the WAL after some time. I checked that wal name pattern, that we use for validating the wal is (.+)\.(\d+)(\.[0-9A-Za-z]+)? . This change is fitting there.

Umeshkumar9414 · 2024-12-12T19:16:10Z

While going through the code I saw some comments and code that are not aligning. As per this comment, WALSplitter.splitLogFile should not be getting used in procedure based splitting but we are using the same method. We are calling SplitLogWorker.splitLog(that is deprecated) in SplitWALCallable. And inside this method here, we call WALSplitter.splitLogFile.
Can someone help me understad this? Is this a miss or it is known? cc @Apache9 , @apurtell

mnpoonia · 2024-12-13T05:10:24Z

@Umeshkumar9414 So the idea here is to have a retry counter attached to the wal name. And whenever split wal fails and another worked picks up same wal, it increments the counter!!

mnpoonia · 2024-12-16T05:51:00Z

@Umeshkumar9414 If the the splitwal proc fails and also root procedure fails the how is that handled?

Apache9

In general I think the approach is OK, renaming is a typical way for fencing. But I suggest we keep the old behavior when there is no retry, so we can get better compatibility.

Apache9 · 2024-12-16T06:31:11Z

hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SplitWALProcedure.java

@@ -51,6 +51,7 @@ public class SplitWALProcedure
  private ServerName worker;
  private ServerName crashedServer;
  private RetryCounter retryCounter;
+  private Integer workerChangeCount = 0;


Why Integer not int?

I might have wrongly remembered that becuase of autoboxing and unboxing, Interger is better for performance. I have changed it to int

Apache9 · 2024-12-16T06:33:01Z

hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractFSWALProvider.java

@@ -237,6 +237,8 @@ static void requestLogRoll(final WAL wal) {
  /** File Extension used while splitting an WAL into regions (HBASE-2312) */
  public static final String SPLITTING_EXT = "-splitting";

+  public static final String RETRYING_EXT = ".retrying";


Better add some comments to explain how we use this.

IIRC the aboce splitting suffix is for the wal directory, and this retrying suffix is for wal file? We'd better mention this difference in the javadoc or comment so later developer will not be confused.

Apache9 · 2024-12-16T06:34:21Z

hbase-server/src/main/java/org/apache/hadoop/hbase/master/SplitWALManager.java

+    } else {
+      originalWALPath = walPath.substring(0, walPath.length() - RETRYING_EXT.length() - 3);
+    }
+    String walNewName =


So when retrying number is 0, we also have the '.retrying' suffix? Will this cause trouble when upgrading?

No when we have retrying number (workerChangeCount) 0 we don't have any suffix. This should not cause any trouble in upgrading. As @mnpoonia pointed out I do need to handle one case when SCP rolled back and second SCP created another splitwalProcedure in that case the name will contian retrying suffix.

I do not think the current code reflection your explaination here...

If you do not want to change the wal name when retry count == 0, you should just return at the first if condition?

We call this method in RELEASE_SPLIT_WORKER state. At this time first try of wal split is already complete. We only reach here if fir try is not able to split the wal.

Then please add this comment as a javadoc of this method?

Umeshkumar9414 · 2024-12-16T08:57:18Z

In general I think the approach is OK, renaming is a typical way for fencing. But I suggest we keep the old behavior when there is no retry, so we can get better compatibility.

Yeah I didn't do any changes when there is no retry and kept that as it was.

Umeshkumar9414 · 2024-12-16T08:59:25Z

@Umeshkumar9414 If the the splitwal p

Thanks @mnpoonia to point this out. I need to when the parent SCP fails and lets say we have created another SCP. It will just list all the files in WALDirectory and create SplitWalProcedure for all but yeah I need to handle the first retry with retryCount 0.

Apache9 · 2024-12-17T03:20:28Z

What about add a new step after acquire worker to rename the wal file, where we just append the worker's name to the wal file name as suffix?

And we need to be very careful when dealing with retrying...

There are several problems currently

Renaming failure does not always mean the old file is still there, maybe the renaming is complete but we just do not get the result because of a network issue, then in the current code we will consider the splitting as finished and cause data loss.
We will do the renaming after release worker, if the renaming fails, when retrying we will release the worker again, this will cause trouble...

Umeshkumar9414 · 2024-12-20T11:34:28Z

While going through the code I saw some comments and code that are not aligning. As per this comment, WALSplitter.splitLogFile should not be getting used in procedure based splitting but we are using the same method. We are calling SplitLogWorker.splitLog(that is deprecated) in SplitWALCallable. And inside this method here, we call WALSplitter.splitLogFile. Can someone help me understad this? Is this a miss or it is known? cc @Apache9 , @apurtell

@Apache9 , @apurtell

… worker

…ch time

…uffix

…count retry

Umeshkumar9414 · 2024-12-20T20:16:52Z

hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractFSWALProvider.java

@@ -237,6 +237,9 @@ static void requestLogRoll(final WAL wal) {
  /** File Extension used while splitting an WAL into regions (HBASE-2312) */
  public static final String SPLITTING_EXT = "-splitting";

+  // Extension for the WAL where the split failed on one worker and is being retried on another.
+  public static final String RETRYING_EXT = ".retrying";
+
  /**
   * Pattern used to validate a WAL file name see {@link #validateWALFilename(String)} for
   * description.


while splitting the wal for meta table. wal name can be rs.XXX.meta.retrying001. Do you think we should update the WAL_FILE_NAME_PATTERN. Althought in splitting we didn't check for valid wal name.

Apache-HBase · 2024-12-20T20:24:24Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 27s		Docker mode activated.
-0 ⚠️	yetus	0m 3s		Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck
			_ Prechecks _
			_ master Compile Tests _
+0 🆗	mvndep	0m 10s		Maven dependency ordering for branch
+1 💚	mvninstall	3m 19s		master passed
+1 💚	compile	1m 33s		master passed
+1 💚	javadoc	0m 37s		master passed
+1 💚	shadedjars	5m 56s		branch has no errors when building our shaded downstream artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 12s		Maven dependency ordering for patch
+1 💚	mvninstall	3m 3s		the patch passed
+1 💚	compile	1m 30s		the patch passed
+1 💚	javac	1m 30s		the patch passed
+1 💚	javadoc	0m 37s		the patch passed
+1 💚	shadedjars	5m 53s		patch has no errors when building our shaded downstream artifacts.
			_ Other Tests _
+1 💚	unit	0m 34s		hbase-protocol-shaded in the patch passed.
-1 ❌	unit	12m 30s	/patch-unit-hbase-server.txt	hbase-server in the patch failed.
		38m 9s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6534/4/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR	#6534
Optional Tests	unit javac javadoc compile shadedjars
uname	Linux 10487a26c15e 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `a2f732d`
Default Java	Eclipse Adoptium-17.0.11+9
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6534/4/testReport/
Max. process+thread count	1790 (vs. ulimit of 30000)
modules	C: hbase-protocol-shaded hbase-server U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6534/4/console
versions	git=2.34.1 maven=3.9.8
Powered by	Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2024-12-20T20:27:18Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 39s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	buf	0m 0s		buf was not available.
+0 🆗	buf	0m 0s		buf was not available.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	hbaseanti	0m 0s		Patch does not have any anti-patterns.
			_ master Compile Tests _
+0 🆗	mvndep	0m 10s		Maven dependency ordering for branch
+1 💚	mvninstall	3m 19s		master passed
+1 💚	compile	3m 13s		master passed
+1 💚	checkstyle	0m 42s		master passed
+1 💚	spotbugs	3m 30s		master passed
+1 💚	spotless	0m 41s		branch has no errors when running spotless:check.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 11s		Maven dependency ordering for patch
+1 💚	mvninstall	2m 46s		the patch passed
+1 💚	compile	3m 9s		the patch passed
+1 💚	cc	3m 9s		the patch passed
+1 💚	javac	3m 9s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	0m 33s	/results-checkstyle-hbase-server.txt	hbase-server: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
+1 💚	spotbugs	3m 44s		the patch passed
+1 💚	hadoopcheck	9m 50s		Patch does not cause any errors with Hadoop 3.3.6 3.4.0.
+1 💚	hbaseprotoc	1m 6s		the patch passed
+1 💚	spotless	0m 40s		patch has no errors when running spotless:check.
			_ Other Tests _
+1 💚	asflicense	0m 21s		The patch does not generate ASF License warnings.
		41m 5s

Subsystem	Report/Notes
Docker	ClientAPI=1.47 ServerAPI=1.47 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6534/4/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#6534
Optional Tests	dupname asflicense cc buflint bufcompat codespell detsecrets hbaseprotoc spotless javac spotbugs checkstyle compile hadoopcheck hbaseanti
uname	Linux 2bff36406c2c 5.4.0-200-generic #220-Ubuntu SMP Fri Sep 27 13:19:16 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	master / `a2f732d`
Default Java	Eclipse Adoptium-17.0.11+9
Max. process+thread count	86 (vs. ulimit of 30000)
modules	C: hbase-protocol-shaded hbase-server U: .
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6534/4/console
versions	git=2.34.1 maven=3.9.8 spotbugs=4.7.3
Powered by	Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

virajjasani self-requested a review December 12, 2024 19:01

This comment has been minimized.

Sign in to view

Apache9 reviewed Dec 16, 2024

View reviewed changes

Umeshkumar9414 force-pushed the HBASE-28951 branch from 1b4fef4 to 10e9b7c Compare December 16, 2024 17:19

This comment has been minimized.

Sign in to view

ukumawat added 5 commits December 21, 2024 01:00

HBASE-28951 rename the wal before retrying the wal-split with another…

369d580

… worker

HBASE-28951 add a counter for worker change to keep different name ea…

bde2b85

…ch time

HBASE-28951 handle first retry of splitWal for a wal with .retrying s…

e2484d8

…uffix

HBASE-28951 remove the counter in procedure and sue wal name only to …

66cb453

…count retry

HBASE-28951 introduce and new state to rename the wal

a2f732d

Umeshkumar9414 force-pushed the HBASE-28951 branch from cd0af96 to a2f732d Compare December 20, 2024 19:31

Umeshkumar9414 commented Dec 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HBASE-28951 rename the wal before retrying the wal-split with another worker #6534

HBASE-28951 rename the wal before retrying the wal-split with another worker #6534

Umeshkumar9414 commented Dec 12, 2024

Umeshkumar9414 commented Dec 12, 2024 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

mnpoonia commented Dec 13, 2024

mnpoonia commented Dec 16, 2024 •

edited

Loading

Apache9 left a comment

Apache9 Dec 16, 2024

Umeshkumar9414 Dec 16, 2024 •

edited

Loading

Apache9 Dec 16, 2024

Umeshkumar9414 Dec 16, 2024

Apache9 Dec 17, 2024

Apache9 Dec 16, 2024

Umeshkumar9414 Dec 16, 2024

Apache9 Dec 16, 2024

Umeshkumar9414 Dec 16, 2024

Apache9 Dec 17, 2024

Umeshkumar9414 commented Dec 16, 2024 •

edited

Loading

Umeshkumar9414 commented Dec 16, 2024

This comment has been minimized.

This comment has been minimized.

Apache9 commented Dec 17, 2024

This comment has been minimized.

This comment has been minimized.

Umeshkumar9414 commented Dec 20, 2024

Umeshkumar9414 Dec 20, 2024 •

edited

Loading

Apache-HBase commented Dec 20, 2024

Apache-HBase commented Dec 20, 2024

HBASE-28951 rename the wal before retrying the wal-split with another worker #6534

Are you sure you want to change the base?

HBASE-28951 rename the wal before retrying the wal-split with another worker #6534

Conversation

Umeshkumar9414 commented Dec 12, 2024

Umeshkumar9414 commented Dec 12, 2024 • edited Loading

This comment has been minimized.

This comment has been minimized.

mnpoonia commented Dec 13, 2024

mnpoonia commented Dec 16, 2024 • edited Loading

Apache9 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Umeshkumar9414 Dec 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Umeshkumar9414 commented Dec 16, 2024 • edited Loading

Umeshkumar9414 commented Dec 16, 2024

This comment has been minimized.

This comment has been minimized.

Apache9 commented Dec 17, 2024

This comment has been minimized.

This comment has been minimized.

Umeshkumar9414 commented Dec 20, 2024

Umeshkumar9414 Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

Apache-HBase commented Dec 20, 2024

Apache-HBase commented Dec 20, 2024

Umeshkumar9414 commented Dec 12, 2024 •

edited

Loading

mnpoonia commented Dec 16, 2024 •

edited

Loading

Umeshkumar9414 Dec 16, 2024 •

edited

Loading

Umeshkumar9414 commented Dec 16, 2024 •

edited

Loading

Umeshkumar9414 Dec 20, 2024 •

edited

Loading