SourabhBadhya commented on code in PR #4740:
URL: https://github.com/apache/hive/pull/4740#discussion_r1463084357
##########
ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/handler/TestAbortedTxnCleaner.java:
##########
@@ -282,9 +284,19 @@ public void testCleaningOfAbortedDirectoriesBelowBase()
throws Exception {
cleaner.setCleanupHandlers(Arrays.asList(mockedTaskHandler));
cleaner.run();
- Mockito.verify(mockedFSRemover,
Mockito.times(1)).clean(any(CleanupRequest.class));
+ Mockito.verifyNoInteractions(mockedFSRemover);
Review Comment:
Why this change in test? If the cleaner has run then ideally to delete the
files there is call made to FsRemover mocked object. Also there is only one
compaction request on this table, so this request cannot be skipped.
##########
ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/handler/TestAbortedTxnCleaner.java:
##########
@@ -282,9 +284,19 @@ public void testCleaningOfAbortedDirectoriesBelowBase()
throws Exception {
cleaner.setCleanupHandlers(Arrays.asList(mockedTaskHandler));
cleaner.run();
- Mockito.verify(mockedFSRemover,
Mockito.times(1)).clean(any(CleanupRequest.class));
+ Mockito.verifyNoInteractions(mockedFSRemover);
Mockito.verify(mockedTaskHandler, Mockito.times(1)).getTasks();
+ directories = getDirectories(conf, t, null);
+ // Both base and delta files are present since the cleaner skips them as
there is a newer write.
+ Assert.assertEquals(5, directories.size());
+ Assert.assertEquals(1, directories.stream().filter(dir ->
dir.getName().startsWith(AcidUtils.BASE_PREFIX)).count());
+
+ // Run compaction and clean up
+ startInitiator();
+ startWorker();
+ startCleaner();
+
Review Comment:
I am not sure why we are calling worker and cleaner again. The scope of this
test is to remove committed and aborted deltas below the base (compacted) file
in a cleaner the first time.
##########
ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/handler/TestAbortedTxnCleaner.java:
##########
@@ -315,7 +327,6 @@ public void
testAbortedCleaningWithThreeTxnsWithDiffWriteIds() throws Exception
addDeltaFile(t, null, writeId1, writeId1, 2);
addDeltaFile(t, null, writeId2, writeId2, 2);
- ms.abortTxns(Collections.singletonList(openTxnId2));
Review Comment:
Why are we not aborting txns? This is what is tested here.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
