apache/accumulo

Integeration test testFatePrintAndSummaryCommandsWithInProgressTxns was dropped in merge

keith-turner opened this issue · 5 comments

keith-turner commented

Describe the bug

When merging main into elasticity in 9144377, dropped the integration test FateSummaryIT.testFatePrintAndSummaryCommandsWithInProgressTxns from #4556. Would still be good to have this test, but it was not working so need to look into recreating it for elasticity. Tried the following while trying to merge before dropping the test.

  • In #4350 FateSummaryIT was moved into FateOpsCommandsIT. So tried manually copying the test there.
  • The test would stop all compactors, but in elasticity compactors are required to run compaction. So removed the code that was stopping compactors.

After doing the above the test would not run successfully. Looking into it a bit I suspect that 500 compactions against the same tablet do not run as quickly as they did in 3.1 and 2.1 for multiple reasons. So maybe this reduces the likelyhood of catching a race condition when running the command, but not sure about this. May need to start concurrent compactions against different tablets in elasticity so they can run concurrently.

A more general problem with the test is that it does not work with the multiple fate stores.

Since the test was not passing in its current form and seemed like it needed a good bit of work, decided to move forward with a merge w/o it and open this issue.

Expected behavior

The behavior of the fate command ignoring race conditions is tested.

dlmarion commented

This test is also timing out in the 2.1 and 3.1 builds. The cause of the timeout is at https://github.com/apache/accumulo/blob/2.1/test/src/main/java/org/apache/accumulo/test/FateSummaryIT.java#L235 when deleting the table.

kevinrr888 commented

I can rewrite this for elasticity and look into the timeout for 2.1/3.1

keith-turner commented

@kevinrr888 the changes from #4631 were not merged in elasticity. I merged them to main and then from main to elasticity I did a merge -sours in commit 3c254d5. When doing the merge -sours I made sure the changes from #4631 were the only outstanding changes.

  • 👍1
kevinrr888 commented

@keith-turner Sounds good. I just rewrote the test for elasticity in the PR above

kevinrr888 commented

This issue can be closed: #4683