[HUDI-820] cleaner repair command should only inspect clean metadata files

[bvaradar]

@lamber-ken : This is something I missed when reviewing cleaner repair code changes. The repair command has a serious bug in that it might delete inflight instants of other actions.

cc @vinothchandar

[vinothchandar]

Randomly saw this in test logs..

298541 [main] WARN  org.apache.hudi.common.HoodieClientTestHarness  - Closing file-system instance used in previous test-run
298630 [main] WARN  org.apache.hudi.table.action.clean.CleanActionExecutor  - Failed to perform previous clean operation, instant: [==>000000023__clean__INFLIGHT]
org.apache.hudi.exception.HoodieIOException: Not an Avro data file
	at org.apache.hudi.table.action.clean.CleanActionExecutor.runPendingClean(CleanActionExecutor.java:212)
	at org.apache.hudi.table.action.clean.CleanActionExecutor.lambda$execute$5(CleanActionExecutor.java:261)
	at java.util.ArrayList.forEach(ArrayList.java:1257)
	at org.apache.hudi.table.action.clean.CleanActionExecutor.execute(CleanActionExecutor.java:258)
	at org.apache.hudi.table.HoodieCopyOnWriteTable.clean(HoodieCopyOnWriteTable.java:197)
	at org.apache.hudi.client.HoodieWriteClient.clean(HoodieWriteClient.java:630)
	at org.apache.hudi.table.TestCleaner.runCleaner(TestCleaner.java:425)
	at org.apache.hudi.table.TestCleaner.runCleaner(TestCleaner.java:414)
	at org.apache.hudi.table.TestCleaner.testCleanPreviousCorruptedCleanFiles(TestCleaner.java:1000)

(May be totally a false alarm, but just saying we should understand if all these are expected)..

[lamberken]

Randomly saw this in test logs..

298541 [main] WARN  org.apache.hudi.common.HoodieClientTestHarness  - Closing file-system instance used in previous test-run
298630 [main] WARN  org.apache.hudi.table.action.clean.CleanActionExecutor  - Failed to perform previous clean operation, instant: [==>000000023__clean__INFLIGHT]
org.apache.hudi.exception.HoodieIOException: Not an Avro data file
	at org.apache.hudi.table.action.clean.CleanActionExecutor.runPendingClean(CleanActionExecutor.java:212)
	at org.apache.hudi.table.action.clean.CleanActionExecutor.lambda$execute$5(CleanActionExecutor.java:261)
	at java.util.ArrayList.forEach(ArrayList.java:1257)
	at org.apache.hudi.table.action.clean.CleanActionExecutor.execute(CleanActionExecutor.java:258)
	at org.apache.hudi.table.HoodieCopyOnWriteTable.clean(HoodieCopyOnWriteTable.java:197)
	at org.apache.hudi.client.HoodieWriteClient.clean(HoodieWriteClient.java:630)
	at org.apache.hudi.table.TestCleaner.runCleaner(TestCleaner.java:425)
	at org.apache.hudi.table.TestCleaner.runCleaner(TestCleaner.java:414)
	at org.apache.hudi.table.TestCleaner.testCleanPreviousCorruptedCleanFiles(TestCleaner.java:1000)

(May be totally a false alarm, but just saying we should understand if all these are expected)..

These are expected, in TestCleaner#testCleanPreviousCorruptedCleanFiles test case, create a corrupted clean files first, then run cleaner.

[codecov-io]

Codecov Report

Merging #1542 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #1542      +/-   ##
============================================
- Coverage     71.78%   71.77%   -0.02%     
  Complexity     1087     1087              
============================================
  Files           385      385              
  Lines         16575    16575              
  Branches       1668     1668              
============================================
- Hits          11899    11897       -2     
- Misses         3947     3949       +2     
  Partials        729      729              

Impacted Files	Coverage Δ	Complexity Δ
...ache/hudi/common/fs/inline/InMemoryFileSystem.java	79.31% <0.00%> (-10.35%)	0.00% <0.00%> (ø%)
...e/hudi/common/table/log/HoodieLogFormatWriter.java	76.92% <0.00%> (+0.96%)	0.00% <0.00%> (ø%)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa6aba7...2c72c50. Read the comment docs.

[lamberken]

hi @bvaradar, the unit test TestCleaner#testCleanPreviousCorruptedCleanFiles has covered. IMO, TestRepairsCommand.java is redundant, WDYT?

[bvaradar]

@lamber-ken : Both tests different aspects of the change. TestCleaner tests the skipping of corrupted files whereas TestRepairsCommand checks if the underlying corrupted file is deleted.