code.onehippo.org is currently readonly. We are migrating to code.bloomreach.com, please continue working there on Monday 14/12. See: https://docs.bloomreach.com/display/engineering/GitLab

Commit b51f32d9 authored by Ard Schrijvers's avatar Ard Schrijvers

REPO-1811 Remove the 'refreshRateSeconds' logic

Just fixed 60 seconds refresh rate
parent 589c19b8
......@@ -42,6 +42,7 @@ public abstract class AbstractLockManager implements InternalLockManager {
private static final long DEFAULT_SCHEDULED_JOBS_INITIAL_DELAY_SECONDS = 5;
private static final long DEFAULT_SCHEDULED_JOBS_INTERVAL_SECONDS = 5;
public static final long REFRESH_RATE_SECONDS = 60;
private long longestIntervalSeconds = DEFAULT_SCHEDULED_JOBS_INTERVAL_SECONDS;
......@@ -50,7 +51,7 @@ public abstract class AbstractLockManager implements InternalLockManager {
protected abstract Logger getLogger();
protected abstract MutableLock createLock(String key, String threadName, int refreshRateSeconds) throws LockException;
protected abstract MutableLock createLock(String key, String threadName) throws LockException;
protected abstract void releasePersistedLock(String key, String threadName);
......@@ -70,22 +71,12 @@ public abstract class AbstractLockManager implements InternalLockManager {
@Override
public synchronized void lock(final String key) throws LockException {
lock(key, 60);
}
@Override
public synchronized void lock(final String key, int refreshRateSeconds) throws LockException {
checkLive();
validateKey(key);
if (refreshRateSeconds < 60) {
getLogger().warn("refreshRateSeconds not allowed to be '{}'. Must be at least 60 or higher. Using 60 now.",
refreshRateSeconds);
refreshRateSeconds = 60;
}
final MutableLock lock = localLocks.get(key);
if (lock == null) {
getLogger().debug("Create lock '{}' for thread '{}'", key, Thread.currentThread().getName());
localLocks.put(key, createLock(key, Thread.currentThread().getName(), refreshRateSeconds));
localLocks.put(key, createLock(key, Thread.currentThread().getName()));
return;
}
final Thread lockThread = lock.getThread().get();
......@@ -93,7 +84,7 @@ public abstract class AbstractLockManager implements InternalLockManager {
getLogger().warn("Thread '{}' that created lock for '{}' has stopped without releasing the lock. Thread '{}' " +
"now gets the lock", lock.getLockThread(), key, Thread.currentThread().getName());
unlock(key);
localLocks.put(key, createLock(key, Thread.currentThread().getName(), refreshRateSeconds));
localLocks.put(key, createLock(key, Thread.currentThread().getName()));
return;
}
if (lockThread == Thread.currentThread()) {
......
......@@ -20,9 +20,7 @@ import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.stream.Stream;
import javax.sql.DataSource;
......@@ -50,11 +48,10 @@ public class DbLockManager extends AbstractLockManager {
"lockThread VARCHAR(256)," +
"status VARCHAR(256) NOT NULL," +
"lockTime BIGINT," +
"refreshRateSeconds SMALLINT," +
"expirationTime BIGINT" +
")";
public static final String CREATE_STATEMENT = "INSERT INTO " + TABLE_NAME_LOCK + " VALUES(?,?,?,'RUNNING',?,?,?)";
public static final String CREATE_STATEMENT = "INSERT INTO " + TABLE_NAME_LOCK + " VALUES(?,?,?,'RUNNING',?,?)";
public static final String SELECT_STATEMENT = "SELECT * FROM " + TABLE_NAME_LOCK + " WHERE lockKey=?";
public static final String LOCK_STATEMENT = "UPDATE " + TABLE_NAME_LOCK + " SET status='RUNNING', lockTime=?, expirationTime=?, lockOwner=?, lockThread=? WHERE lockKey=? AND status='FREE'";
......@@ -65,7 +62,6 @@ public class DbLockManager extends AbstractLockManager {
"lockThread=NULL, " +
"status='FREE', " +
"lockTime=0, " +
"refreshRateSeconds=0, " +
"expirationTime=0 " +
"WHERE lockKey=? AND lockOwner=? AND lockThread=?";
......@@ -75,15 +71,14 @@ public class DbLockManager extends AbstractLockManager {
"lockThread=NULL, " +
"status='FREE', " +
"lockTime=0, " +
"refreshRateSeconds=0, " +
"expirationTime=0 " +
"WHERE expirationTime<? AND (status='RUNNING' OR status='ABORT')";
public static final String ABORT_STATEMENT = "UPDATE " + TABLE_NAME_LOCK + " SET status='ABORT' WHERE lockKey=? AND status='RUNNING'";
// only refreshes its own cluster locks
public static final String REFRESH_LOCK_STATEMENT = "UPDATE " + TABLE_NAME_LOCK + " SET expirationTime=expirationTime+60000 " +
"WHERE lockOwner=? AND expirationTime<? AND (status='RUNNING' OR status='ABORT')";
public static final String REFRESH_LOCK_STATEMENT = "UPDATE " + TABLE_NAME_LOCK + " SET expirationTime=expirationTime+"+ REFRESH_RATE_SECONDS * 1000 +
" WHERE lockOwner=? AND expirationTime<? AND (status='RUNNING' OR status='ABORT')";
public static final String SELECT_ABORT_STATEMENT = "SELECT * FROM " + TABLE_NAME_LOCK + " WHERE status='ABORT' AND lockOwner=?";
......@@ -107,7 +102,7 @@ public class DbLockManager extends AbstractLockManager {
}
@Override
protected synchronized MutableLock createLock(final String key, final String threadName, final int refreshRateSeconds) throws LockException {
protected synchronized MutableLock createLock(final String key, final String threadName) throws LockException {
Connection connection = null;
boolean originalAutoCommit = false;
try {
......@@ -116,7 +111,7 @@ public class DbLockManager extends AbstractLockManager {
connection.setAutoCommit(true);
final long lockTime = System.currentTimeMillis();
final long expirationTime = lockTime + refreshRateSeconds * 1000;
final long expirationTime = lockTime + REFRESH_RATE_SECONDS * 1000;
final PreparedStatement lockStatement = connection.prepareStatement(LOCK_STATEMENT);
lockStatement.setLong(1, lockTime);
lockStatement.setLong(2, expirationTime);
......@@ -134,8 +129,7 @@ public class DbLockManager extends AbstractLockManager {
createStatement.setString(2, clusterNodeId);
createStatement.setString(3, threadName);
createStatement.setLong(4, lockTime);
createStatement.setInt(5, refreshRateSeconds);
createStatement.setLong(6, expirationTime);
createStatement.setLong(5, expirationTime);
try {
createStatement.execute();
createStatement.close();
......
......@@ -40,7 +40,7 @@ public class MemoryLockManager extends AbstractLockManager {
}
@Override
protected MutableLock createLock(final String key, final String threadName, final int refreshRateSeconds) throws LockException {
protected MutableLock createLock(final String key, final String threadName) throws LockException {
return new MutableLock(key, "default", threadName, System.currentTimeMillis(), "RUNNING");
}
......
......@@ -36,6 +36,7 @@ import org.onehippo.repository.testutils.RepositoryTestCase;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
import static org.onehippo.repository.lock.AbstractLockManager.REFRESH_RATE_SECONDS;
import static org.onehippo.repository.lock.db.DbLockManager.CREATE_STATEMENT;
import static org.onehippo.repository.lock.db.DbLockManager.SELECT_STATEMENT;
......@@ -114,7 +115,7 @@ public abstract class AbstractLockManagerTest extends RepositoryTestCase {
}
protected void addManualLockToDatabase(final String key, final String clusterNodeId,
final String threadName, final int refreshRateSeconds) throws LockException {
final String threadName) throws LockException {
if (dataSource != null) {
try (Connection connection = dataSource.getConnection()) {
......@@ -124,8 +125,7 @@ public abstract class AbstractLockManagerTest extends RepositoryTestCase {
createStatement.setString(3, threadName);
long lockTime = System.currentTimeMillis();
createStatement.setLong(4, lockTime);
createStatement.setLong(5, refreshRateSeconds);
createStatement.setLong(6, lockTime + refreshRateSeconds * 1000);
createStatement.setLong(5, lockTime + REFRESH_RATE_SECONDS * 1000);
try {
createStatement.execute();
} catch (SQLException e) {
......@@ -147,8 +147,7 @@ public abstract class AbstractLockManagerTest extends RepositoryTestCase {
createStatement.setString(2, clusterId);
createStatement.setString(3, threadName);
createStatement.setLong(4, lockTime);
createStatement.setInt(5, 60);
createStatement.setLong(6, expirationTime);
createStatement.setLong(5, expirationTime);
createStatement.execute();
}
}
......
......@@ -287,8 +287,8 @@ public class LockManagerBasicTest extends AbstractLockManagerTest {
lockManager.lock("b");
// insert manually non-owned rows
addManualLockToDatabase("c", "otherNode", "otherThreadName", 60);
addManualLockToDatabase("d", "otherNode", "otherThreadName", 60);
addManualLockToDatabase("c", "otherNode", "otherThreadName");
addManualLockToDatabase("d", "otherNode", "otherThreadName");
dbRowAssertion("a", "RUNNING", "node1", Thread.currentThread().getName());
dbRowAssertion("b", "RUNNING");
......@@ -327,8 +327,8 @@ public class LockManagerBasicTest extends AbstractLockManagerTest {
lockManager.lock("b");
// insert manually non-owned rows
addManualLockToDatabase("c", "otherNode", "otherThreadName", 60);
addManualLockToDatabase("d", "otherNode", "otherThreadName", 60);
addManualLockToDatabase("c", "otherNode", "otherThreadName");
addManualLockToDatabase("d", "otherNode", "otherThreadName");
List<Lock> locks = lockManager.getLocks();
if (dataSource == null) {
......@@ -351,8 +351,8 @@ public class LockManagerBasicTest extends AbstractLockManagerTest {
assertTrue(lockManager.isLocked("a"));
assertTrue(lockManager.isLocked("b"));
if (dataSource != null) {
addManualLockToDatabase("c", "otherNode", "otherThreadName", 60);
addManualLockToDatabase("d", "otherNode", "otherThreadName", 60);
addManualLockToDatabase("c", "otherNode", "otherThreadName");
addManualLockToDatabase("d", "otherNode", "otherThreadName");
assertTrue(lockManager.isLocked("c"));
assertTrue(lockManager.isLocked("d"));
}
......
......@@ -27,7 +27,6 @@ import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.onehippo.repository.lock.db.DbLockManager.CREATE_STATEMENT;
import static org.onehippo.repository.lock.db.DbLockManager.TABLE_NAME_LOCK;
public class LockManagerRefreshTest extends AbstractLockManagerTest {
......@@ -40,7 +39,7 @@ public class LockManagerRefreshTest extends AbstractLockManagerTest {
return;
}
final String key = "123";
final LockRunnable runnable = new LockRunnable(key, true, 60);
final LockRunnable runnable = new LockRunnable(key, true);
final Thread lockThread = new Thread(runnable);
lockThread.start();
......@@ -62,6 +61,10 @@ public class LockManagerRefreshTest extends AbstractLockManagerTest {
Thread.sleep(100);
}
// assert expires time got exactly bumped 60000 millis, see org.onehippo.repository.lock.db.DbLockManager.REFRESH_LOCK_STATEMENT
// at 'SET expirationTime=expirationTime+"+ REFRESH_RATE_SECONDS * 1000'
assertEquals(expires+60_000, getExpireTime(key));
runnable.keepAlive = false;
// after the thread is finished, the lock manager should have no locks any more
lockThread.join();
......@@ -74,13 +77,13 @@ public class LockManagerRefreshTest extends AbstractLockManagerTest {
return;
}
final String key1 = "123";
final LockRunnable runnable = new LockRunnable(key1, true, 60);
final LockRunnable runnable = new LockRunnable(key1, true);
final Thread lockThread = new Thread(runnable);
lockThread.start();
final String key2 = "456";
final LockRunnable runnable2 = new LockRunnable(key2, true, 60, true);
final LockRunnable runnable2 = new LockRunnable(key2, true, true);
final Thread lockThread2 = new Thread(runnable2);
lockThread2.start();
......@@ -120,6 +123,12 @@ public class LockManagerRefreshTest extends AbstractLockManagerTest {
long expireAfter3 = getExpireTime(key3);
assertEquals("The expiration time of a lock in possession of a different cluster node should not be refreshed",
expirationTimeOtherClusterNodeLock, expireAfter3);
// end locks
runnable.keepAlive = false;
runnable2.keepAlive = false;
lockThread.join();
lockThread2.join();
}
......@@ -151,26 +160,22 @@ public class LockManagerRefreshTest extends AbstractLockManagerTest {
private String key;
private volatile boolean keepAlive;
private int refreshRateSeconds;
private boolean ignoreInterruption;
LockRunnable(final String key , final boolean keepAlive,
final int refreshRateSeconds) {
this(key, keepAlive, refreshRateSeconds, false);
LockRunnable(final String key , final boolean keepAlive) {
this(key, keepAlive, false);
}
LockRunnable(final String key , final boolean keepAlive,
final int refreshRateSeconds, final boolean ignoreInterruption) {
LockRunnable(final String key , final boolean keepAlive, final boolean ignoreInterruption) {
this.key = key;
this.keepAlive = keepAlive;
this.refreshRateSeconds = refreshRateSeconds;
this.ignoreInterruption = ignoreInterruption;
}
@Override
public void run() {
try {
lockManager.lock(key, refreshRateSeconds);
lockManager.lock(key);
while (keepAlive) {
Thread.sleep(25);
}
......@@ -185,7 +190,8 @@ public class LockManagerRefreshTest extends AbstractLockManagerTest {
}
}
}
fail(e.toString());
} finally {
lockManager.unlock(key);
}
}
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment