Copilot commented on code in PR #8646:
URL: https://github.com/apache/hbase/pull/8646#discussion_r4015194278
##########
hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/WALInputFormat.java:
##########
@@ -391,18 +393,52 @@ List<FileStatus> getFiles(FileSystem fs, Path dir, long
startTime, long endTime,
return result;
}
- static void addFile(List<FileStatus> result, LocatedFileStatus lfs, long
startTime,
+ /**
+ * Whether the file is closed and its final modification time precedes
{@code time}. Only a closed
+ * file has a reliable modification time, so an open file or a non-HDFS file
always returns
+ * {@code false} (kept). When the file is confirmed closed, its status is
re-fetched because the
+ * {@code lfs} from {@code listLocatedStatus} may carry a stale
creation-time mtime from when the
+ * file was still open.
+ */
+ private static boolean isClosedBefore(FileSystem fs, LocatedFileStatus lfs,
long time) {
+ if (lfs.getModificationTime() >= time) {
+ return false;
+ }
+ try {
+ FileSystem backing = fs instanceof HFileSystem ? ((HFileSystem)
fs).getBackingFs() : fs;
+ if (
+ !(backing instanceof DistributedFileSystem)
+ || !((DistributedFileSystem) backing).isFileClosed(lfs.getPath())
+ ) {
+ return false;
+ }
+ FileStatus refreshed = fs.getFileStatus(lfs.getPath());
+ return refreshed.getModificationTime() < time;
+ } catch (IOException | UnsupportedOperationException e) {
+ LOG.debug("Could not confirm closure of {}, keeping it", lfs.getPath(),
e);
+ return false;
+ }
+ }
+
+ static void addFile(List<FileStatus> result, FileSystem fs,
LocatedFileStatus lfs, long startTime,
long endTime) {
long timestamp =
AbstractFSWALProvider.getTimestamp(lfs.getPath().getName());
if (timestamp > 0) {
- // Looks like a valid timestamp.
- if (timestamp <= endTime && timestamp >= startTime) {
- LOG.info("Found {}", lfs.getPath());
- result.add(lfs);
- } else {
- LOG.info("Skipped {}, outside range [{}/{} - {}/{}]", lfs.getPath(),
startTime,
- Instant.ofEpochMilli(startTime), endTime,
Instant.ofEpochMilli(endTime));
+ // The name carries the WAL's 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 must not be dropped on the strength of its name alone.
Review Comment:
The `getFiles` Javadoc above still says that a timestamped file is filtered
whenever its name is older than `startTime`, but this new branch intentionally
retains such files when they may still be open. Please update that public
behavior description so callers and future maintainers do not rely on the
now-incorrect name-only filtering contract.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
