Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HW-53073: Allow Thread Interruption for WriteLockManager #1

Open
wants to merge 14 commits into
base: 2.7
Choose a base branch
from
Open

HW-53073: Allow Thread Interruption for WriteLockManager #1

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
ad09e39
HW-53073: Allow Thread Interruption for WriteLockManager
agupta-hw May 27, 2019
e9e72ae
more logging.
agupta-hw Jun 3, 2019
fb013ff
more logging.
agupta-hw Jun 3, 2019
2d68361
more logging.
agupta-hw Jun 3, 2019
ef37683
wait 1 min before interrupt.
agupta-hw Jun 4, 2019
fdb4970
wait 1 min before interrupt.
agupta-hw Jun 4, 2019
c4663eb
writelock stuff changes.
agupta-hw Jun 18, 2019
965591d
log changes
agupta-hw Jun 19, 2019
35bff2a
log changes
agupta-hw Jun 24, 2019
4db2cb3
bump 70 to 130
agupta-hw Jul 4, 2019
5cd96c3
renames.
agupta-hw Jul 4, 2019
d22ff4a
renames.
agupta-hw Jul 4, 2019
c6f50ef
new logic with flag based conditionals.
agupta-hw Jul 22, 2019
5d35263
new logic with flag based conditionals.
agupta-hw Jul 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
*.iml
.idea/*
.DS_Store
*.class
*test.jar
Expand Down
57 changes: 45 additions & 12 deletions ...clipse.persistence.core/src/org/eclipse/persistence/internal/helper/WriteLockManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,27 +22,26 @@
// - 526957 : Split the logging and trace messages
package org.eclipse.persistence.internal.helper;

import java.util.IdentityHashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;

import org.eclipse.persistence.descriptors.ClassDescriptor;
import org.eclipse.persistence.descriptors.FetchGroupManager;
import org.eclipse.persistence.exceptions.ConcurrencyException;
import org.eclipse.persistence.internal.helper.linkedlist.ExposedNodeLinkedList;
import org.eclipse.persistence.internal.identitymaps.CacheKey;
import org.eclipse.persistence.internal.localization.LoggingLocalization;
import org.eclipse.persistence.internal.localization.TraceLocalization;
import org.eclipse.persistence.internal.queries.ContainerPolicy;
import org.eclipse.persistence.internal.sessions.AbstractSession;
import org.eclipse.persistence.internal.sessions.MergeManager;
import org.eclipse.persistence.internal.sessions.ObjectChangeSet;
import org.eclipse.persistence.internal.sessions.UnitOfWorkChangeSet;
import org.eclipse.persistence.internal.sessions.UnitOfWorkImpl;
import org.eclipse.persistence.internal.sessions.*;
import org.eclipse.persistence.logging.SessionLog;
import org.eclipse.persistence.mappings.DatabaseMapping;

import java.time.LocalDateTime;
import java.time.temporal.ChronoUnit;
import java.util.IdentityHashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.logging.Level;
import java.util.logging.Logger;

/**
* INTERNAL:
* <p>
Expand All @@ -60,11 +59,17 @@
* @since 10.0.3
*/
public class WriteLockManager {
private static Logger logger = Logger.getLogger(WriteLockManager.class.getSimpleName());

// this will allow us to prevent a readlock thread from looping forever.
public static final int MAXTRIES = 10000;

public static final int MAX_WAIT = 600000; //10 mins
private static final int SIXTY_SECONDS = 60; //1 minute
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Constant should be named for their purpose, not their value.
SIXTY_SECONDS should be SECONDS_BEFORE_HONOUR_INTERRUPT
SEVENTY_SECONDS should be SECONDS_IN_BAD_STATE

private static final int SEVENTY_SECONDS = 130; //Once we reached 130s, we can assume we are in a bad state, so allow interruption for everything.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment isn't accurate. Should be
// Once we reach SECONDS_BEFORE_HONOUR_INTERRUPT we consider the node in bad state for SECONDS_IN_BAD_STATE seconds. During which time, we will honour all interrupts.

//Initialize with 0 when we first find a lock.
//is volatile enough for this case? How do we guarantee Thread-Safe access? Do we want to add more complexity with synchronized
private static volatile LocalDateTime honourInterruptsWithin70secondsOfThisTime = LocalDateTime.now().minusYears(1);

/* This attribute stores the list of threads that have had a problem acquiring locks */
/* the first element in this list will be the prevailing thread */
Expand All @@ -84,6 +89,7 @@ public Map acquireLocksForClone(Object objectForClone, ClassDescriptor descripto
IdentityHashMap lockedObjects = new IdentityHashMap();
IdentityHashMap refreshedObjects = new IdentityHashMap();
try {
LocalDateTime localTime = null;
// if the descriptor has indirection for all mappings then wait as there will be no deadlock risks
CacheKey toWaitOn = acquireLockAndRelatedLocks(objectForClone, lockedObjects, refreshedObjects, cacheKey, descriptor, cloningSession);
int tries = 0;
Expand All @@ -98,7 +104,34 @@ public Map acquireLocksForClone(Object objectForClone, ClassDescriptor descripto
toWaitOn.wait();// wait for lock on object to be released
}
} catch (InterruptedException ex) {
// Ignore exception thread should continue.
//https://jira.site1.hyperwallet.local/browse/HW-53073
//Custom change to allow thread interruptions for bad threads stuck in org.eclipse.persistence.internal.helper.WriteLockManager.acquireLocksForClone pattern
jportner-hw marked this conversation as resolved.
Show resolved Hide resolved
long secondsPassed = ChronoUnit.SECONDS.between(honourInterruptsWithin70secondsOfThisTime, LocalDateTime.now());
if (secondsPassed < SEVENTY_SECONDS) {
logger.log(Level.SEVERE,
"Static Timer: Reached threshold to interrupt. Attempts: {0}, Time elapsed in seconds: {1}, Timer: {2}",
new Object[]{tries,
secondsPassed, honourInterruptsWithin70secondsOfThisTime});
throw ConcurrencyException.waitWasInterrupted(ex.getMessage());
} else {
agupta-hw marked this conversation as resolved.
Show resolved Hide resolved
if (localTime == null) {
localTime = LocalDateTime.now();
}
long secondsPassedLocally = ChronoUnit.SECONDS.between(localTime, LocalDateTime.now());
if (secondsPassedLocally < SIXTY_SECONDS) {
logger.log(Level.WARNING,
"This is a custom eclipselink change to allow interrupts, it will not interrupt till 1 minute. Attempts: "
+ "{0}, Time elapsed in seconds: {1}, Local Timer: {2}, Static Timer: {3}",
new Object[]{tries, secondsPassedLocally, localTime, honourInterruptsWithin70secondsOfThisTime});
} else {
logger.log(Level.SEVERE,
"Local Timer: Reached threshold to interrupt. Attempts: {0}, Time elapsed in seconds: {1}, Local Timer: "
+ "{2}, Static Timer: {3}",
new Object[]{tries, secondsPassed, localTime, honourInterruptsWithin70secondsOfThisTime});
honourInterruptsWithin70secondsOfThisTime = LocalDateTime.now();
throw ConcurrencyException.waitWasInterrupted(ex.getMessage());
}
}
}
}
Object waitObject = toWaitOn.getObject();
Expand Down