Replace AbstractFateStore.verifyReserved() with new method FateMutator.requireReserved() - #6521
Replace AbstractFateStore.verifyReserved() with new method FateMutator.requireReserved()#6521Amemeda wants to merge 5 commits into
Conversation
|
(OUTDATED) |
| @Override | ||
| public Repo<T> top() { | ||
| verifyReservedAndNotDeleted(false); | ||
| newMutator(fateId).requireReserved(reservation); |
There was a problem hiding this comment.
Pretty sure all these spots can just be removed instead of replaced. verifyReservedAndNotDeleted(false) is a no-op. Also this newMutator(fateId).requireReserved(reservation); creates a conditional mutation but never sends it which we don't want or need to do.
| verifyReservedAndNotDeleted(true); | ||
|
|
||
| newMutator(fateId).putStatus(status).mutate(); | ||
| newMutator(fateId).requireReserved(reservation).putStatus(status).mutate(); |
There was a problem hiding this comment.
| newMutator(fateId).requireReserved(reservation).putStatus(status).mutate(); | |
| newReservedMutator().putStatus(status).mutate(); |
It might be nice to create a helper in this class that returns newMutator(fateId).requireReserved(reservation). Then we can just replace the newMutator() calls with newReservedMutator().
It also has the benefit of making the code shorter and we can add a check in this new helper method like
Preconditions.checkState(!deleted, Attempted write on deleted FATE transaction: " + fateId); for completeness
Add new method to
FateMutator.javathat requires a FateId to be reserved for certain transactions. This method will replaceAbstractFateStore.verifyReservedAndNotDeleted()implementation inUserFateStore.java.FateMutator.requireReserved()and its implementation inFateMutatorImpl.java.requireReserved()requires a transaction to be reserved with a specificFateStore.FateReservation.verifyReservedandNotDeleted()fromUserFateStore.javamethods. To replace its implementation, new methodrequireReserved()was appended onto existingnewMutators in push(), pop(), setStatus(), setTransactionInfo(), delete(), and forceDelete(). For methods top(), getStack(), getTransactionInfo(), and timeCreated(),verifyReservedandNotDeleted()simply was removed.Closes #4908