accumulo icon indicating copy to clipboard operation
accumulo copied to clipboard

Invalid External Compaction Progress Fix

Open kevinrr888 opened this issue 1 year ago • 5 comments

closes #4422

This fixes the error with external compactions reporting invalid (>100%) progress when the compactions include bulk imported files. This occurs because the estimated number of entries for bulk imported files is 0. Following the suggestion of Keith, instead of modifying the bulk import code to compute the estimated entries, I modified the Compactor code to compute the estimated number of entries on files with 0 entries. This value is computed by opening the RFile and summing the number of entries in each of the IndexEntry's which overlap with the extent of the external compaction.

Also added a test to ExternalCompactionProgressIT to ensure that this fix works as intended. This tests that the Compactor and CompactionCoordinator report a valid progress.

I unfortunately was not able to ensure that these changes are also reflected in the Accumulo Monitor page. Large compactions would only briefly show up in the Running Compactions table, always with a progress of 0. I also could not use something like a SlowIterator to slow the progress down (I believe since SlowIterator is part of a test package, it isn't available). If anyone is able to ensure that these changes are reflected in the Monitor page, or is able to provide guidance on how I can better test this against the Monitor page, that would be helpful. The environment I used to run an external compaction was jshell using uno.

kevinrr888 avatar Apr 11 '24 21:04 kevinrr888

@kevinrr888 when I wrote #4422 I had confirmed that the progress counts in the compaction-coordinator log files were also incorrect values which matched the monitor output because I wasn't sure if it was just a monitor specific display problem.

So if the log files report the correct progress I wouldn't worry about the monitor output.

ddanielr avatar Apr 12 '24 14:04 ddanielr

Testing the monitor may be possible by pulling json from it, but not sure. Did you manually try looking at the monitor with this change? Personally I think a manual check would be good.

keith-turner avatar Apr 17 '24 19:04 keith-turner

Testing the monitor may be possible by pulling json from it, but not sure. Did you manually try looking at the monitor with this change? Personally I think a manual check would be good.

Okay, had just tried my test using uno again with an even larger compaction and now I can see that the progress does in fact go above 100% prior to these changes. Perhaps my compaction wasn't large enough before to show any progress, I'm not sure. Will build uno using my changes and check the monitor page and get back to you.

kevinrr888 avatar Apr 18 '24 14:04 kevinrr888

Just confirmed that these changes are reflected in the monitor page. So the logs and the monitor page both no longer show >100% progress

kevinrr888 avatar Apr 18 '24 15:04 kevinrr888

I reviewed this, I see no issues but I think we should have @keith-turner review again. It looks like you have answered some of his questions, I just want to make sure that he sees that and concurs. Have you run any other ITs?

Sounds good. I have only run ExternalCompactionProgressIT.java and those that github runs. Let me know if there are others you think that should also be verified.

kevinrr888 avatar Apr 23 '24 18:04 kevinrr888