Refactor Remote Store Metadata Lock Manager Utils

Signed-off-by: Harish Bhakuni <[email protected]>
opensearch-project · Oct 5, 2023 · 6fe4ce7 · 6fe4ce7
1 parent 3dd9fc1
commit 6fe4ce7
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 7 deletions.
diff --git a/server/src/main/java/org/opensearch/index/store/lockmanager/FileLockInfo.java b/server/src/main/java/org/opensearch/index/store/lockmanager/FileLockInfo.java
@@ -11,6 +11,7 @@
 import java.nio.file.NoSuchFileException;
 import java.util.Arrays;
 import java.util.List;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 /**
@@ -21,6 +22,7 @@
 public class FileLockInfo implements LockInfo {
     private String fileToLock;
     private String acquirerId;
+    private static final int INVALID_INDEX = -1;
 
     public String getAcquirerId() {
         return acquirerId;
@@ -88,7 +90,11 @@ static String generateLockName(String fileToLock, String acquirerId) {
         }
 
         public static String getFileToLockNameFromLock(String lockName) {
-            String[] lockNameTokens = lockName.split(RemoteStoreLockManagerUtils.SEPARATOR);
+            if (lockName.endsWith(RemoteStoreLockManagerUtils.V1_LOCK_FILE_EXTENSION)) {
+                // this is the old lock file created for versions <= 2.10
+                return getFileToLockNameFromV1Lock(lockName);
+            }
+            String[] lockNameTokens = lockName.split(Pattern.quote(RemoteStoreLockManagerUtils.SEPARATOR));
 
             if (lockNameTokens.length != 2) {
                 throw new IllegalArgumentException("Provided Lock Name " + lockName + " is not Valid.");
@@ -97,13 +103,40 @@ public static String getFileToLockNameFromLock(String lockName) {
         }
 
         public static String getAcquirerIdFromLock(String lockName) {
-            String[] lockNameTokens = lockName.split(RemoteStoreLockManagerUtils.SEPARATOR);
+            if (lockName.endsWith(RemoteStoreLockManagerUtils.V1_LOCK_FILE_EXTENSION)) {
+                // this is the old lock file created for versions <= 2.10
+                return getAcquirerIdFromV1Lock(lockName);
+            }
+            String[] lockNameTokens = lockName.split(Pattern.quote(RemoteStoreLockManagerUtils.SEPARATOR));
 
             if (lockNameTokens.length != 2) {
-                throw new IllegalArgumentException("Provided Lock Name " + lockName + " is not Valid.");
+                throw new IllegalArgumentException(
+                    "Provided Lock Name " + lockName + " is not Valid. got length = " + lockNameTokens.length
+                );
             }
             return lockNameTokens[1].replace(RemoteStoreLockManagerUtils.LOCK_FILE_EXTENSION, "");
         }
+
+        private static String getFileToLockNameFromV1Lock(String lockName) {
+            if (lockName.lastIndexOf(RemoteStoreLockManagerUtils.V1_LOCK_SEPARATOR) == INVALID_INDEX
+                || lockName.lastIndexOf(RemoteStoreLockManagerUtils.V1_LOCK_FILE_EXTENSION) == INVALID_INDEX) {
+                throw new IllegalArgumentException("Provided Lock Name " + lockName + " is not Valid.");
+            }
+            String acquirerId = getAcquirerIdFromV1Lock(lockName);
+            return lockName.substring(0, lockName.lastIndexOf(acquirerId));
+        }
+
+        private static String getAcquirerIdFromV1Lock(String lockName) {
+            if (lockName.lastIndexOf(RemoteStoreLockManagerUtils.V1_LOCK_SEPARATOR) == INVALID_INDEX
+                || lockName.lastIndexOf(RemoteStoreLockManagerUtils.V1_LOCK_FILE_EXTENSION) == INVALID_INDEX) {
+                throw new IllegalArgumentException("Provided Lock Name " + lockName + " is not Valid.");
+            }
+            return lockName.substring(
+                lockName.lastIndexOf(RemoteStoreLockManagerUtils.V1_LOCK_SEPARATOR) + RemoteStoreLockManagerUtils.V1_LOCK_SEPARATOR
+                    .length(),
+                lockName.lastIndexOf(RemoteStoreLockManagerUtils.V1_LOCK_FILE_EXTENSION)
+            );
+        }
     }
 
     /**

diff --git a/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreLockManagerUtils.java b/server/src/main/java/org/opensearch/index/store/lockmanager/RemoteStoreLockManagerUtils.java
@@ -15,8 +15,11 @@
  */
 public class RemoteStoreLockManagerUtils {
     static final String FILE_TO_LOCK_NAME = "file_to_lock";
-    static final String SEPARATOR = "___";
-    static final String LOCK_FILE_EXTENSION = ".lock";
+    static final String V1_LOCK_SEPARATOR = "___";
+    static final String SEPARATOR = "...";
+    // for versions <= 2.10, we have lock files with this extension.
+    static final String V1_LOCK_FILE_EXTENSION = ".lock";
+    static final String LOCK_FILE_EXTENSION = ".v2_lock";
     static final String ACQUIRER_ID = "acquirer_id";
     public static final String NO_TTL = "-1";
     static final String LOCK_EXPIRY_TIME = "lock_expiry_time";

diff --git a/server/src/test/java/org/opensearch/index/store/lockmanager/FileLockInfoTests.java b/server/src/test/java/org/opensearch/index/store/lockmanager/FileLockInfoTests.java
@@ -15,10 +15,24 @@
 public class FileLockInfoTests extends OpenSearchTestCase {
     String testMetadata = "testMetadata";
     String testAcquirerId = "testAcquirerId";
+    String testAcquirerId2 = "ZxZ4Wh89SXyEPmSYAHrIrQ";
+    String testAcquirerId3 = "ZxZ4Wh89SXyEPmSYAHrItS";
+    String testMetadata1 = "metadata__9223372036854775806__9223372036854775803__9223372036854775790"
+        + "__9223372036854775800___Hf3Dbw2QQagfGLlVBOUrg__9223370340398865071__1";
+
+    String oldLock = testMetadata1 + RemoteStoreLockManagerUtils.V1_LOCK_SEPARATOR + testAcquirerId2
+        + RemoteStoreLockManagerUtils.V1_LOCK_FILE_EXTENSION;
+    String newLock = testMetadata1 + RemoteStoreLockManagerUtils.SEPARATOR + testAcquirerId3
+        + RemoteStoreLockManagerUtils.LOCK_FILE_EXTENSION;
 
     public void testGenerateLockName() {
         FileLockInfo fileLockInfo = FileLockInfo.getLockInfoBuilder().withFileToLock(testMetadata).withAcquirerId(testAcquirerId).build();
         assertEquals(fileLockInfo.generateLockName(), FileLockInfo.LockFileUtils.generateLockName(testMetadata, testAcquirerId));
+
+        // validate that lock generated will be the new version lock
+        fileLockInfo = FileLockInfo.getLockInfoBuilder().withFileToLock(testMetadata1).withAcquirerId(testAcquirerId3).build();
+        assertEquals(fileLockInfo.generateLockName(), newLock);
+
     }
 
     public void testGenerateLockNameFailureCase1() {
@@ -42,12 +56,22 @@ public void testGetLockPrefixFailureCase() {
     }
 
     public void testGetLocksForAcquirer() throws NoSuchFileException {
+
         String[] locks = new String[] {
             FileLockInfo.LockFileUtils.generateLockName(testMetadata, testAcquirerId),
-            FileLockInfo.LockFileUtils.generateLockName(testMetadata, "acquirerId2") };
+            FileLockInfo.LockFileUtils.generateLockName(testMetadata, "acquirerId2"),
+            oldLock,
+            newLock };
         FileLockInfo fileLockInfo = FileLockInfo.getLockInfoBuilder().withAcquirerId(testAcquirerId).build();
-
         assertEquals(fileLockInfo.getLockForAcquirer(locks), FileLockInfo.LockFileUtils.generateLockName(testMetadata, testAcquirerId));
+
+        // validate old lock
+        fileLockInfo = FileLockInfo.getLockInfoBuilder().withAcquirerId(testAcquirerId2).build();
+        assertEquals(fileLockInfo.getLockForAcquirer(locks), oldLock);
+
+        // validate new lock
+        fileLockInfo = FileLockInfo.getLockInfoBuilder().withAcquirerId(testAcquirerId3).build();
+        assertEquals(fileLockInfo.getLockForAcquirer(locks), newLock);
     }
 
 }