Do not start multiple file watchers for each core state

Prevent starting of multiple file watchers from for LocalDatabase that can be started multiple times, add additional assertion that will prevent multiple watcher threads in future.
neo4j · Mar 2, 2017 · c63b139 · c63b139
1 parent b0b739f
commit c63b139
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 3 deletions.
diff --git a/...nel/src/main/java/org/neo4j/kernel/impl/util/watcher/DefaultFileSystemWatcherService.java b/...nel/src/main/java/org/neo4j/kernel/impl/util/watcher/DefaultFileSystemWatcherService.java
@@ -53,6 +53,7 @@ public void init() throws Throwable
     @Override
     public void start() throws Throwable
     {
+        assert watcher == null;
         watcher = fileWatchers.newThread( eventWatcher );
         watcher.start();
     }

diff --git a/...-clustering/src/main/java/org/neo4j/causalclustering/catchup/storecopy/LocalDatabase.java b/...-clustering/src/main/java/org/neo4j/causalclustering/catchup/storecopy/LocalDatabase.java
@@ -88,11 +88,16 @@ public LocalDatabase( File storeDir, StoreFiles storeFiles, DataSourceManager da
     public void init() throws Throwable
     {
         dataSourceManager.init();
+        watcherService.init();
     }
 
     @Override
     public synchronized void start() throws Throwable
     {
+        if ( isAvailable() )
+        {
+            return;
+        }
         storeId = readStoreIdFromDisk();
         log.info( "Starting with storeId: " + storeId );
 
@@ -127,6 +132,7 @@ public boolean isAvailable()
     @Override
     public void shutdown() throws Throwable
     {
+        watcherService.shutdown();
         dataSourceManager.shutdown();
     }
 

diff --git a/...clustering/src/main/java/org/neo4j/causalclustering/core/EnterpriseCoreEditionModule.java b/...clustering/src/main/java/org/neo4j/causalclustering/core/EnterpriseCoreEditionModule.java
@@ -169,7 +169,6 @@ public void registerEditionSpecificProcedures( Procedures procedures ) throws Ke
         watcherService = createFileSystemWatcherService( fileSystem, storeDir, logging,
                 platformModule.jobScheduler, fileWatcherFileNameFilter() );
         dependencies.satisfyDependencies( watcherService );
-        life.add( watcherService );
         LocalDatabase localDatabase = new LocalDatabase( platformModule.storeDir,
                 new StoreFiles( fileSystem, platformModule.pageCache ),
                 platformModule.dataSourceManager,

diff --git a/.../main/java/org/neo4j/causalclustering/readreplica/EnterpriseReadReplicaEditionModule.java b/.../main/java/org/neo4j/causalclustering/readreplica/EnterpriseReadReplicaEditionModule.java
@@ -130,7 +130,6 @@ public class EnterpriseReadReplicaEditionModule extends EditionModule
         watcherService = createFileSystemWatcherService( fileSystem, storeDir, logging,
                 platformModule.jobScheduler, fileWatcherFileNameFilter() );
         dependencies.satisfyDependencies( watcherService );
-        life.add( watcherService );
 
         GraphDatabaseFacade graphDatabaseFacade = platformModule.graphDatabaseFacade;
 

diff --git a/...stering/src/test/java/org/neo4j/causalclustering/catchup/storecopy/LocalDatabaseTest.java b/...stering/src/test/java/org/neo4j/causalclustering/catchup/storecopy/LocalDatabaseTest.java
@@ -40,7 +40,9 @@
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 
 public class LocalDatabaseTest
 {
@@ -130,16 +132,43 @@ public void availabilityGuardRaisedBeforeDataSourceManagerIsStoppedForStoreCopy(
         inOrder.verify( dataSourceManager ).stop();
     }
 
+    @Test
+    public void doNotRestartServicesIfAlreadyStarted() throws Throwable
+    {
+        DataSourceManager dataSourceManager = mock( DataSourceManager.class );
+        FileSystemWatcherService watcherService = mock( FileSystemWatcherService.class );
+        LocalDatabase localDatabase = newLocalDatabase( newAvailabilityGuard(), dataSourceManager, watcherService );
+
+        localDatabase.start();
+
+        verify( dataSourceManager ).start();
+        verify( watcherService ).start();
+        reset( dataSourceManager );
+        reset( watcherService );
+
+        localDatabase.start();
+        localDatabase.start();
+
+        verify( dataSourceManager, times( 0 ) ).start();
+        verify( watcherService, times( 0 ) ).start();
+    }
+
     private static LocalDatabase newLocalDatabase( AvailabilityGuard availabilityGuard )
     {
         return newLocalDatabase( availabilityGuard, mock( DataSourceManager.class ) );
     }
 
     private static LocalDatabase newLocalDatabase( AvailabilityGuard availabilityGuard,
             DataSourceManager dataSourceManager )
+    {
+        return newLocalDatabase( availabilityGuard, dataSourceManager, FileSystemWatcherService.EMPTY_WATCHER );
+    }
+
+    private static LocalDatabase newLocalDatabase( AvailabilityGuard availabilityGuard,
+            DataSourceManager dataSourceManager, FileSystemWatcherService fileWatcher )
     {
         return new LocalDatabase( new File( "." ), mock( StoreFiles.class ), dataSourceManager,
-                () -> mock( DatabaseHealth.class ), FileSystemWatcherService.EMPTY_WATCHER, availabilityGuard,
+                () -> mock( DatabaseHealth.class ), fileWatcher, availabilityGuard,
                 NullLogProvider.getInstance() );
     }