From c996970a61dd1abbb468544b27276cdb0cd849e7 Mon Sep 17 00:00:00 2001 From: Florian Habermann Date: Wed, 9 Oct 2024 15:09:45 +0200 Subject: [PATCH] Ensure lock file exists (#282) * Ensure lock file exists * fixing lock manager bugs * reworking threading * clear buffer instead of setting limit --------- Co-authored-by: hg-ms <53219833+hg-ms@users.noreply.github.com> --- .../store/storage/types/StorageFile.java | 7 +- .../storage/types/StorageLockFileManager.java | 353 ++++++++++-------- .../StorageLockFileManagerThreadProvider.java | 35 +- .../store/storage/types/StorageSystem.java | 43 ++- .../storage/types/StorageThreadProvider.java | 4 +- 5 files changed, 256 insertions(+), 186 deletions(-) diff --git a/storage/storage/src/main/java/org/eclipse/store/storage/types/StorageFile.java b/storage/storage/src/main/java/org/eclipse/store/storage/types/StorageFile.java index 88158075..e37a2a0e 100644 --- a/storage/storage/src/main/java/org/eclipse/store/storage/types/StorageFile.java +++ b/storage/storage/src/main/java/org/eclipse/store/storage/types/StorageFile.java @@ -83,9 +83,11 @@ public default String identifier() public boolean delete(); public void moveTo(AWritableFile target); - - + public void truncate(final long newLength); + + + public static VarString assembleNameAndSize(final VarString vs, final StorageFile file) { return vs.add(file.file().identifier() + "[" + file.file().size() + "]"); @@ -361,6 +363,7 @@ public final synchronized long copyFrom( } } + @Override public synchronized void truncate(final long newLength) { this.ensureWritable().truncate(newLength); diff --git a/storage/storage/src/main/java/org/eclipse/store/storage/types/StorageLockFileManager.java b/storage/storage/src/main/java/org/eclipse/store/storage/types/StorageLockFileManager.java index 41189332..e736fb01 100644 --- a/storage/storage/src/main/java/org/eclipse/store/storage/types/StorageLockFileManager.java +++ b/storage/storage/src/main/java/org/eclipse/store/storage/types/StorageLockFileManager.java @@ -17,6 +17,12 @@ import static org.eclipse.serializer.util.X.notNull; import java.nio.ByteBuffer; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import org.eclipse.serializer.afs.types.AFile; import org.eclipse.serializer.afs.types.AFileSystem; @@ -24,55 +30,82 @@ import org.eclipse.serializer.chars.XChars; import org.eclipse.serializer.collections.ArrayView; import org.eclipse.serializer.collections.XArrays; -import org.eclipse.serializer.concurrency.XThreads; import org.eclipse.serializer.memory.XMemory; import org.eclipse.serializer.util.X; +import org.eclipse.serializer.util.logging.Logging; import org.eclipse.store.storage.exceptions.StorageException; -import org.eclipse.store.storage.exceptions.StorageExceptionInitialization; - +import org.slf4j.Logger; +/** + * The StorageLockFileManager purpose is to provide a mechanism that prevents + * other storage instances running in different processes from accessing the + * storage data. + */ public interface StorageLockFileManager extends Runnable { - public default StorageLockFileManager start() - { - this.setRunning(true); - return this; - } + /** + * Start periodical lock file checks. + * + * @return this + */ + public StorageLockFileManager start(); - public default StorageLockFileManager stop() - { - this.setRunning(false); - return this; - } - - public boolean isRunning(); - - public StorageLockFileManager setRunning(boolean running); + /** + * Stop periodical lock file checks + * + * @return this + */ + public StorageLockFileManager stop(); + /** + * Initialize the lock file manager without starting any periodical actions. + */ + public void initialize(); + /** + * Check if the StorageLockFileManager has been successfully initialized and is ready to start. + * + * @return true if {@link #initialize()} was successfully executed. + */ + public boolean isInitialized(); + public static StorageLockFileManager New( - final StorageLockFileSetup setup , - final StorageOperationController operationController + final StorageLockFileSetup setup , + final StorageOperationController operationController, + final StorageLockFileManagerThreadProvider threadProvider ) { return new StorageLockFileManager.Default( notNull(setup), - notNull(operationController) + notNull(operationController), + notNull(threadProvider) ); } + /** + * Default implementation of the #StorageLockFileManager. + * This implementation uses a file to indicate if the storage data + * is already in use by a storage instance. + * The file contains a process depended ID and time-stamps of the last modification + * and expiring time. + * A storage is accessible if: + * - no lock file exists + * - a lock file exists and the process id matches + * - a lock file exists and the current system time is greater than the expiring time + update interval + */ public final class Default implements StorageLockFileManager { + private final static Logger logger = Logging.getLogger(StorageLockFileManager.class); + /////////////////////////////////////////////////////////////////////////// // instance fields // //////////////////// - private final StorageLockFileSetup setup ; - private final StorageOperationController operationController; - + private final StorageLockFileSetup setup ; + private final StorageOperationController operationController; + // cached values - private transient boolean isRunning ; private transient StorageLockFile lockFile ; private transient LockFileData lockFileData ; private transient ByteBuffer[] wrappedByteBuffer ; @@ -80,9 +113,9 @@ public final class Default implements StorageLockFileManager private transient ByteBuffer directByteBuffer ; private transient byte[] stringReadBuffer ; private transient byte[] stringWriteBuffer ; - private transient VarString vs ; - private transient AFileSystem fileSystem ; - + private transient VarString vs ; + private transient AFileSystem fileSystem ; + private transient ScheduledExecutorService executor ; /////////////////////////////////////////////////////////////////////////// @@ -90,8 +123,9 @@ public final class Default implements StorageLockFileManager ///////////////// Default( - final StorageLockFileSetup setup , - final StorageOperationController operationController + final StorageLockFileSetup setup , + final StorageOperationController operationController, + final StorageLockFileManagerThreadProvider threadProvider ) { super(); @@ -99,7 +133,6 @@ public final class Default implements StorageLockFileManager this.fileSystem = setup.lockFileProvider().fileSystem(); this.operationController = operationController; this.vs = VarString.New() ; - // 2 timestamps with separators and an identifier. Should suffice. this.wrappedByteBuffer = new ByteBuffer[1]; this.wrappedWrappedByteBuffer = X.ArrayView(this.wrappedByteBuffer); @@ -107,6 +140,8 @@ public final class Default implements StorageLockFileManager this.stringReadBuffer = new byte[64]; this.stringWriteBuffer = this.stringReadBuffer.clone(); this.allocateBuffer(this.stringReadBuffer.length); + + this.executor = Executors.newSingleThreadScheduledExecutor(threadProvider); } @@ -115,92 +150,169 @@ public final class Default implements StorageLockFileManager // methods // //////////// + @Override - public final synchronized boolean isRunning() - { - return this.isRunning; - } - - @Override - public final synchronized StorageLockFileManager setRunning(final boolean running) + public final synchronized boolean isInitialized() { - this.isRunning = running; - - return this; + return this.lockFile != null; } - private synchronized boolean checkIsRunning() + private synchronized boolean isReady() { - return this.isRunning && this.operationController.checkProcessingEnabled(); + boolean result = this.isInitialized() && this.operationController.checkProcessingEnabled(); + logger.trace("Storage LockFile Manager isReady: {}" , result); + return result; } @Override public StorageLockFileManager.Default start() { - this.ensureInitialized(); - StorageLockFileManager.super.start(); + logger.info("Starting log file manager thread "); + this.executor.scheduleWithFixedDelay(this, 0, this.setup.updateInterval(), TimeUnit.MICROSECONDS); return this; } @Override public final void run() { - final long updateInterval = this.setup.updateInterval(); - - Throwable closingCause = null; try { - this.checkInitialized(); - - // wait first after the initial write, then perform the regular update - while(this.checkIsRunning()) + if(this.isReady()) { - XThreads.sleep(updateInterval); this.updateFile(); } + else + { + logger.error("Lock File Manager is not ready!"); + } } catch(final Exception e) { - closingCause = e; + this.stop(); this.operationController.registerDisruption(e); throw e; } - finally - { - // ensure closed file in any case. Regular shutdown or forceful shutdown by exception. - this.ensureClosedLockFile(closingCause); - } } - private void ensureInitialized() + @Override + public StorageLockFileManager stop() { - if(this.lockFile != null) + if(this.executor.isShutdown()) return this; + + this.executor.shutdown(); + try { - return; + if(!this.executor.awaitTermination(100, TimeUnit.MILLISECONDS)) + { + this.executor.shutdownNow(); + if (!this.executor.awaitTermination(100, TimeUnit.MILLISECONDS)) + { + logger.error("Failed to shutdown StorageLockFileManager Service executor!"); + } + } } - - try + catch(InterruptedException e) { - this.initialize(); + this.executor.shutdownNow(); + Thread.currentThread().interrupt(); } - catch(final Exception e) + finally { - this.operationController.registerDisruption(e); - this.ensureClosedLockFile(e); - throw e; + this.ensureClosedLockFile(null); } + + logger.info("Storage Lock File Manager stopped"); + + return this; } - private void checkInitialized() + /** + * Initialize the storage lock file manager without starting the periodical + * lock file check. + */ + @Override + public void initialize() { - if(this.lockFile != null) + logger.info("initializing lock file manager for storage {}", this.setup.processIdentity()); + + final StorageLiveFileProvider fileProvider = this.setup.lockFileProvider(); + final AFile lockFile = fileProvider.provideLockFile(); + this.lockFile = StorageLockFile.New(lockFile); + + + LockFileData initialFileData = null; + if(this.lockFileHasContent()) { + + initialFileData = this.readLockFileData(); + if(!this.validateExistingLockFileData(initialFileData)) + { + // wait one interval and try a second time + logger.warn("Non expired storage lock found! Retrying once"); + + ScheduledFuture future = this.executor.schedule(() -> + this.validateExistingLockFileData(this.readLockFileData()), + initialFileData.updateInterval, + TimeUnit.MILLISECONDS); + + try { + if(!future.get(initialFileData.updateInterval *2, TimeUnit.MILLISECONDS)) { + this.executor.shutdownNow(); + throw new StorageException("Storage already in use by: " + initialFileData.identifier); + } + } catch(InterruptedException | ExecutionException | TimeoutException e) { + this.executor.shutdownNow(); + throw new StorageException("failed to validate lock file", e); + } + } + + + } + + if(this.isReadOnlyMode()) { + if(initialFileData != null) + { + // write buffer must be filled with the file's current content so the check will be successful. + this.setToWriteBuffer(initialFileData); + } + + // abort, since neither lockFileData nor writing is required/allowed in read-only mode. return; } + + this.lockFileData = new LockFileData(this.setup.processIdentity(), this.setup.updateInterval()); - throw new StorageExceptionInitialization(StorageLockFileManager.class.getSimpleName() + " not initialized."); + this.lockFile.file().ensureExists(); + this.writeLockFileData(); } + + private boolean validateExistingLockFileData(final LockFileData lockFileData) + { + + final String identifier = this.setup.processIdentity(); + if(identifier.equals(lockFileData.identifier)) + { + // database is already owned by "this" process (e.g. crash shorty before), so just continue and reuse. + logger.info("Storage already owned by process!"); + return true; + } + + if(lockFileData.isLongExpired()) + { + /* + * The lock file is no longer updated, meaning the database is not used anymore + * and the lockfile is just a zombie, probably left by a crash. + */ + logger.info("Storage lock file outdated, aquiring storage!"); + return true; + } + + logger.debug("Storage lock file not validated! Owner {}, expire time {}", lockFileData.identifier, lockFileData.expirationTime); + + return false; + } + private ByteBuffer ensureReadingBuffer(final int fileLength) { this.ensureBufferCapacity(fileLength); @@ -209,7 +321,7 @@ private ByteBuffer ensureReadingBuffer(final int fileLength) this.stringReadBuffer = new byte[fileLength]; } - this.directByteBuffer.limit(fileLength); + this.directByteBuffer.clear(); return this.directByteBuffer; } @@ -217,7 +329,7 @@ private ByteBuffer ensureReadingBuffer(final int fileLength) private ArrayView ensureWritingBuffer(final byte[] bytes) { this.ensureBufferCapacity(bytes.length); - this.directByteBuffer.limit(bytes.length); + this.directByteBuffer.clear(); this.stringWriteBuffer = bytes; @@ -261,7 +373,7 @@ private void fillReadBufferFromFile() this.lockFile.readBytes(this.ensureReadingBuffer(fileLength), 0, fileLength); XMemory.copyRangeToArray(XMemory.getDirectByteBufferAddress(this.directByteBuffer), this.stringReadBuffer); } - + private LockFileData readLockFileData() { final String currentFileData = this.readString(); @@ -348,69 +460,7 @@ private boolean lockFileHasContent() { return this.lockFile.exists() && this.lockFile.size() > 0; } - - private void initialize() - { - final StorageLiveFileProvider fileProvider = this.setup.lockFileProvider(); - final AFile lockFile = fileProvider.provideLockFile(); - this.lockFile = StorageLockFile.New(lockFile); - - final LockFileData initialFileData = this.lockFileHasContent() - ? this.validateExistingLockFileData(true) - : null - ; - - if(this.isReadOnlyMode()) - { - if(initialFileData != null) - { - // write buffer must be filled with the file's current content so the check will be successful. - this.setToWriteBuffer(initialFileData); - } - - // abort, since neither lockFileData nor writing is required/allowed in read-only mode. - return; - } - - this.lockFileData = new LockFileData(this.setup.processIdentity(), this.setup.updateInterval()); - - this.writeLockFileData(); - } - private LockFileData validateExistingLockFileData(final boolean firstAttempt) - { - final LockFileData existingFiledata = this.readLockFileData(); - - final String identifier = this.setup.processIdentity(); - if(identifier.equals(existingFiledata.identifier)) - { - // database is already owned by "this" process (e.g. crash shorty before), so just continue and reuse. - return existingFiledata; - } - - if(existingFiledata.isLongExpired()) - { - /* - * The lock file is no longer updated, meaning the database is not used anymore - * and the lockfile is just a zombie, probably left by a crash. - */ - return existingFiledata; - } - - // not owned and not expired - if(firstAttempt) - { - // wait one interval and try a second time - XThreads.sleep(existingFiledata.updateInterval); - return this.validateExistingLockFileData(false); - - // returning here means no exception (but expiration) on the second attempt, meaning success. - } - - // not owned, not expired and still active, meaning really still in use, so exception - - throw new StorageException("Storage already in use by: " + existingFiledata.identifier); - } private void checkForModifiedLockFile() { @@ -445,12 +495,15 @@ private void writeLockFileData() } this.lockFileData.update(); - - + final ArrayView bb = this.setToWriteBuffer(this.lockFileData); // no need for the writer detour (for now) since it makes no sense to backup lock files. + + //don't delete file! + this.lockFile.truncate(0); this.lockFile.writeBytes(bb); + } private ArrayView setToWriteBuffer(final LockFileData lockFileData) @@ -465,18 +518,14 @@ private ArrayView setToWriteBuffer(final LockFileData lockFileData) final ArrayView bb = this.ensureWritingBuffer(bytes); XMemory.copyArrayToAddress(bytes, XMemory.getDirectByteBufferAddress(this.directByteBuffer)); + this.directByteBuffer.limit(bytes.length); return bb; } private void updateFile() { - // check again after the wait time. - if(!this.checkIsRunning()) - { - // abort to avoid an unnecessary write. - return; - } + logger.trace("updating lock file"); this.checkForModifiedLockFile(); this.writeLockFileData(); @@ -488,7 +537,8 @@ private void ensureClosedLockFile(final Throwable cause) { return; } - + + logger.debug("closing lockfile!"); StorageClosableFile.close(this.lockFile, cause); this.lockFile = null; } @@ -504,8 +554,9 @@ public static Creator Creator() public interface Creator { public StorageLockFileManager createLockFileManager( - StorageLockFileSetup setup , - StorageOperationController operationController + StorageLockFileSetup setup , + StorageOperationController operationController, + StorageLockFileManagerThreadProvider threadProvider ); public final class Default implements StorageLockFileManager.Creator @@ -517,13 +568,15 @@ public final class Default implements StorageLockFileManager.Creator @Override public StorageLockFileManager createLockFileManager( - final StorageLockFileSetup setup , - final StorageOperationController operationController + final StorageLockFileSetup setup , + final StorageOperationController operationController, + final StorageLockFileManagerThreadProvider threadProvider ) { return StorageLockFileManager.New( setup , - operationController + operationController, + threadProvider ); } diff --git a/storage/storage/src/main/java/org/eclipse/store/storage/types/StorageLockFileManagerThreadProvider.java b/storage/storage/src/main/java/org/eclipse/store/storage/types/StorageLockFileManagerThreadProvider.java index c30ba73c..47ec27b7 100644 --- a/storage/storage/src/main/java/org/eclipse/store/storage/types/StorageLockFileManagerThreadProvider.java +++ b/storage/storage/src/main/java/org/eclipse/store/storage/types/StorageLockFileManagerThreadProvider.java @@ -1,5 +1,7 @@ package org.eclipse.store.storage.types; +import java.util.concurrent.ThreadFactory; + /*- * #%L * EclipseStore Storage @@ -15,25 +17,30 @@ */ @FunctionalInterface -public interface StorageLockFileManagerThreadProvider extends StorageThreadProviding +public interface StorageLockFileManagerThreadProvider extends StorageThreadProviding, ThreadFactory { /** - * Provides a newly created, yet un-started {@link Thread} instance wrapping the passed - * {@link StorageLockFileManager} instance. - * The thread will be used as an exclusive, permanent lock file validator and updater worker thread - * until the storage is shut down. - * Interfering with the thread from outside the storage compound has undefined and potentially - * unpredictable and erroneous behavior. - * @param lockFileManager the lock file manager to wrap - * @return a {@link Thread} instance to be used as a storage lock file managing worker thread. + * Provide a ThreadFactory for the StorageLockFileManager that uses the configured StorageThreadNameProvider + * to name threads. */ - public default Thread provideLockFileManagerThread(final StorageLockFileManager lockFileManager) + @Override + public default Thread newThread(final Runnable runnable) { - return this.provideLockFileManagerThread(lockFileManager, StorageThreadNameProvider.NoOp()); + return this.provideLockFileManagerThread(runnable, StorageThreadNameProvider.NoOp()); } + + + @Deprecated + public default Thread provideLockFileManagerThread(final Runnable runnable) + { + return this.provideLockFileManagerThread(runnable, StorageThreadNameProvider.NoOp()); + } + + + @Deprecated public Thread provideLockFileManagerThread( - StorageLockFileManager lockFileManager , + Runnable runnable , StorageThreadNameProvider threadNameProvider ); @@ -53,14 +60,14 @@ public final class Default implements StorageLockFileManagerThreadProvider @Override public Thread provideLockFileManagerThread( - final StorageLockFileManager lockFileManager , + final Runnable runnable , final StorageThreadNameProvider threadNameProvider ) { final String threadName = StorageLockFileManager.class.getSimpleName(); return new Thread( - lockFileManager, + runnable, threadNameProvider.provideThreadName(this, threadName) ); } diff --git a/storage/storage/src/main/java/org/eclipse/store/storage/types/StorageSystem.java b/storage/storage/src/main/java/org/eclipse/store/storage/types/StorageSystem.java index 39376ab9..7e95f2ee 100644 --- a/storage/storage/src/main/java/org/eclipse/store/storage/types/StorageSystem.java +++ b/storage/storage/src/main/java/org/eclipse/store/storage/types/StorageSystem.java @@ -122,15 +122,15 @@ public final class Default implements StorageSystem, Unpersistable, StorageKilla private final AtomicLong operationModeTime = new AtomicLong() ; // running state members // - private volatile StorageTaskBroker taskbroker ; - private final ChannelKeeper[] channelKeepers; + private volatile StorageTaskBroker taskbroker ; + private final ChannelKeeper[] channelKeepers; - private StorageBackupHandler backupHandler; - private Thread backupThread ; + private StorageBackupHandler backupHandler; + private Thread backupThread ; - private Thread lockFileManagerThread; + private StorageLockFileManager lockFileManager; - private StorageIdAnalysis initializationIdAnalysis; + private StorageIdAnalysis initializationIdAnalysis; @@ -383,32 +383,37 @@ private void initializeLockFileManager() return; } - final StorageLockFileManager lockFileManager = this.lockFileManagerCreator.createLockFileManager( + this.lockFileManager = this.lockFileManagerCreator.createLockFileManager( this.lockFileSetup, - this.operationController + this.operationController, + this.threadProvider ); - // initialize lock file manager state to being running - lockFileManager.start(); - - // set up a lock file manager thread and start it if initialization (obtaining the "lock") was successful. - this.lockFileManagerThread = this.threadProvider.provideLockFileManagerThread(lockFileManager); - // can't start before the operation controller isn't in proper running state... + // initialize lock file manager state to be ready for running + this.lockFileManager.initialize(); } private void startLockFileManagerThread() { - if(this.lockFileManagerThread == null) + if(this.lockFileManager == null) { // can be null if lock file is not desired. See #initializeLockFileManager return; } - // can't start before the operation controller isn't in proper running state, hence the extra method - this.lockFileManagerThread.start(); + this.lockFileManager.start(); } - + + private void stopLockFileManagerThread() + { + if(this.lockFileManager == null) + { + return; + } + + this.lockFileManager.stop(); + } // "Please do not disturb the Keepers" :-D static final class ChannelKeeper implements StorageActivePart @@ -553,6 +558,8 @@ private void internalShutdown() throws InterruptedException this.operationController.deactivate(); + this.stopLockFileManagerThread(); + this.monitorManager.shutdown(); /* (07.03.2019 TM)FIXME: Shutdown must wait for ongoing activities. diff --git a/storage/storage/src/main/java/org/eclipse/store/storage/types/StorageThreadProvider.java b/storage/storage/src/main/java/org/eclipse/store/storage/types/StorageThreadProvider.java index ef6becbf..d8ebf173 100644 --- a/storage/storage/src/main/java/org/eclipse/store/storage/types/StorageThreadProvider.java +++ b/storage/storage/src/main/java/org/eclipse/store/storage/types/StorageThreadProvider.java @@ -104,7 +104,7 @@ public final Thread provideBackupThread(final StorageBackupHandler backupHandler } @Override - public final Thread provideLockFileManagerThread(final StorageLockFileManager lockFileManager) + public final Thread provideLockFileManagerThread(final Runnable lockFileManager) { return this.lockFileManagerThreadProvider.provideLockFileManagerThread( lockFileManager, @@ -138,7 +138,7 @@ public final Thread provideBackupThread( @Override public final Thread provideLockFileManagerThread( - final StorageLockFileManager lockFileManager , + final Runnable lockFileManager , final StorageThreadNameProvider threadNameProvider ) {