ISPN-8670 Lock the persistent global state location and add a shared …

…persistent location

client/hotrod-client/src/test/java/org/infinispan/client/hotrod/admin/RemoteCacheAdminPermanentTest.java

@@ -15,6 +15,7 @@
 import org.infinispan.configuration.cache.ConfigurationBuilder;
 import org.infinispan.configuration.global.GlobalConfigurationBuilder;
 import org.infinispan.configuration.internal.PrivateGlobalConfigurationBuilder;
+import org.infinispan.globalstate.ConfigurationStorage;
 import org.infinispan.manager.EmbeddedCacheManager;
 import org.infinispan.server.core.admin.embeddedserver.EmbeddedServerAdminOperationHandler;
 import org.infinispan.server.hotrod.HotRodServer;
@@ -40,14 +41,24 @@ protected HotRodServer addHotRodServer(ConfigurationBuilder builder) {
       return addStatefulHotRodServer(builder, serverId++);
    }
 
+   protected boolean isShared() {
+      return false;
+   }
+
    protected HotRodServer addStatefulHotRodServer(ConfigurationBuilder builder, char id) {
       GlobalConfigurationBuilder gcb = GlobalConfigurationBuilder.defaultClusteredBuilder();
       gcb.addModule(PrivateGlobalConfigurationBuilder.class).serverMode(true);
       String stateDirectory = TestingUtil.tmpDirectory(this.getClass().getSimpleName() + File.separator + id);
       if (clear)
          Util.recursiveFileRemove(stateDirectory);
-      gcb.globalState().enable().persistentLocation(stateDirectory);
-
+      gcb.globalState().enable().persistentLocation(stateDirectory).
+         configurationStorage(ConfigurationStorage.OVERLAY);
+      if (isShared()) {
+         String sharedDirectory = TestingUtil.tmpDirectory(this.getClass().getSimpleName() + File.separator + "COMMON");
+         gcb.globalState().sharedPersistentLocation(sharedDirectory);
+      } else {
+         gcb.globalState().sharedPersistentLocation(stateDirectory);
+      }
       EmbeddedCacheManager cm = addClusterEnabledCacheManager(gcb, builder);
       cm.defineConfiguration("template", builder.build());
       HotRodServerConfigurationBuilder serverBuilder = new HotRodServerConfigurationBuilder();

client/hotrod-client/src/test/java/org/infinispan/client/hotrod/counter/RemoteCounterManagerTest.java

@@ -82,8 +82,10 @@ public void testGetCounterNames(Method method) {
 
    @Override
    protected void modifyGlobalConfiguration(GlobalConfigurationBuilder builder) {
+      char id = 'A';
+      id += cacheManagers.size();
       builder.globalState().enable()
-            .persistentLocation(PERSISTENT_LOCATION)
+            .persistentLocation(PERSISTENT_LOCATION + File.separator + id)
             .temporaryLocation(TMP_LOCATION);
    }
 

commons/src/main/java/org/infinispan/commons/util/Util.java

@@ -1,7 +1,6 @@
 package org.infinispan.commons.util;
 
 import java.io.ByteArrayOutputStream;
-import java.io.Closeable;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
@@ -434,7 +433,7 @@ public static String read(InputStream is) throws IOException {
        }
     }
 
-   public static void close(Closeable cl) {
+   public static void close(AutoCloseable cl) {
       if (cl == null) return;
       try {
          cl.close();
@@ -450,8 +449,8 @@ public static void close(Socket s) {
       }
    }
 
-   public static void close(Closeable... cls) {
-      for (Closeable cl : cls) {
+   public static void close(AutoCloseable... cls) {
+      for (AutoCloseable cl : cls) {
          close(cl);
       }
    }

core/src/main/java/org/infinispan/configuration/global/GlobalStateConfiguration.java

@@ -21,6 +21,9 @@ public class GlobalStateConfiguration {
    public static final AttributeDefinition<String> PERSISTENT_LOCATION = AttributeDefinition
          .builder("persistentLocation", null, String.class)
             .initializer(() -> SecurityActions.getSystemProperty("user.dir")).immutable().build();
+   public static final AttributeDefinition<String> SHARED_PERSISTENT_LOCATION = AttributeDefinition
+         .builder("sharedPersistentLocation", null, String.class)
+         .initializer(() -> SecurityActions.getSystemProperty("user.dir")).immutable().build();
    public static final AttributeDefinition<String> TEMPORARY_LOCATION = AttributeDefinition
          .builder("temporaryLocation", null, String.class)
             .initializer(() -> SecurityActions.getSystemProperty("java.io.tmpdir")).immutable().build();
@@ -32,12 +35,13 @@ public class GlobalStateConfiguration {
          .immutable().build();
 
    public static AttributeSet attributeDefinitionSet() {
-      return new AttributeSet(GlobalStateConfiguration.class, ENABLED, PERSISTENT_LOCATION, TEMPORARY_LOCATION, CONFIGURATION_STORAGE, CONFIGURATION_STORAGE_SUPPLIER);
+      return new AttributeSet(GlobalStateConfiguration.class, ENABLED, PERSISTENT_LOCATION, SHARED_PERSISTENT_LOCATION, TEMPORARY_LOCATION, CONFIGURATION_STORAGE, CONFIGURATION_STORAGE_SUPPLIER);
    }
 
    private final AttributeSet attributes;
    private final Attribute<Boolean> enabled;
    private final Attribute<String> persistentLocation;
+   private Attribute<String> sharedPersistentLocation;
    private final Attribute<String> temporaryLocation;
    private final Attribute<ConfigurationStorage> configurationStorage;
    private final Attribute<Supplier<? extends LocalConfigurationStorage>> configurationStorageSupplier;
@@ -46,6 +50,7 @@ public GlobalStateConfiguration(AttributeSet attributes) {
       this.attributes = attributes.checkProtection();
       this.enabled = attributes.attribute(ENABLED);
       this.persistentLocation = attributes.attribute(PERSISTENT_LOCATION);
+      this.sharedPersistentLocation = attributes.attribute(SHARED_PERSISTENT_LOCATION);
       this.temporaryLocation = attributes.attribute(TEMPORARY_LOCATION);
       this.configurationStorage = attributes.attribute(CONFIGURATION_STORAGE);
       this.configurationStorageSupplier = attributes.attribute(CONFIGURATION_STORAGE_SUPPLIER);
@@ -58,12 +63,21 @@ public boolean enabled() {
    /**
     * Returns the filesystem path where persistent state data which needs to survive container
     * restarts should be stored. Defaults to the user.dir system property which usually is where the
-    * application was started.
+    * application was started. Warning: this path must NOT be shared with other instances.
     */
    public String persistentLocation() {
       return persistentLocation.get();
    }
 
+   /**
+    * Returns the filesystem path where shared persistent state data which needs to survive container
+    * restarts should be stored. Defaults to the user.dir system property which usually is where the
+    * application was started. This path may be shared among multiple instances.
+    */
+   public String sharedPersistentLocation() {
+      return sharedPersistentLocation.get();
+   }
+
    /**
     * Returns the filesystem path where temporary state should be stored. Defaults to the value of
     * the java.io.tmpdir system property.
@@ -91,6 +105,4 @@ public AttributeSet attributes() {
    public String toString() {
       return "GlobalStateConfiguration [attributes=" + attributes + "]";
    }
-
-
 }

core/src/main/java/org/infinispan/configuration/global/GlobalStateConfigurationBuilder.java

@@ -4,6 +4,7 @@
 import static org.infinispan.configuration.global.GlobalStateConfiguration.CONFIGURATION_STORAGE_SUPPLIER;
 import static org.infinispan.configuration.global.GlobalStateConfiguration.ENABLED;
 import static org.infinispan.configuration.global.GlobalStateConfiguration.PERSISTENT_LOCATION;
+import static org.infinispan.configuration.global.GlobalStateConfiguration.SHARED_PERSISTENT_LOCATION;
 import static org.infinispan.configuration.global.GlobalStateConfiguration.TEMPORARY_LOCATION;
 
 import java.lang.invoke.MethodHandles;
@@ -55,14 +56,26 @@ public boolean enabled() {
    /**
     * Defines the filesystem path where persistent state data which needs to survive container restarts
     * should be stored. The data stored at this location is required for graceful
-      shutdown and restore. Defaults to the user.dir system property which usually is where the
+    * shutdown and restore. This path must NOT be shared among multiple instances.
+    * Defaults to the user.dir system property which usually is where the
     * application was started. This value should be overridden to a more appropriate location.
     */
    public GlobalStateConfigurationBuilder persistentLocation(String location) {
       attributes.attribute(PERSISTENT_LOCATION).set(location);
       return this;
    }
 
+   /**
+    * Defines the filesystem path where shared persistent state data which needs to survive container restarts
+    * should be stored. This path can be safely shared among multiple instances.
+    * Defaults to the user.dir system property which usually is where the
+    * application was started. This value should be overridden to a more appropriate location.
+    */
+   public GlobalStateConfigurationBuilder sharedPersistentLocation(String location) {
+      attributes.attribute(SHARED_PERSISTENT_LOCATION).set(location);
+      return this;
+   }
+
    /**
     * Defines the filesystem path where temporary state should be stored. Defaults to the value of the
     * java.io.tmpdir system property.

core/src/main/java/org/infinispan/configuration/parsing/Element.java

@@ -80,6 +80,7 @@ public enum Element {
     ROOT("infinispan"),
     SCATTERED_CACHE("scattered-cache"),
     SCATTERED_CACHE_CONFIGURATION("scattered-cache-configuration"),
+    SHARED_PERSISTENT_LOCATION("shared-persistent-location"),
     SCHEDULED_THREAD_POOL("scheduled-thread-pool"),
     SECURITY("security"),
     SERIALIZATION("serialization"),
@@ -124,7 +125,7 @@ public String getLocalName() {
     private static final Map<String, Element> MAP;
 
     static {
-        final Map<String, Element> map = new HashMap<String, Element>(8);
+        final Map<String, Element> map = new HashMap<>(8);
         for (Element element : values()) {
             final String name = element.getLocalName();
             if (name != null) map.put(name, element);

core/src/main/java/org/infinispan/configuration/parsing/Parser.java

@@ -1138,6 +1138,10 @@ private void parseGlobalState(XMLExtendedStreamReader reader, ConfigurationBuild
                builder.persistentLocation(parseGlobalStatePath(reader));
                break;
             }
+            case SHARED_PERSISTENT_LOCATION: {
+               builder.sharedPersistentLocation(parseGlobalStatePath(reader));
+               break;
+            }
             case TEMPORARY_LOCATION: {
                builder.temporaryLocation(parseGlobalStatePath(reader));
                break;

core/src/main/java/org/infinispan/globalstate/impl/GlobalStateManagerImpl.java

@@ -4,10 +4,13 @@
 
 import java.io.BufferedReader;
 import java.io.File;
+import java.io.FileOutputStream;
 import java.io.FileReader;
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.lang.invoke.MethodHandles;
+import java.nio.channels.FileLock;
+import java.nio.channels.OverlappingFileLockException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
@@ -48,11 +51,14 @@ public class GlobalStateManagerImpl implements GlobalStateManager {
 
    private List<GlobalStateProvider> stateProviders = new ArrayList<>();
    private boolean persistentState;
+   FileOutputStream globalLockFile;
+   private FileLock globalLock;
 
    @Start(priority = 1) // Must start before everything else
    public void start() {
       persistentState = globalConfiguration.globalState().enabled();
       if (persistentState) {
+         acquireGlobalLock();
          loadGlobalState();
       }
    }
@@ -61,9 +67,32 @@ public void start() {
    public void stop() {
       if (persistentState) {
          writeGlobalState();
+         releaseGlobalLock();
       }
    }
 
+   private void acquireGlobalLock() {
+      File lockFile = getLockFile();
+      try {
+         lockFile.getParentFile().mkdirs();
+         globalLockFile = new FileOutputStream(lockFile);
+         globalLock = globalLockFile.getChannel().tryLock();
+         if (globalLock == null) {
+            throw log.globalStateCannotAcquireLockFile(null, lockFile);
+         }
+      } catch (IOException | OverlappingFileLockException e) {
+         throw log.globalStateCannotAcquireLockFile(e, lockFile);
+      }
+   }
+
+   private void releaseGlobalLock() {
+      if (globalLock != null && globalLock.isValid())
+         Util.close(globalLock);
+      globalLock = null;
+      Util.close(globalLockFile);
+      getLockFile().delete();
+   }
+
    private void loadGlobalState() {
       File stateFile = getStateFile(GLOBAL_SCOPE);
       Optional<ScopedPersistentState> globalState = readScopedState(GLOBAL_SCOPE);
@@ -78,13 +107,8 @@ private void loadGlobalState() {
 
          stateProviders.forEach(provider -> provider.prepareForRestore(state));
       } else {
-         // Clean slate. Try to create an empty state file before proceeding
-         try {
-            stateFile.getParentFile().mkdirs();
-            stateFile.createNewFile();
-         } catch (IOException e) {
-            throw log.nonWritableStateFile(stateFile);
-         }
+         // Clean slate. Create the persistent location if necessary and acquire a lock
+         stateFile.getParentFile().mkdirs();
       }
    }
 
@@ -147,6 +171,10 @@ private File getStateFile(String scope) {
       return new File(globalConfiguration.globalState().persistentLocation(), scope + ".state");
    }
 
+   private File getLockFile() {
+      return new File(globalConfiguration.globalState().persistentLocation(), GLOBAL_SCOPE + ".lck");
+   }
+
    @Override
    public void registerStateProvider(GlobalStateProvider provider) {
       this.stateProviders.add(provider);

core/src/main/java/org/infinispan/globalstate/impl/OverlayLocalConfigurationStorage.java

@@ -77,7 +77,9 @@ public Map<String, Configuration> loadAll() {
 
    private void storeAll() {
       try {
-         File temp = File.createTempFile("caches", null, new File(globalConfiguration.globalState().persistentLocation()));
+         File sharedDirectory = new File(globalConfiguration.globalState().sharedPersistentLocation());
+         sharedDirectory.mkdirs();
+         File temp = File.createTempFile("caches", null, sharedDirectory);
          Map<String, Configuration> configurationMap = new HashMap<>();
          for (String cacheName : persistentCaches) {
             configurationMap.put(cacheName, cacheManager.getCacheConfiguration(cacheName));
@@ -103,10 +105,10 @@ private void storeAll() {
    }
 
    private File getPersistentFile() {
-      return new File(globalConfiguration.globalState().persistentLocation(), "caches.xml");
+      return new File(globalConfiguration.globalState().sharedPersistentLocation(), "caches.xml");
    }
 
    private File getPersistentFileLock() {
-      return new File(globalConfiguration.globalState().persistentLocation(), "caches.xml.lck");
+      return new File(globalConfiguration.globalState().sharedPersistentLocation(), "caches.xml.lck");
    }
 }

core/src/main/java/org/infinispan/util/logging/Log.java

@@ -1745,4 +1745,7 @@ CacheConfigurationException offHeapMemoryEvictionSizeNotLargeEnoughForAddresses(
 
    @Message(value = "ConfigurationStrategy cannot be set to MANAGED in embedded mode", id = 511)
    CacheConfigurationException managerConfigurationStorageUnavailable();
+
+   @Message(value = "Cannot acquire lock '%s' for persistent global state", id = 512)
+   CacheConfigurationException globalStateCannotAcquireLockFile(@Cause Throwable cause, File lockFile);
 }

core/src/main/resources/schema/infinispan-config-9.2.xsd

@@ -552,17 +552,28 @@
 
   <xs:complexType name="global-state">
     <xs:sequence>
-      <xs:element name="persistent-location" type="tns:global-state-path" minOccurs="1" maxOccurs="1">
+      <xs:element name="persistent-location" type="tns:global-state-path" minOccurs="0">
         <xs:annotation>
           <xs:documentation>
             Defines the filesystem path where persistent state data which needs to survive container restarts
             should be stored. The data stored at this location is required for graceful
-            shutdown and restore. Defaults to the user.dir system property which usually is where the
+            shutdown and restore. This path must NOT be shared among multiple instances.
+            Defaults to the user.dir system property which usually is where the
             application was started. This value should be overridden to a more appropriate location.
           </xs:documentation>
         </xs:annotation>
       </xs:element>
-      <xs:element name="temporary-location" type="tns:global-state-path" minOccurs="0" maxOccurs="1">
+      <xs:element name="shared-persistent-location" type="tns:global-state-path" minOccurs="0">
+        <xs:annotation>
+          <xs:documentation>
+            Defines the filesystem path where shared persistent state data which needs to survive container restarts
+            should be stored. This path can be safely shared among multiple instances.
+            Defaults to the user.dir system property which usually is where the
+            application was started. This value should be overridden to a more appropriate location.
+          </xs:documentation>
+        </xs:annotation>
+      </xs:element>
+      <xs:element name="temporary-location" type="tns:global-state-path" minOccurs="0">
         <xs:annotation>
           <xs:documentation>
             Defines the filesystem path where temporary state should be stored. Defaults to the value of the

core/src/test/java/org/infinispan/globalstate/GlobalStateTest.java

@@ -0,0 +1,37 @@
+package org.infinispan.globalstate;
+
+import java.io.File;
+
+import org.infinispan.commons.util.Util;
+import org.infinispan.configuration.cache.ConfigurationBuilder;
+import org.infinispan.configuration.global.GlobalConfigurationBuilder;
+import org.infinispan.manager.EmbeddedCacheManager;
+import org.infinispan.manager.EmbeddedCacheManagerStartupException;
+import org.infinispan.test.AbstractInfinispanTest;
+import org.infinispan.test.Exceptions;
+import org.infinispan.test.TestingUtil;
+import org.infinispan.test.fwk.TestCacheManagerFactory;
+import org.infinispan.test.fwk.TransportFlags;
+import org.testng.annotations.Test;
+
+/**
+ * @since 9.2
+ */
+@Test(testName = "globalstate.GlobalStateTest", groups = "functional")
+public class GlobalStateTest extends AbstractInfinispanTest {
+
+   public void testLockPersistentLocation() {
+      String stateDirectory = TestingUtil.tmpDirectory(this.getClass().getSimpleName() + File.separator + "COMMON");
+      Util.recursiveFileRemove(stateDirectory);
+      GlobalConfigurationBuilder global = GlobalConfigurationBuilder.defaultClusteredBuilder();
+      global.globalState().enable().persistentLocation(stateDirectory);
+      EmbeddedCacheManager cm1 = TestCacheManagerFactory.createClusteredCacheManager(false, global, new ConfigurationBuilder(), new TransportFlags(), false);
+      EmbeddedCacheManager cm2 = TestCacheManagerFactory.createClusteredCacheManager(false, global, new ConfigurationBuilder(), new TransportFlags(), false);
+      try {
+         cm1.start();
+         Exceptions.expectException(EmbeddedCacheManagerStartupException.class, "org.infinispan.commons.CacheConfigurationException: ISPN000512: Cannot acquire lock.*", () -> cm2.start());
+      } finally {
+         TestingUtil.killCacheManagers(cm1, cm2);
+      }
+   }
+}