Skip to content

Commit

Permalink
Introduce way of graceful close of forseti client from another thread…
Browse files Browse the repository at this point in the history
… by tracking status and number of active operations. Clear updateHolder reference to allow SharedLock be garbage collected. More gentle way of handling duplicate references during lock acquisition. Downgrading update lock to shared on exclusive lock release to keep common lifecycle of shared locks.
  • Loading branch information
MishaDemianenko committed Aug 3, 2015
1 parent 00b94e5 commit ca0ff45
Show file tree
Hide file tree
Showing 13 changed files with 498 additions and 230 deletions.
Expand Up @@ -95,7 +95,8 @@ public List<LockInfo> getLocks()
lockManager.accept( new Locks.Visitor()
{
@Override
public void visit( Locks.ResourceType resourceType, long resourceId, String description, long waitTime )
public void visit( Locks.ResourceType resourceType, long resourceId, String description, long waitTime,
long lockIdentityHashCode )
{
locks.add( new LockInfo( resourceType.toString(), String.valueOf( resourceId ), description ) );
}
Expand Down
Expand Up @@ -31,8 +31,10 @@ public DumpLocksVisitor( Log log )
}

@Override
public void visit( Locks.ResourceType resourceType, long resourceId, String description, long estimatedWaitTime )
public void visit( Locks.ResourceType resourceType, long resourceId, String description, long estimatedWaitTime,
long lockIdentityHashCode )
{
log.info( "%s{id=%d, waitTime=%d, description=%s}", resourceType, resourceId, estimatedWaitTime, description );
log.info( "%s{id=%d, waitTime=%d, description=%s, lockHash=%d}", resourceType, resourceId, estimatedWaitTime,
description, lockIdentityHashCode );
}
}
Expand Up @@ -29,4 +29,4 @@ public LockClientAlreadyClosedException( String message )
{
super( message );
}
}
}
@@ -0,0 +1,37 @@
/*
* Copyright (c) 2002-2015 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.com]
*
* This file is part of Neo4j.
*
* Neo4j is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package org.neo4j.kernel.impl.locking;

public class LockCountVisitor implements Locks.Visitor
{
private int lockCount = 0;

@Override
public void visit( Locks.ResourceType resourceType, long resourceId, String description, long estimatedWaitTime,
long lockIdentityHashCode )
{
lockCount++;
}

public int getLockCount()
{
return lockCount;
}
}
Expand Up @@ -58,7 +58,8 @@ public Factory( String key, String... altKeys )
interface Visitor
{
/** Visit the description of a lock held by at least one client. */
void visit( ResourceType resourceType, long resourceId, String description, long estimatedWaitTime );
void visit( ResourceType resourceType, long resourceId, String description, long estimatedWaitTime,
long lockIdentityHashCode );
}

/** Locks are split by resource types. It is up to the implementation to define the contract for these. */
Expand Down
Expand Up @@ -45,7 +45,7 @@ public boolean visit( RWLock element ) throws RuntimeException
{
LockResource lockResource = (LockResource)resource;
visitor.visit( lockResource.type(), lockResource.resourceId(),
element.describe(), element.maxWaitTime() );
element.describe(), element.maxWaitTime(), System.identityHashCode( lockResource ) );
}
return false;
}
Expand Down
Expand Up @@ -26,6 +26,7 @@
import org.junit.Test;

import org.neo4j.kernel.GraphDatabaseAPI;
import org.neo4j.kernel.impl.locking.LockCountVisitor;
import org.neo4j.kernel.impl.locking.Locks;
import org.neo4j.test.TestGraphDatabaseFactory;

Expand Down Expand Up @@ -138,17 +139,8 @@ public Void call() throws Exception

private static int lockCount( Locks locks )
{
final int[] counter = new int[1];

locks.accept( new Locks.Visitor()
{
@Override
public void visit( Locks.ResourceType resourceType, long resourceId, String description, long waitTime )
{
counter[0]++;
}
} );

return counter[0];
LockCountVisitor lockCountVisitor = new LockCountVisitor();
locks.accept( lockCountVisitor );
return lockCountVisitor.getLockCount();
}
}
Expand Up @@ -20,12 +20,10 @@
package org.neo4j.kernel.impl.locking;

import org.junit.Assert;
import org.junit.Ignore;
import org.junit.Test;

import static org.neo4j.kernel.impl.locking.ResourceTypes.NODE;

@Ignore("Not a test. This is a compatibility suite, run from LockingCompatibilityTestSuite.")
public class CloseCompatibility extends LockingCompatibilityTestSuite.Compatibility
{
public CloseCompatibility( LockingCompatibilityTestSuite suite )
Expand All @@ -34,7 +32,6 @@ public CloseCompatibility( LockingCompatibilityTestSuite suite )
}

@Test
@Ignore("Will be merged and enabled when both lock managers will support it")
public void closeShouldWaitAllOperationToFinish()
{
// given
Expand All @@ -54,41 +51,31 @@ public void closeShouldWaitAllOperationToFinish()
// reader/writer waiter in any threads
// those should be gracefully finish and client should be closed

final int[] counters = new int[1];
locks.accept( new Locks.Visitor()
{
@Override
public void visit( Locks.ResourceType resourceType, long resourceId, String description,
long estimatedWaitTime )
{
counters[0] ++;
}
} );
Assert.assertEquals( 0, counters[0] );
LockCountVisitor lockCountVisitor = new LockCountVisitor();
locks.accept( lockCountVisitor );
Assert.assertEquals( 0, lockCountVisitor.getLockCount() );

}

@Test(expected = LockClientAlreadyClosedException.class)
@Ignore("Will be enabled when both clients support it.")
@Test( expected = LockClientAlreadyClosedException.class )
public void shouldNotBeAbleToAcquireSharedLockFromClosedClient()
{
clientA.close();
clientA.acquireShared( NODE, 1l);
clientA.acquireShared( NODE, 1l );
}

@Test(expected = LockClientAlreadyClosedException.class)
@Ignore("Will be enabled when both clients support it.")
@Test( expected = LockClientAlreadyClosedException.class )
public void shouldNotBeAbleToAcquireExclusiveLockFromClosedClient()
{
clientA.close();
clientA.acquireExclusive( NODE, 1l);
clientA.acquireExclusive( NODE, 1l );
}

@Test
@Ignore("Will be enabled when both clients support it.")
public void shouldNotBeAbleToAcquireLocksUsingTryFromClosedClient()
{
clientA.close();
Assert.assertFalse( clientA.trySharedLock( NODE, 1l ) );
Assert.assertFalse( clientA.tryExclusiveLock( NODE, 1l ) );
}
}
}
Expand Up @@ -19,11 +19,12 @@
*/
package org.neo4j.kernel.impl.locking;

import java.util.concurrent.Future;

import org.junit.Ignore;
import org.junit.Test;

import java.util.concurrent.Future;

import static org.junit.Assert.assertEquals;
import static org.neo4j.kernel.impl.locking.ResourceTypes.NODE;

@Ignore("Not a test. This is a compatibility suite, run from LockingCompatibilityTestSuite.")
Expand Down Expand Up @@ -200,4 +201,66 @@ public void sharedLocksShouldNotReplaceExclusiveLocks() throws Exception
// Then
assertNotWaiting( clientB, clientBLock );
}

@Test
public void shouldUpgradeAndDowngradeSameSharedLock() throws InterruptedException
{
// when
clientA.acquireShared( NODE, 1L );
clientB.acquireShared( NODE, 1L );


LockIdentityExplorer sharedLockExplorer = new LockIdentityExplorer( NODE, 1L );
locks.accept( sharedLockExplorer );

// then xclusive should wait for shared from other client to be released
Future<Object> exclusiveLockFuture = acquireExclusive( clientB, NODE, 1L ).callAndAssertWaiting();

// and when
clientA.releaseAll();

// exclusive lock should be received
assertNotWaiting( clientB, exclusiveLockFuture );

// and when releasing exclusive
clientB.releaseExclusive( NODE, 1L );

// we still should have same read lock
LockIdentityExplorer releasedLockExplorer = new LockIdentityExplorer( NODE, 1L );
locks.accept( releasedLockExplorer );

// we still hold same lock as before
assertEquals( sharedLockExplorer.getLockIdentityHashCode(),
releasedLockExplorer.getLockIdentityHashCode() );
}

private static class LockIdentityExplorer implements Locks.Visitor
{

private Locks.ResourceType resourceType;
private long resourceId;
private long lockIdentityHashCode;

public LockIdentityExplorer( Locks.ResourceType resourceType, long resourceId )
{
this.resourceType = resourceType;
this.resourceId = resourceId;
}

@Override
public void visit( Locks.ResourceType resourceType, long resourceId, String description,
long estimatedWaitTime,
long lockIdentityHashCode )
{
if ( this.resourceType.equals( resourceType ) && this.resourceId == resourceId )
{
this.lockIdentityHashCode = lockIdentityHashCode;
}
}

public long getLockIdentityHashCode()
{
return lockIdentityHashCode;
}
}
}

0 comments on commit ca0ff45

Please sign in to comment.