You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "DieterDP-ng (via GitHub)" <gi...@apache.org> on 2026/06/09 09:53:51 UTC
[PR] HBASE-29774 incremental backup fails on empty WAL [hbase]
DieterDP-ng opened a new pull request, #8327:
URL: https://github.com/apache/hbase/pull/8327
Add handling for WALHeaderEOFException in WALInputFormat so WALPlayer now skips empty WAL files rather than crashing. Empty WAL files (also lacking a header) either originate from crashed Region Servers, or might exist for a very short period when using the legacy WAL writer on newly created active WALs.
This solves a problem where incremental backups could fail when converting WAL files to HFiles. In this use case, all WAL files passed to the WALPlayer are rolled (so no longer active), so empty WAL files were due to crashed Region Servers.
(This is an updated PR of the one originally created by my colleague @thomassngdata. It applies some review remarks I had when reviewing that PR, but Thomas is currently unavailable to process them himself.)
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29774 incremental backup fails on empty WAL [hbase]
Posted by "DieterDePaepe (via GitHub)" <gi...@apache.org>.
DieterDePaepe commented on PR #8327:
URL: https://github.com/apache/hbase/pull/8327#issuecomment-4691173739
@VladRodionov - mind giving a formal approval?
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29774 incremental backup fails on empty WAL [hbase]
Posted by "DieterDePaepe (via GitHub)" <gi...@apache.org>.
DieterDePaepe merged PR #8327:
URL: https://github.com/apache/hbase/pull/8327
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29774 incremental backup fails on empty WAL [hbase]
Posted by "VladRodionov (via GitHub)" <gi...@apache.org>.
VladRodionov commented on code in PR #8327:
URL: https://github.com/apache/hbase/pull/8327#discussion_r3384424923
##########
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestWALInputFormat.java:
##########
@@ -120,4 +122,44 @@ public void testHandlesArchivedWALFiles() throws Exception {
assertEquals(archiveWal.toString(), split.getLogFileName());
}
+ /**
+ * Test that an empty WAL file (which causes WALHeaderEOFException) is gracefully handled and
+ * skipped rather than causing the job to fail.
+ */
+ @Test
+ public void testHandlesEmptyWALFile() throws Exception {
+ Configuration conf = TEST_UTIL.getConfiguration();
+
+ // Create an empty WAL file
+ Path walRootDir = CommonFSUtils.getWALRootDir(conf);
+ Path emptyWalFile =
+ new Path(walRootDir, "WALs/empty-wal-test/empty." + EnvironmentEdgeManager.currentTime());
+ TEST_UTIL.getTestFileSystem().mkdirs(emptyWalFile.getParent());
+ TEST_UTIL.getTestFileSystem().create(emptyWalFile).close();
+
+ JobContext ctx = Mockito.mock(JobContext.class);
+ conf.set(FileInputFormat.INPUT_DIR, emptyWalFile.toString());
+ conf.set(WALPlayer.INPUT_FILES_SEPARATOR_KEY, ";");
+ Mockito.when(ctx.getConfiguration()).thenReturn(conf);
+ Job job = Job.getInstance(conf);
+ TableMapReduceUtil.initCredentialsForCluster(job, conf);
+ Mockito.when(ctx.getCredentials()).thenReturn(job.getCredentials());
+
+ // Create record reader and verify it handles the empty file gracefully
+ try (WALInputFormat.WALKeyRecordReader reader = new WALInputFormat.WALKeyRecordReader()) {
+ TaskAttemptContext taskCtx = Mockito.mock(TaskAttemptContext.class);
+ Mockito.when(taskCtx.getConfiguration()).thenReturn(conf);
+
+ WALInputFormat wif = new WALInputFormat();
+ List<InputSplit> splits = wif.getSplits(ctx);
+ assertEquals(1, splits.size());
+ WALInputFormat.WALSplit split = (WALInputFormat.WALSplit) splits.get(0);
+
+ // This should not throw WALHeaderEOFException - it should return false for nextKeyValue()
+ reader.initialize(split, taskCtx);
+ // nextKeyValue() should return false since the file is empty (reader is null)
+ assertFalse(reader.nextKeyValue());
+ }
Review Comment:
nit: Could we delete emptyWalFile in a finally block? The test creates a WAL file under the mini-cluster WAL root and does not appear to clean it up. Even if the test cluster cleanup eventually removes it, local cleanup would make the test more isolated.
Otherwise, lgtm.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29774 incremental backup fails on empty WAL [hbase]
Posted by "ndimiduk (via GitHub)" <gi...@apache.org>.
ndimiduk commented on PR #8327:
URL: https://github.com/apache/hbase/pull/8327#issuecomment-4692244176
Wait what's this, we need to manually approve PR workflows now??
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29774 incremental backup fails on empty WAL [hbase]
Posted by "DieterDePaepe (via GitHub)" <gi...@apache.org>.
DieterDePaepe commented on PR #8327:
URL: https://github.com/apache/hbase/pull/8327#issuecomment-4658601089
As I've helped edit this PR myself, we need someone else of HBase community to give an approval before I can merge. @ndimiduk or @hgromer - you've reviewed backup related PRs before. Would appreciate if either of you had time for a review.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-29774 incremental backup fails on empty WAL [hbase]
Posted by "DieterDP-ng (via GitHub)" <gi...@apache.org>.
DieterDP-ng commented on code in PR #8327:
URL: https://github.com/apache/hbase/pull/8327#discussion_r3403245276
##########
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestWALInputFormat.java:
##########
@@ -120,4 +122,44 @@ public void testHandlesArchivedWALFiles() throws Exception {
assertEquals(archiveWal.toString(), split.getLogFileName());
}
+ /**
+ * Test that an empty WAL file (which causes WALHeaderEOFException) is gracefully handled and
+ * skipped rather than causing the job to fail.
+ */
+ @Test
+ public void testHandlesEmptyWALFile() throws Exception {
+ Configuration conf = TEST_UTIL.getConfiguration();
+
+ // Create an empty WAL file
+ Path walRootDir = CommonFSUtils.getWALRootDir(conf);
+ Path emptyWalFile =
+ new Path(walRootDir, "WALs/empty-wal-test/empty." + EnvironmentEdgeManager.currentTime());
+ TEST_UTIL.getTestFileSystem().mkdirs(emptyWalFile.getParent());
+ TEST_UTIL.getTestFileSystem().create(emptyWalFile).close();
+
+ JobContext ctx = Mockito.mock(JobContext.class);
+ conf.set(FileInputFormat.INPUT_DIR, emptyWalFile.toString());
+ conf.set(WALPlayer.INPUT_FILES_SEPARATOR_KEY, ";");
+ Mockito.when(ctx.getConfiguration()).thenReturn(conf);
+ Job job = Job.getInstance(conf);
+ TableMapReduceUtil.initCredentialsForCluster(job, conf);
+ Mockito.when(ctx.getCredentials()).thenReturn(job.getCredentials());
+
+ // Create record reader and verify it handles the empty file gracefully
+ try (WALInputFormat.WALKeyRecordReader reader = new WALInputFormat.WALKeyRecordReader()) {
+ TaskAttemptContext taskCtx = Mockito.mock(TaskAttemptContext.class);
+ Mockito.when(taskCtx.getConfiguration()).thenReturn(conf);
+
+ WALInputFormat wif = new WALInputFormat();
+ List<InputSplit> splits = wif.getSplits(ctx);
+ assertEquals(1, splits.size());
+ WALInputFormat.WALSplit split = (WALInputFormat.WALSplit) splits.get(0);
+
+ // This should not throw WALHeaderEOFException - it should return false for nextKeyValue()
+ reader.initialize(split, taskCtx);
+ // nextKeyValue() should return false since the file is empty (reader is null)
+ assertFalse(reader.nextKeyValue());
+ }
Review Comment:
Added.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org