HBASE-30377 WALInputFormat drops WAL files that span the requested time range by junegunn · Pull Request #8646 · apache/hbase

junegunn · 2026-09-10T11:52:16Z

The timestamp in a WAL's name is its creation time, which only bounds its entries from below. A WAL stays open until it rolls, so one created before startTime can still hold entries in range, and comparing the name against startTime threw the whole file away.

startTime = 100, endTime = 200
WAL created at t=50, rolled at t=150  ->  holds entries 50..150
  50 <= 200 passes, but 50 >= 100 fails  ->  file skipped
  lost: every entry in 100..150

Fix

Compare against the modification time instead, which bounds the entries from above. That is only meaningful once the file is closed, since HDFS leaves mtime at the creation time through hflush and hsync, so it is gated on DistributedFileSystem.isFileClosed. Anything we cannot answer for, including non-HDFS filesystems, is treated as open and kept.

End-to-end test

I verified the fix on a 6-node test cluster running on a local Kubernetes cluster. Without the 1-hour logroll.period workaround, WALPlayer correctly processes all WAL files.

This is safe and conservative

With @mosmeh's suggestion applied, this patch never excludes files that the previous code included.

…me range The timestamp in a WAL's name is its creation time, which only bounds its entries from below. A WAL stays open until it rolls, so one created before startTime can still hold entries in range, and comparing the name against startTime threw the whole file away. Compare against the modification time instead, which bounds the entries from above, but only once the file is closed: HDFS leaves mtime at the creation time through hflush and hsync. Gate it on DistributedFileSystem.isFileClosed and keep anything we cannot answer for, including non-HDFS filesystems. The call is short-circuited. wal.start.time defaults to Long.MIN_VALUE, so a job that does not ask for a start time never reaches it, and one that does pays it only for files mtime alone would prune. TestWALRecordReader.testPartialRead asserted the old behaviour: it writes an entry at exactly startTime into a WAL created earlier and expected that file to be skipped. Corrected to expect both splits, asserting the entry that was being lost.

Copilot

🟢 Approval recommended

The change is localized, matches the described correctness fix for WAL spanning behavior, and is backed by updated and newly added test coverage targeting the new selection logic.

Pull request overview

This PR fixes WAL time-range filtering in hbase-mapreduce by preventing WALInputFormat from incorrectly skipping WAL files whose creation timestamp predates startTime but that remained open long enough to contain in-range entries. It does so by only pruning on the startTime boundary when the WAL is confirmed closed and its final modification time is before startTime.

Changes:

Update WALInputFormat file selection to (a) still skip WALs created after endTime, but (b) avoid skipping WALs created before startTime unless the WAL is known closed and has mtime < startTime.
Add HDFS-aware “is file closed” detection (including unwrapping HFileSystem) to make mtime meaningful only for closed files.
Extend/adjust unit tests to cover WALs spanning the startTime boundary and the closed-vs-open mtime behavior.

File summaries

File	Description
hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/WALInputFormat.java	Switches start-time pruning from WAL-name timestamp to (closed-only) modification-time checks; introduces `isClosed` using `DistributedFileSystem.isFileClosed`.
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestWALInputFormat.java	Updates existing addFile test expectations and adds targeted coverage for “use mtime only when closed” behavior.
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestWALRecordReader.java	Updates partial-read expectations to ensure WALs spanning the `startTime` boundary are retained and read correctly.

Review details

Files reviewed: 3/3 changed files
Comments generated: 0
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Co-authored-by: Yuta Imazu <yuta.imazu@gmail.com>

mosmeh · 2026-09-15T08:19:43Z

+      // The modification time is the upper bound, but HDFS leaves it at the creation time until
+      // the file is closed, so it is only meaningful once the file is. Order the checks so the
+      // extra RPC is only paid for files that the modification time alone would prune.
+      if (timestamp < startTime && lfs.getModificationTime() < startTime && isClosed(fs, lfs.getPath())) {


Sorry for meat-proxying, but Codex says

[P1] Refresh file status after confirming that the WAL is closed — hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/WALInputFormat.java:372-372
If a WAL closes after listLocatedStatus captures its status but before isClosed runs, lfs still contains the open file's creation-time mtime. This check then sees a closed file and incorrectly prunes it despite in-range entries and a newer actual mtime, silently omitting edits from WALPlayer. Re-fetch the file status after confirming closure before using its mtime to prune, retaining the file if that refresh fails.

With the timestamp < startTime guard, this patch still is an improvement, though.

Ah, yeah, that's possible. Pretty unlikely in real-world use cases though, but it's better to be right.

Addressed in 5adb255. Could you take another look?

listLocatedStatus captures mtime while the file is open (mtime = creation time). If the file closes before isFileClosed runs, the stale mtime can cause incorrect pruning. Re-fetch FileStatus after confirming closure to get the final mtime. - Rename isClosed to isClosedBefore, fold mtime check and re-fetch - Stale mtime check remains as fast path to skip the RPC

ViewDistributedFileSystem passes the instanceof DistributedFileSystem check but throws UnsupportedOperationException when the mounted filesystem does not support isFileClosed(). Treat this the same as a non-HDFS filesystem: keep the file.

thirdTs is sampled immediately after WAL shutdown, so it can equal the WAL's final modification time. Using it directly as startTime makes isClosedBefore retain the WAL, failing the assertTrue(splits.isEmpty()) assertion. Offset by 1 ms to make the boundary unambiguous.

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Copilot

🟡 Changes recommended

Two unresolved Javadoc updates remain in WALInputFormat.java.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/WALInputFormat.java:423

The startTime Javadoc immediately above still says that a timestamped file is filtered out whenever its filename timestamp is older than startTime, but this method now intentionally retains such files when they are open or when their closed-file modification time reaches the window. Please update that parameter documentation so it describes the creation-time lower bound and the closed-file mtime check; otherwise the method contract is misleading for maintainers and callers.

  static void addFile(List<FileStatus> result, FileSystem fs, LocatedFileStatus lfs, long startTime,

Files reviewed: 3/3 changed files
Comments generated: 1
Review effort level: Lite

The old description implied pure name-based filtering, but files created before startTime are only dropped when confirmed closed.

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

+    assertEquals(2, splits.size());
+    // The 1st file was created before startTime but stayed open until it rolled, so its 2nd
+    // entry, written at exactly startTime, is in-range.
+    testSplit(splits.get(0), Bytes.toBytes("2"));
    // Only the 1st entry from the 2nd file is in-range.
-    testSplit(splits.get(0), Bytes.toBytes("3"));
+    testSplit(splits.get(1), Bytes.toBytes("3"));


+    // now set a start time strictly after the last WAL's modification time
    jobConf.setLong(WALInputFormat.END_TIME_KEY, Long.MAX_VALUE);
-    jobConf.setLong(WALInputFormat.START_TIME_KEY, thirdTs);
+    jobConf.setLong(WALInputFormat.START_TIME_KEY, thirdTs + 1);


junegunn requested a lite review from Copilot September 10, 2026 11:52

junegunn self-assigned this Sep 10, 2026

Copilot started reviewing on behalf of junegunn September 10, 2026 12:05 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

mosmeh reviewed Sep 15, 2026

View reviewed changes

Comment thread hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/WALInputFormat.java Outdated

Guard mtime-based pruning with timestamp < startTime

7d4588a

Co-authored-by: Yuta Imazu <yuta.imazu@gmail.com>

junegunn requested a review from mosmeh September 15, 2026 07:45

mosmeh reviewed Sep 15, 2026

View reviewed changes

junegunn requested a review from mosmeh September 15, 2026 09:04

junegunn added 2 commits September 15, 2026 19:39

junegunn requested a lite review from Copilot September 15, 2026 10:44

Copilot AI reviewed Sep 15, 2026

View reviewed changes

Copilot started reviewing on behalf of junegunn September 15, 2026 11:02 View session

junegunn requested a lite review from Copilot September 15, 2026 11:30

Copilot started reviewing on behalf of junegunn September 15, 2026 11:41 View session

Copilot AI reviewed Sep 15, 2026

View reviewed changes

Comment thread hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/WALInputFormat.java

Update getFiles Javadoc to reflect closure-aware filtering

66583d8

The old description implied pure name-based filtering, but files created before startTime are only dropped when confirmed closed.

junegunn requested a lite review from Copilot September 15, 2026 12:08

Copilot AI reviewed Sep 15, 2026

View reviewed changes

Copilot started reviewing on behalf of junegunn September 15, 2026 12:41 View session

Navigation Menu

Uh oh!

HBASE-30377 WALInputFormat drops WAL files that span the requested time range - #8646

HBASE-30377 WALInputFormat drops WAL files that span the requested time range#8646
junegunn wants to merge 6 commits into
apache:masterfrom
junegunn:HBASE-30377

junegunn commented Sep 10, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

mosmeh Sep 15, 2026

Uh oh!

junegunn Sep 15, 2026

Uh oh!

junegunn Sep 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

junegunn commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix

End-to-end test

This is safe and conservative

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Uh oh!

mosmeh Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

junegunn Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

junegunn Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

junegunn commented Sep 10, 2026 •

edited

Loading