Enhancement implemented #2954 : Recover from fatal invalid cache state

ehcache-core/src/main/java/org/ehcache/core/spi/service/LocalPersistenceService.java

@@ -77,14 +77,14 @@ interface SafeSpaceIdentifier {
   }
 
   /**
-   * Identify state(normal/abnormal) of stopped service.
-   * @return <tt>true</tt> if service stopped normally.
+   * Return the cleanliness of the state stored in this service.
+   * <p>
+   * Stored state is assumed to be clean if the service detects
+   * that the last started instantiation of this service was shutdown
+   * successfully.
+   *
+   * @return {@code true} if the state is clean
+   * @throws IllegalStateException if the service is not started
    */
   boolean isClean();
-
-  /**
-   * Identify status of service.
-   * @return <tt>true</tt> if service is started.
-   */
-  boolean isServiceStarted();
 }

ehcache-impl/src/main/java/org/ehcache/impl/persistence/DefaultDiskResourceService.java

@@ -60,6 +60,10 @@ private boolean isStarted() {
   @Override
   public void start(final ServiceProvider<Service> serviceProvider) {
     innerStart(serviceProvider);
+    if (persistenceService != null && !persistenceService.isClean()) {
+      destroyAll();
+      LOGGER.info("Probably unclean shutdown was done, so deleted root directory.");
+    }
   }
 
   /**
@@ -73,10 +77,6 @@ public void startForMaintenance(ServiceProvider<? super MaintainableService> ser
   private void innerStart(ServiceProvider<? super MaintainableService> serviceProvider) {
     persistenceService = serviceProvider.getService(LocalPersistenceService.class);
     isStarted = true;
-    if (persistenceService!=null && persistenceService.isServiceStarted() && !persistenceService.isClean()) {
-      destroyAll();
-      LOGGER.info("Probably unclean shutdown was done, so deleted root directory.");
-    }
   }
 
   /**

ehcache-impl/src/main/java/org/ehcache/impl/persistence/DefaultLocalPersistenceService.java

@@ -144,12 +144,14 @@ public synchronized void stop() {
         if (cleanFile.createNewFile()) {
           LOGGER.debug("clean file is created.");
         } else {
-          LOGGER.warn("clean file already exists. It's not deleted either user's permission or network issue." +
-            "\n Hint: clean file exists on service startup, indicates service was stopped cleanly last time. It gets created while the service is stopped and it should be deleted while the service is started.");
+          LOGGER.warn("clean file already exists. The file didn't got deleted, may be due to network issue or file permission on directory." +
+            "\n Hint: clean file exists on service start-up, indicates service was stopped cleanly last time. It gets created while the service is stopped and it should be deleted while the service is started." +
+            "\n Action: Please verify there permission to delete the file and delete the root directory prior to start the service again.");
         }
       } catch (IOException e) {
         LOGGER.warn("clean file is not created. Reason: " + e.getMessage() +
-          "\n Hint: clean file exists on service startup, indicates service was stopped cleanly last time. It gets created while the service is stopped and it should be deleted while the service is started.");
+          "\n Hint: clean file exists on service start-up, indicates service was stopped cleanly last time. It gets created while the service is stopped and it should be deleted while the service is started." +
+          "\n Action: Do resolve the exception received. Prior to start the service again, please delete the root directory.");
       }
       try {
         lock.release();
@@ -237,10 +239,6 @@ public void destroyAll(String owner) {
 
   /**
    * {@inheritDoc}
-   * Abnormally stopped service may lead to data corruption.
-   * Can take appropriate action by identifying state of service stopped.
-   *
-   * @throws IllegalStateException if service is not running.
    */
   @Override
   public final synchronized boolean isClean() {
@@ -251,14 +249,6 @@ public final synchronized boolean isClean() {
     }
   }
 
-  /**
-   * {@inheritDoc}
-   */
-  @Override
-  public final boolean isServiceStarted() {
-    return started;
-  }
-
   private void destroy(SafeSpace ss, boolean verbose) {
     if (verbose) {
       LOGGER.debug("Destroying file based persistence context for {}", ss.identifier);
@@ -270,7 +260,6 @@ private void destroy(SafeSpace ss, boolean verbose) {
     }
   }
 
-
   private SafeSpace createSafeSpaceLogical(String owner, String identifier) {
     File ownerDirectory = new File(rootDirectory, owner);
     File directory = new File(ownerDirectory, safeIdentifier(identifier));

ehcache-impl/src/test/java/org/ehcache/impl/persistence/DefaultDiskResourceServiceTest.java

@@ -30,6 +30,7 @@
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.ehcache.test.MockitoUtil.uncheckedGenericMock;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -74,7 +75,7 @@ public void testHandlesResourceType() {
     @Test
     public void testDestroyAll() {
       service.destroyAll();
-      verify(persistenceService).destroyAll(DefaultDiskResourceService.PERSISTENCE_SPACE_OWNER);
+      verify(persistenceService, times(2)).destroyAll(DefaultDiskResourceService.PERSISTENCE_SPACE_OWNER);
     }
 
     @Test

ehcache-impl/src/test/java/org/ehcache/impl/persistence/DefaultLocalPersistenceServiceTest.java

@@ -144,32 +144,44 @@ public void testExclusiveLock() throws IOException {
   }
 
   @Test
-  public void testServiceStoppedStatusWhenStoppedCleanly() throws IOException {
-    File f = folder.newFolder("testServiceStoppedStatusWhenStoppedCleanly");
+  public void testServiceShutdownWithEmptyDirectory() throws IOException {
+    File f = folder.newFolder("testServiceShutdownWithEmptyDirectory");
     final DefaultLocalPersistenceService service = new DefaultLocalPersistenceService(new DefaultPersistenceConfiguration(f));
     service.start(null);
+    assertTrue(service.isClean());
     service.stop();
     service.start(null);
     assertTrue(service.isClean());
     service.stop();
   }
 
   @Test
-  public void testServiceStoppedStatusWhenStoppedUnexpectedly() throws IOException {
-    // Service stopped unexpectedly means directory exists with some data but without .clean file.
-    File f = folder.newFolder("testServiceStoppedStatusWhenStoppedUnexpectedly");
+  public void testServiceShutdownWithNonEmptyDirectory() throws IOException {
+    File f = folder.newFolder("testServiceShutdownWithNonEmptyDirectory");
+    final DefaultLocalPersistenceService service = new DefaultLocalPersistenceService(new DefaultPersistenceConfiguration(f));
+    new File(f, "dummy.txt").createNewFile();
+    new File(f, ".clean").createNewFile();
+    service.start(null);
+    assertTrue(service.isClean());
+    service.stop();
+  }
+
+  @Test
+  public void testServiceShutdownUnexpectedly() throws IOException {
+    // Service shutdown unexpectedly means directory exists with some data but without .clean file.
+    File f = folder.newFolder("testServiceShutdownUnexpectedly");
     final DefaultLocalPersistenceService service = new DefaultLocalPersistenceService(new DefaultPersistenceConfiguration(f));
-    new File(f.getAbsolutePath()+"\\dummy.txt").createNewFile();
+    new File(f, "dummy.txt").createNewFile();
     service.start(null);
     assertFalse(service.isClean());
     service.stop();
   }
 
   @Test
-  public void testServiceStoppedStatusIfServiceIsNotRunning() throws IOException {
+  public void testServiceShutdownStatusIfServiceIsNotRunning() throws IOException {
+    File f = folder.newFolder("testServiceShutdownStatusIfServiceIsNotRunning");
+    final DefaultLocalPersistenceService service = new DefaultLocalPersistenceService(new DefaultPersistenceConfiguration(f));
     try {
-      File f = folder.newFolder("testServiceStoppedStatusIfServiceIsNotRunning");
-      final DefaultLocalPersistenceService service = new DefaultLocalPersistenceService(new DefaultPersistenceConfiguration(f));
       service.isClean();
       fail("Expected IllegalStateException");
     } catch(IllegalStateException e) {