junegunn commented on code in PR #8646:
URL: https://github.com/apache/hbase/pull/8646#discussion_r4015069947
##########
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestWALInputFormat.java:
##########
@@ -65,34 +66,98 @@ public static void setupClass() throws Exception {
@Test
public void testAddFile() {
List<FileStatus> lfss = new ArrayList<>();
+ // a plain FileSystem is never reported closed, so nothing is skipped on
the startTime side
+ FileSystem fs = Mockito.mock(FileSystem.class);
LocatedFileStatus lfs = Mockito.mock(LocatedFileStatus.class);
long now = EnvironmentEdgeManager.currentTime();
Mockito.when(lfs.getPath()).thenReturn(new Path("/name." + now));
- WALInputFormat.addFile(lfss, lfs, now, now);
+ WALInputFormat.addFile(lfss, fs, lfs, now, now);
assertEquals(1, lfss.size());
- WALInputFormat.addFile(lfss, lfs, now - 1, now - 1);
+ WALInputFormat.addFile(lfss, fs, lfs, now - 1, now - 1);
assertEquals(1, lfss.size());
- WALInputFormat.addFile(lfss, lfs, now - 2, now - 1);
+ WALInputFormat.addFile(lfss, fs, lfs, now - 2, now - 1);
assertEquals(1, lfss.size());
- WALInputFormat.addFile(lfss, lfs, now - 2, now);
+ WALInputFormat.addFile(lfss, fs, lfs, now - 2, now);
assertEquals(2, lfss.size());
- WALInputFormat.addFile(lfss, lfs, Long.MIN_VALUE, now);
+ WALInputFormat.addFile(lfss, fs, lfs, Long.MIN_VALUE, now);
assertEquals(3, lfss.size());
- WALInputFormat.addFile(lfss, lfs, Long.MIN_VALUE, Long.MAX_VALUE);
+ WALInputFormat.addFile(lfss, fs, lfs, Long.MIN_VALUE, Long.MAX_VALUE);
assertEquals(4, lfss.size());
- WALInputFormat.addFile(lfss, lfs, now, now + 2);
+ WALInputFormat.addFile(lfss, fs, lfs, now, now + 2);
assertEquals(5, lfss.size());
- WALInputFormat.addFile(lfss, lfs, now + 1, now + 2);
- assertEquals(5, lfss.size());
- Mockito.when(lfs.getPath()).thenReturn(new Path("/name"));
- WALInputFormat.addFile(lfss, lfs, Long.MIN_VALUE, Long.MAX_VALUE);
+ // created before startTime, but it may have stayed open and collected
in-range entries
+ WALInputFormat.addFile(lfss, fs, lfs, now + 1, now + 2);
assertEquals(6, lfss.size());
- Mockito.when(lfs.getPath()).thenReturn(new Path("/name.123"));
- WALInputFormat.addFile(lfss, lfs, Long.MIN_VALUE, Long.MAX_VALUE);
+ Mockito.when(lfs.getPath()).thenReturn(new Path("/name"));
+ WALInputFormat.addFile(lfss, fs, lfs, Long.MIN_VALUE, Long.MAX_VALUE);
assertEquals(7, lfss.size());
- Mockito.when(lfs.getPath()).thenReturn(new Path("/name." + now + ".meta"));
- WALInputFormat.addFile(lfss, lfs, now, now);
+ Mockito.when(lfs.getPath()).thenReturn(new Path("/name.123"));
+ WALInputFormat.addFile(lfss, fs, lfs, Long.MIN_VALUE, Long.MAX_VALUE);
assertEquals(8, lfss.size());
+ Mockito.when(lfs.getPath()).thenReturn(new Path("/name." + now + ".meta"));
+ WALInputFormat.addFile(lfss, fs, lfs, now, now);
+ assertEquals(9, lfss.size());
+ }
+
+ private static boolean isKept(FileSystem fs, long created, long mtime, long
start, long end) {
+ return isKept(fs, created, mtime, mtime, start, end);
+ }
+
+ private static boolean isKept(FileSystem fs, long created, long staleMtime,
long refreshedMtime,
+ long start, long end) {
+ List<FileStatus> result = new ArrayList<>();
+ Path path = new Path("/name." + created);
+ LocatedFileStatus lfs = Mockito.mock(LocatedFileStatus.class);
+ Mockito.when(lfs.getPath()).thenReturn(path);
+ Mockito.when(lfs.getModificationTime()).thenReturn(staleMtime);
+ try {
+ FileStatus refreshed = Mockito.mock(FileStatus.class);
+ Mockito.when(refreshed.getModificationTime()).thenReturn(refreshedMtime);
+ Mockito.when(fs.getFileStatus(path)).thenReturn(refreshed);
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
Review Comment:
`isKept` is a `private static` helper that does not declare `throws`.
`Mockito.when(...).thenReturn(...)` requires catching the checked `IOException`
inside the helper itself. Pre-existing code, not part of this change.
##########
hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestWALRecordReader.java:
##########
@@ -245,9 +248,9 @@ public void testWALRecordReader() throws Exception {
assertEquals(1, splits.size());
testSplit(splits.get(0), Bytes.toBytes("1"));
- // now set a start time
+ // 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);
Review Comment:
`thirdTs` is sampled immediately after `log.shutdown()`, which flushes and
closes the WAL. The WAL's final modification time is set at close, so `thirdTs
>= mtime` holds. Adding `+ 1` makes it strictly greater. The comment describes
the relationship correctly. The variable name is pre-existing.
--
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]
