vldpyatkov commented on code in PR #1447:
URL: https://github.com/apache/ignite-3/pull/1447#discussion_r1057331892
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/HeapLockManager.java:
##########
@@ -248,6 +262,28 @@ private boolean isWaiterReadyToNotify(WaiterImpl waiter,
boolean skipFail) {
return true;
}
+ /**
+ * Fails the waiter if there is wait timeout in deadlock prevention
policy. If timeout is {@code 0}, waiter is failed
+ * instantly. If timeout is greater than {@code 0}, waiter should be
failed after timeout, if it can't acquire lock within
+ * this timeout. If timeout is lesser than {@code 0}, waiter is not
failed at all.
+ *
+ * @param waiter Current waiter.
+ * @param conflictingWaiter Conflicting waiter.
+ * @return True if current waiter ready to notify, false otherwise.
+ */
+ private boolean failWaiterIfTimeoutIsPresent(WaiterImpl waiter,
WaiterImpl conflictingWaiter) {
+ if (deadlockPreventionPolicy.waitTimeout() > 0) {
+ waiter.failAfterTimeout(waiters,
deadlockPreventionPolicy.waitTimeout());
Review Comment:
I would prefer to see this as timeout in future (WaiterImpl#fut).
There is no reason to set timeout outside of waiter creation.
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/HeapLockManager.java:
##########
@@ -151,11 +155,20 @@ public Waiter waiter(LockKey key, UUID txId) {
*/
private static class LockState {
/** Waiters. */
- private final TreeMap<UUID, WaiterImpl> waiters = new TreeMap<>();
+ private final TreeMap<UUID, WaiterImpl> waiters;
+
+ private final DeadlockPreventionPolicy deadlockPreventionPolicy;
/** Marked for removal flag. */
private boolean markedForRemove = false;
+ public LockState(DeadlockPreventionPolicy deadlockPreventionPolicy) {
+ Comparator<UUID> txComparator =
+ deadlockPreventionPolicy.txIdComparator() != null ?
deadlockPreventionPolicy.txIdComparator() : UUID::compareTo;
+ this.waiters = new TreeMap<>(txComparator);
+ this.deadlockPreventionPolicy = deadlockPreventionPolicy;
Review Comment:
This looks ugly when we get a comparator from a policy, that save the object
entirely in a field.
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/HeapLockManager.java:
##########
@@ -368,6 +404,15 @@ private ArrayList<WaiterImpl> unlockCompatibleWaiters() {
return toNotify;
}
+ /**
+ * Whether transaction priority if used for conflict resolution.
+ *
+ * @return Whether priority is used.
+ */
+ private boolean usePriority() {
+ return deadlockPreventionPolicy.txIdComparator() != null;
Review Comment:
I think, this is not obviously, but up to you.
--
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]
