kevinrr888 opened a new pull request, #4992: URL: https://github.com/apache/accumulo/pull/4992
This PR * Makes it so the reservation column is created on reservation and deleted on unreservation (no longer store an unreserved value in the column) * Addresses a bug with `MultipleStoresIT.testDeadReservationsCleanup()` (`ZooUtil.LockID` was missing `equals()` and `hashCode()`) (see NOTE) NOTE about original bug when I first attempted these changes: After making the reservation column changes, I tested `testDeadReservationsCleanup()` to see if I could recreate the same bug noted in this thread https://github.com/apache/accumulo/pull/4524#discussion_r1670884720 (TLDR - <4 transactions would show as being reserved, even though 4 threads were working on 4 transactions). I was able to recreate the bug (extremely rarely until I messed with the `DeadReservationCleaner` timings), but found that it is not related to the reservation column changes and is instead just a bug with the test. I'm not sure why I only saw this bug with the column changes the first go-around, but regardless it's fixed now. I was concerned if this was a bug with the code, but turns out it was just a problem with the test. The issue was the dead reservation cleaner was removing transactions that weren't dead because the `isLockHeld` predicate being passed was: `final Predicate<ZooUtil.LockID> isLockHeld = liveLocks::contains;` where `final Set<ZooUtil.LockID> liveLocks = new HashSet<>();` `ZooUtil.LockID` did not have `equals()` or `hashCode()` methods resulting in transactions being unexpectedly unreserved by the dead reservation cleaner. The real code (the Manager code) never calls equals on the LockID object, so this was just a bug with the test. closes #4907 -- 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]
