Skip to content

Commit

Permalink
Replace multiple locks in GBPTree
Browse files Browse the repository at this point in the history
... with single simple lock.
  • Loading branch information
burqen committed May 19, 2017
1 parent 92fcf24 commit 7705a2b
Show file tree
Hide file tree
Showing 4 changed files with 287 additions and 34 deletions.
Expand Up @@ -27,9 +27,6 @@
import java.nio.file.NoSuchFileException;
import java.nio.file.StandardOpenOption;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.concurrent.locks.StampedLock;
import java.util.function.Consumer;
import java.util.function.LongSupplier;
import java.util.function.Supplier;
Expand Down Expand Up @@ -249,24 +246,20 @@ public void startupState( boolean clean )
private volatile boolean changesSinceLastCheckpoint;

/**
* Lock with two individual parts. Writer lock and cleaner lock.
* <p>
* There are a few different scenarios that involve writing or flushing that can not be happen concurrently:
* <ul>
* <li>Checkpoint and writing</li>
* <li>Checkpoint and close</li>
* <li>Write and checkpoint</li>
* </ul>
* This lock is acquired as part of all of those tasks.
*/
private final Lock writerLock = new ReentrantLock();

/**
* If cleaning of crash pointers is needed the tree can not be allowed to perform a checkpoint until that job
* has finished. Therefore this lock is acquired by checkpoint and when cleaning is needed.
* For those scenarios, writer lock is taken.
* <p>
* The cleaner job is responsible for releasing this lock after it has finished. This job will be executed by
* some other thread, that's why this lock is a {@link StampedLock}.
* If cleaning of crash pointers is needed the tree can not be allowed to perform a checkpoint until that job
* has finished. For this scenario, cleaner lock is taken.
*/
private final StampedLock cleanerLock = new StampedLock();
private final GBPTreeLock lock = new GBPTreeLock();

/**
* Page size, i.e. tree node size, of the tree nodes in this tree. The page size is determined on
Expand Down Expand Up @@ -771,8 +764,8 @@ private void checkpoint( IOLimiter ioLimiter, Header.Writer headerWriter ) throw

// Block writers, or if there's a current writer then wait for it to complete and then block
// From this point and till the lock is released we know that the tree won't change.
long stamp = cleanerLock.writeLock();
writerLock.lock();
lock.cleanerLock();
lock.writerLock();
try
{
// Flush dirty pages since that last flush above. This should be a very small set of pages
Expand All @@ -799,8 +792,8 @@ private void checkpoint( IOLimiter ioLimiter, Header.Writer headerWriter ) throw
{
// Unblock writers, any writes after this point and up until the next checkpoint will have
// the new unstable generation.
writerLock.unlock();
cleanerLock.unlockWrite( stamp );
lock.writerUnlock();
lock.cleanerUnlock();
}
}

Expand All @@ -822,7 +815,7 @@ private void assertRecoveryCleanSuccessful() throws IOException
@Override
public void close() throws IOException
{
writerLock.lock();
lock.writerLock();
try
{
if ( closed )
Expand All @@ -847,7 +840,7 @@ public void close() throws IOException
}
finally
{
writerLock.unlock();
lock.writerUnlock();
}
}

Expand Down Expand Up @@ -923,7 +916,7 @@ private CleanupJob createCleanupJob( boolean needsCleaning ) throws IOException
}
else
{
long stamp = cleanerLock.writeLock();
lock.cleanerLock();

long generation = this.generation;
long stableGeneration = stableGeneration( generation );
Expand All @@ -933,7 +926,7 @@ private CleanupJob createCleanupJob( boolean needsCleaning ) throws IOException
CrashGenerationCleaner crashGenerationCleaner =
new CrashGenerationCleaner( pagedFile, bTreeNode, IdSpace.MIN_TREE_NODE_ID, highTreeNodeId,
stableGeneration, unstableGeneration, monitor );
return new GBPTreeCleanupJob( crashGenerationCleaner, cleanerLock, stamp );
return new GBPTreeCleanupJob( crashGenerationCleaner, lock );
}
}

Expand Down Expand Up @@ -1016,13 +1009,13 @@ private class SingleWriter implements Writer<KEY,VALUE>
* Either fully initialized:
* <ul>
* <li>{@link #writerTaken} - true</li>
* <li>{@link #writerLock} - locked</li>
* <li>{@link #lock} - writerLock locked</li>
* <li>{@link #cursor} - not null</li>
* </ul>
* Of fully closed:
* <ul>
* <li>{@link #writerTaken} - false</li>
* <li>{@link #writerLock} - unlocked</li>
* <li>{@link #lock} - writerLock unlocked</li>
* <li>{@link #cursor} - null</li>
* </ul>
*
Expand All @@ -1040,7 +1033,7 @@ void initialize() throws IOException
boolean success = false;
try
{
writerLock.lock();
lock.writerLock();
cursor = openRootCursor( PagedFile.PF_SHARED_WRITE_LOCK );
stableGeneration = stableGeneration( generation );
unstableGeneration = unstableGeneration( generation );
Expand Down Expand Up @@ -1144,7 +1137,7 @@ public void close() throws IOException
", but writer is already closed." );
}
closeCursor();
writerLock.unlock();
lock.writerUnlock();
}

private void closeCursor()
Expand Down
Expand Up @@ -20,26 +20,22 @@
package org.neo4j.index.internal.gbptree;

import java.io.IOException;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.StampedLock;

class GBPTreeCleanupJob implements CleanupJob
{
private final CrashGenerationCleaner crashGenerationCleaner;
private final StampedLock stampedLock;
private final long stamp;
private final GBPTreeLock gbpTreeLock;
private volatile boolean needed;
private volatile Exception failure;

/**
* @param crashGenerationCleaner {@link CrashGenerationCleaner} to use for cleaning.
* @param lock {@link Lock} to be released when job has either successfully finished or failed.
* @param gbpTreeLock {@link GBPTreeLock} to be released when job has either successfully finished or failed.
*/
GBPTreeCleanupJob( CrashGenerationCleaner crashGenerationCleaner, StampedLock lock, long stamp )
GBPTreeCleanupJob( CrashGenerationCleaner crashGenerationCleaner, GBPTreeLock gbpTreeLock )
{
this.crashGenerationCleaner = crashGenerationCleaner;
this.stampedLock = lock;
this.stamp = stamp;
this.gbpTreeLock = gbpTreeLock;
this.needed = true;

}
Expand Down Expand Up @@ -76,7 +72,7 @@ public void run()
}
finally
{
stampedLock.unlockWrite( stamp );
gbpTreeLock.cleanerUnlock();
}
}
}
@@ -0,0 +1,101 @@
/*
* Copyright (c) 2002-2017 "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.index.internal.gbptree;

import org.neo4j.unsafe.impl.internal.dragons.UnsafeUtil;

class GBPTreeLock
{
private static final long stateOffset = UnsafeUtil.getFieldOffset( GBPTreeLock.class, "state" );
private static final long writerLockBit = 0x00000000_00000001L;
private static final long cleanerLockBit = 0x00000000_00000002L;
private volatile long state;

void writerLock()
{
doLock( writerLockBit );
}

void writerUnlock()
{
doUnlock( writerLockBit );
}

void cleanerLock()
{
doLock( cleanerLockBit );
}

void cleanerUnlock()
{
doUnlock( cleanerLockBit );
}

private void doLock( long targetLockBit )
{
long currentState;
long newState;
do
{
currentState = state;
while ( isLocked( currentState, targetLockBit ) )
{
// sleep
sleep();
currentState = state;
}
newState = currentState | targetLockBit;
} while ( !UnsafeUtil.compareAndSwapLong( this, stateOffset, currentState, newState ) );
}

private boolean isLocked( long state, long targetLockBit )
{
return (state & targetLockBit) == targetLockBit;
}

private void doUnlock( long targetLockBit )
{
long currentState;
long newState;
do
{
currentState = state;
if ( !isLocked( currentState, targetLockBit) )
{
throw new IllegalStateException( "Can not unlock lock that is already locked" );
}
newState = currentState & ~targetLockBit;
}
while ( !UnsafeUtil.compareAndSwapLong( this, stateOffset, currentState, newState ) );
}

private void sleep()
{
try
{
Thread.sleep( 10 );
}
catch ( InterruptedException e )
{
// todo what to do in this case
throw new RuntimeException( e );
}
}
}

0 comments on commit 7705a2b

Please sign in to comment.