aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject git commit: Dropping synchronized from validateIfLocked()
Date Fri, 12 Sep 2014 00:14:45 GMT
Repository: incubator-aurora
Updated Branches:
  refs/heads/master b3e0583eb -> dc69a11ed


Dropping synchronized from validateIfLocked()

Bugs closed: AURORA-702

Reviewed at https://reviews.apache.org/r/25556/


Project: http://git-wip-us.apache.org/repos/asf/incubator-aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-aurora/commit/dc69a11e
Tree: http://git-wip-us.apache.org/repos/asf/incubator-aurora/tree/dc69a11e
Diff: http://git-wip-us.apache.org/repos/asf/incubator-aurora/diff/dc69a11e

Branch: refs/heads/master
Commit: dc69a11edc911b9407b2b155b945e8b9298af054
Parents: b3e0583
Author: Maxim Khutornenko <maxim@apache.org>
Authored: Thu Sep 11 17:14:37 2014 -0700
Committer: Maxim Khutornenko <maxim@apache.org>
Committed: Thu Sep 11 17:14:37 2014 -0700

----------------------------------------------------------------------
 .../aurora/scheduler/state/LockManagerImpl.java |  2 +-
 .../scheduler/state/LockManagerImplTest.java    | 80 +++++++++++++++++++-
 2 files changed, 77 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/dc69a11e/src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java b/src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java
index 8befeca..f9521f9 100644
--- a/src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java
+++ b/src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java
@@ -91,7 +91,7 @@ class LockManagerImpl implements LockManager {
   }
 
   @Override
-  public synchronized void validateIfLocked(final ILockKey context, Optional<ILock>
heldLock)
+  public void validateIfLocked(final ILockKey context, Optional<ILock> heldLock)
       throws LockException {
 
     Optional<ILock> stored = storage.consistentRead(new Work.Quiet<Optional<ILock>>()
{

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/dc69a11e/src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java b/src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java
index 791aa6f..c9ec8e5 100644
--- a/src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/state/LockManagerImplTest.java
@@ -14,9 +14,12 @@
 package org.apache.aurora.scheduler.state;
 
 import java.util.UUID;
+import java.util.concurrent.CountDownLatch;
 
 import com.google.common.base.Optional;
+import com.google.common.base.Throwables;
 import com.google.common.collect.Iterables;
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
 import com.twitter.common.quantity.Amount;
 import com.twitter.common.quantity.Time;
 import com.twitter.common.testing.easymock.EasyMockTest;
@@ -31,13 +34,17 @@ import org.apache.aurora.scheduler.storage.entities.IJobKey;
 import org.apache.aurora.scheduler.storage.entities.ILock;
 import org.apache.aurora.scheduler.storage.entities.ILockKey;
 import org.apache.aurora.scheduler.storage.mem.MemStorage;
+import org.apache.aurora.scheduler.storage.testing.StorageTestUtil;
 import org.easymock.EasyMock;
+import org.easymock.IAnswer;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 
 import static org.apache.aurora.gen.apiConstants.DEFAULT_ENVIRONMENT;
+import static org.apache.aurora.scheduler.storage.Storage.Work;
+import static org.easymock.EasyMock.expect;
 import static org.junit.Assert.assertEquals;
 
 public class LockManagerImplTest extends EasyMockTest {
@@ -48,6 +55,8 @@ public class LockManagerImplTest extends EasyMockTest {
   private static final ILockKey LOCK_KEY = ILockKey.build(LockKey.job(JOB_KEY.newBuilder()));
   private static final UUID TOKEN = UUID.fromString("79d6d790-3212-11e3-aa6e-0800200c9a66");
 
+  private FakeClock clock;
+  private UUIDGenerator tokenGenerator;
   private LockManager lockManager;
   private long timestampMs;
 
@@ -56,19 +65,20 @@ public class LockManagerImplTest extends EasyMockTest {
 
   @Before
   public void setUp() throws Exception {
-    FakeClock clock = new FakeClock();
+    clock = new FakeClock();
     clock.advance(Amount.of(12345L, Time.SECONDS));
     timestampMs = clock.nowMillis();
 
-    UUIDGenerator tokenGenerator = createMock(UUIDGenerator.class);
-    EasyMock.expect(tokenGenerator.createNew()).andReturn(TOKEN).anyTimes();
+    tokenGenerator = createMock(UUIDGenerator.class);
+    expect(tokenGenerator.createNew()).andReturn(TOKEN).anyTimes();
 
     lockManager = new LockManagerImpl(MemStorage.newEmptyStorage(), clock, tokenGenerator);
-    control.replay();
   }
 
   @Test
   public void testAcquireLock() throws Exception {
+    control.replay();
+
     ILock expected = ILock.build(new Lock()
         .setKey(LOCK_KEY.newBuilder())
         .setToken(TOKEN.toString())
@@ -81,6 +91,8 @@ public class LockManagerImplTest extends EasyMockTest {
 
   @Test
   public void testAcquireLockInProgress() throws Exception {
+    control.replay();
+
     expectLockException(JOB_KEY);
     lockManager.acquireLock(LOCK_KEY, USER);
     lockManager.acquireLock(LOCK_KEY, USER);
@@ -88,6 +100,8 @@ public class LockManagerImplTest extends EasyMockTest {
 
   @Test
   public void testReleaseLock() throws Exception {
+    control.replay();
+
     ILock lock = lockManager.acquireLock(LOCK_KEY, USER);
     lockManager.releaseLock(lock);
 
@@ -97,17 +111,23 @@ public class LockManagerImplTest extends EasyMockTest {
 
   @Test
   public void testValidateLockStoredEqualHeld() throws Exception {
+    control.replay();
+
     ILock lock = lockManager.acquireLock(LOCK_KEY, USER);
     lockManager.validateIfLocked(LOCK_KEY, Optional.of(lock));
   }
 
   @Test
   public void testValidateLockNotStoredNotHeld() throws Exception {
+    control.replay();
+
     lockManager.validateIfLocked(LOCK_KEY, Optional.<ILock>absent());
   }
 
   @Test
   public void testValidateLockStoredNotEqualHeld() throws Exception {
+    control.replay();
+
     expectLockException(JOB_KEY);
     ILock lock = lockManager.acquireLock(LOCK_KEY, USER);
     lock = ILock.build(lock.newBuilder().setUser("bob"));
@@ -116,6 +136,8 @@ public class LockManagerImplTest extends EasyMockTest {
 
   @Test
   public void testValidateLockStoredNotEqualHeldWithHeldNull() throws Exception {
+    control.replay();
+
     expectLockException(JOB_KEY);
     lockManager.acquireLock(LOCK_KEY, USER);
     lockManager.validateIfLocked(LOCK_KEY, Optional.<ILock>absent());
@@ -123,6 +145,8 @@ public class LockManagerImplTest extends EasyMockTest {
 
   @Test
   public void testValidateLockNotStoredHeld() throws Exception {
+    control.replay();
+
     IJobKey jobKey = JobKeys.from("r", "e", "n");
     expectLockException(jobKey);
     ILock lock = lockManager.acquireLock(LOCK_KEY, USER);
@@ -132,10 +156,58 @@ public class LockManagerImplTest extends EasyMockTest {
 
   @Test
   public void testGetLocks() throws Exception {
+    control.replay();
+
     ILock lock = lockManager.acquireLock(LOCK_KEY, USER);
     assertEquals(lock, Iterables.getOnlyElement(lockManager.getLocks()));
   }
 
+  // Test for regression of AURORA-702.
+  @Test
+  public void testNoDeadlock() throws Exception {
+    final StorageTestUtil storageUtil = new StorageTestUtil(this);
+
+    expect(storageUtil.storeProvider.getLockStore()).andReturn(storageUtil.lockStore).times(2);
+    expect(storageUtil.lockStore.fetchLock(LOCK_KEY)).andReturn(Optional.<ILock>absent()).times(2);
+
+    final CountDownLatch reads = new CountDownLatch(2);
+    EasyMock.makeThreadSafe(storageUtil.storage, false);
+    expect(storageUtil.storage.consistentRead(EasyMock.<Work<?, ?>>anyObject()))
+        .andAnswer(new IAnswer<Object>() {
+          @Override
+          public Object answer() throws Throwable {
+            @SuppressWarnings("unchecked")
+            Work<?, ?> work = (Work<?, ?>) EasyMock.getCurrentArguments()[0];
+            Object result = work.apply(storageUtil.storeProvider);
+            reads.countDown();
+            reads.await();
+            return result;
+          }
+        }).times(2);
+
+    lockManager = new LockManagerImpl(storageUtil.storage, clock, tokenGenerator);
+
+    control.replay();
+
+    new ThreadFactoryBuilder()
+        .setDaemon(true)
+        .setNameFormat("LockRead-%s")
+        .build()
+        .newThread(new Runnable() {
+          @Override
+          public void run() {
+            try {
+              lockManager.validateIfLocked(LOCK_KEY, Optional.<ILock>absent());
+            } catch (LockException e) {
+              throw Throwables.propagate(e);
+            }
+          }
+        })
+        .start();
+
+    lockManager.validateIfLocked(LOCK_KEY, Optional.<ILock>absent());
+  }
+
   private void expectLockException(IJobKey key) {
     expectedException.expect(LockException.class);
     expectedException.expectMessage(JobKeys.canonicalString(key));


Mime
View raw message