Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Prevent destroy queries from blocking status queries

The destroy method in WorkspaceHomeImpl was taking a per-instance lock
for the whole duration of an instance termination. This blocked the find
method (called by --status queries) which tries to take the same lock.

This commit changes the locking code of destroy so that it is released
while making the lengthy call to the workspace control agent.  We also
add an additional instance-specific lock for destroy. This way, a second
call to destroy will block at the beginning. When this second call
eventually proceeds, it will not find the instance because it has been
removed (which is the current behavior).  It also prevents the remove
handler to be called concurrently with a destroy from another workspace
action (for instance at the end of a start).

Closes #102.
  • Loading branch information...
commit d2cd8cc2872912367669be5f824504df0c6396dc 1 parent 04b767c
Pierre Riteau priteau authored
4 service/service/java/source/src/org/globus/workspace/creation/defaults/IdempotentInstanceResource.java
View
@@ -241,6 +241,10 @@ public void setTargetState(int state) {
throw new UnsupportedOperationException(UNSUPPORTED_MSG);
}
+ public void setTargetStateUnderLockEvaluate(int state) {
+ throw new UnsupportedOperationException(UNSUPPORTED_MSG);
+ }
+
public int getTargetState() {
throw new UnsupportedOperationException(UNSUPPORTED_MSG);
}
4 service/service/java/source/src/org/globus/workspace/service/InstanceResource.java
View
@@ -214,6 +214,10 @@ public void setTargetState(int state)
throws ManageException;
+ public void setTargetStateUnderLockEvaluate(int newstate)
+
+ throws ManageException;
+
public int getTargetState();
/**
2  service/service/java/source/src/org/globus/workspace/service/impls/InstanceResourceImpl.java
View
@@ -770,7 +770,7 @@ public synchronized boolean remove() throws ManageException {
}
}
- this.setTargetState(WorkspaceConstants.STATE_DESTROYING);
+ this.setTargetStateUnderLockEvaluate(WorkspaceConstants.STATE_DESTROYING);
if (this.getState() != WorkspaceConstants.STATE_DESTROY_SUCCEEDED) {
logger.debug(Lager.id(this.id) + " destroy failed (state " + this.getState() + ")");
return false;
21 service/service/java/source/src/org/globus/workspace/service/impls/StateTransition.java
View
@@ -16,9 +16,11 @@
package org.globus.workspace.service.impls;
+import edu.emory.mathcs.backport.java.util.concurrent.locks.Lock;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.globus.workspace.Lager;
+import org.globus.workspace.LockManager;
import org.globus.workspace.ProgrammingError;
import org.globus.workspace.WorkspaceConstants;
import org.globus.workspace.persistence.DataConvert;
@@ -330,6 +332,10 @@ private boolean remove(final StatefulResourceImpl resource,
throws ManageException {
+ final LockManager lockManager = resource.getLockManager();
+ final Lock lock = lockManager.getLock(id);
+ final Lock destroy_lock = lockManager.getLock("destroy_" + id);
+
if (current >= STATE_CANCELLING_STAGING_IN
&& current <= STATE_CANCELLING_STAGING_OUT) {
@@ -483,12 +489,27 @@ private boolean remove(final StatefulResourceImpl resource,
+ ", executing " + req.toString() + "\n");
}
+ try {
+ destroy_lock.lockInterruptibly();
+ } catch (InterruptedException e) {
+ throw new ManageException(e.getMessage(), e);
+ }
+
+ lock.unlock();
+
// TODO: add a timeout
try {
req.execute(); // could block
} catch (Throwable t) {
// candidate for admin log/trigger of severe issues
logger.error("",t);
+ } finally {
+ try {
+ lock.lockInterruptibly();
+ destroy_lock.unlock();
+ } catch (InterruptedException e) {
+ throw new ManageException(e.getMessage(), e);
+ }
}
if (this.trace) {
21 service/service/java/source/src/org/globus/workspace/service/impls/StatefulResourceImpl.java
View
@@ -111,6 +111,13 @@ protected StatefulResourceImpl(PersistenceAdapter persistenceImpl,
this.timerManager = timerManagerImpl;
}
+ // -------------------------------------------------------------------------
+ // ACCESSORS
+ // -------------------------------------------------------------------------
+
+ public LockManager getLockManager() {
+ return this.lockMgr;
+ }
// -------------------------------------------------------------------------
// ACTIVATION
@@ -169,12 +176,21 @@ private void setState(int newstate, Throwable t, boolean evaluate)
throws LockAcquisitionFailure {
Lock lock = null;
+ Lock destroy_lock = null;
if (evaluate) {
+ destroy_lock = lockMgr.getLock("destroy_" + this.id);
lock = lockMgr.getLock(this.id);
try {
+ destroy_lock.lockInterruptibly();
+ } catch (InterruptedException e) {
+ throw new LockAcquisitionFailure(e);
+ }
+
+ try {
lock.lockInterruptibly();
} catch (InterruptedException e) {
+ destroy_lock.unlock();
throw new LockAcquisitionFailure(e);
}
@@ -191,6 +207,7 @@ private void setState(int newstate, Throwable t, boolean evaluate)
logger.trace(Lager.id(this.id) + ": releasing lock");
}
lock.unlock();
+ destroy_lock.unlock();
}
}
}
@@ -613,6 +630,10 @@ void setTargetStateUnderLock(int newstate) throws ManageException {
setTargetStateImpl(newstate, false, false);
}
+ public void setTargetStateUnderLockEvaluate(int newstate) throws ManageException {
+ setTargetStateImpl(newstate, false, true);
+ }
+
/**
* @param targetState state int
* @return null for success or failure msg for helpful logging
9 service/service/java/source/src/org/globus/workspace/service/impls/WorkspaceHomeImpl.java
View
@@ -429,10 +429,18 @@ public void destroy(String id)
throw new IllegalArgumentException("id may not be null");
}
+ final Lock destroy_lock = this.lockManager.getLock("destroy_" + id);
final Lock lock = this.lockManager.getLock(id);
try {
+ destroy_lock.lockInterruptibly();
+ } catch (InterruptedException e) {
+ throw new ManageException(e.getMessage(), e);
+ }
+
+ try {
lock.lockInterruptibly();
} catch (InterruptedException e) {
+ destroy_lock.unlock();
throw new ManageException(e.getMessage(), e);
}
@@ -444,6 +452,7 @@ public void destroy(String id)
} finally {
lock.unlock();
+ destroy_lock.unlock();
}
}
Please sign in to comment.
Something went wrong with that request. Please try again.