Force DOCUMENT replication on lock index (#417) (#418)

* Force DOCUMENT replication on lock index Signed-off-by: Daniel Widdis <[email protected]> * Test releasing lock to avoid any query cache issues Signed-off-by: Daniel Widdis <[email protected]> * Restore original test config Signed-off-by: Daniel Widdis <[email protected]> --------- Signed-off-by: Daniel Widdis <[email protected]> (cherry picked from commit 877b425) Co-authored-by: Daniel Widdis <[email protected]>
opensearch-project · Jul 10, 2023 · 9ee9d3e · 9ee9d3e
1 parent 87be65a
commit 9ee9d3e
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 4 deletions.
diff --git a/spi/src/main/java/org/opensearch/jobscheduler/spi/utils/LockService.java b/spi/src/main/java/org/opensearch/jobscheduler/spi/utils/LockService.java
@@ -24,6 +24,7 @@
 import org.opensearch.action.update.UpdateRequest;
 import org.opensearch.client.Client;
 import org.opensearch.cluster.service.ClusterService;
+import org.opensearch.common.settings.Settings;
 import org.opensearch.common.xcontent.LoggingDeprecationHandler;
 import org.opensearch.core.xcontent.NamedXContentRegistry;
 import org.opensearch.core.xcontent.ToXContent;
@@ -42,6 +43,9 @@
 import java.nio.charset.StandardCharsets;
 import java.time.Instant;
 
+import static org.opensearch.indices.replication.common.ReplicationType.DOCUMENT;
+import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE;
+
 public final class LockService {
     private static final Logger logger = LogManager.getLogger(LockService.class);
     private static final String LOCK_INDEX_NAME = ".opendistro-job-scheduler-lock";
@@ -80,7 +84,10 @@ void createLockIndex(ActionListener<Boolean> listener) {
         if (lockIndexExist()) {
             listener.onResponse(true);
         } else {
-            final CreateIndexRequest request = new CreateIndexRequest(LOCK_INDEX_NAME).mapping(lockMapping());
+            // Temporarily force DOCUMENT replication until SEGMENT supports GET by id
+            // https://github.com/opensearch-project/OpenSearch/issues/8536
+            Settings replicationSettings = Settings.builder().put(SETTING_REPLICATION_TYPE, DOCUMENT.name()).build();
+            final CreateIndexRequest request = new CreateIndexRequest(LOCK_INDEX_NAME, replicationSettings).mapping(lockMapping());
             client.admin()
                 .indices()
                 .create(request, ActionListener.wrap(response -> listener.onResponse(response.isAcknowledged()), exception -> {

diff --git a/src/test/java/org/opensearch/jobscheduler/multinode/GetLockMultiNodeRestIT.java b/src/test/java/org/opensearch/jobscheduler/multinode/GetLockMultiNodeRestIT.java
@@ -8,8 +8,6 @@
  */
 package org.opensearch.jobscheduler.multinode;
 
-import com.google.common.collect.ImmutableMap;
-
 import java.io.IOException;
 
 import org.junit.Before;
@@ -23,13 +21,16 @@
 import org.opensearch.jobscheduler.transport.AcquireLockResponse;
 import org.opensearch.test.OpenSearchIntegTestCase;
 
+import com.google.common.collect.ImmutableMap;
+
 @OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.SUITE, numDataNodes = 2)
 public class GetLockMultiNodeRestIT extends ODFERestTestCase {
 
     private String initialJobId;
     private String initialJobIndexName;
     private Response initialGetLockResponse;
 
+    @Override
     @Before
     public void setUp() throws Exception {
         super.setUp();
@@ -53,6 +54,7 @@ public void testGetLockRestAPI() throws Exception {
 
         // Submit 10 requests to generate new lock models for different job indexes
         for (int i = 0; i < 10; i++) {
+            String expectedLockId = TestHelpers.generateExpectedLockId(String.valueOf(i), String.valueOf(i));
             Response getLockResponse = TestHelpers.makeRequest(
                 client(),
                 "GET",
@@ -61,10 +63,20 @@ public void testGetLockRestAPI() throws Exception {
                 TestHelpers.toHttpEntity(TestHelpers.generateAcquireLockRequestBody(String.valueOf(i), String.valueOf(i))),
                 null
             );
+            // Releasing lock will test that it exists (Get by ID)
+            Response releaseLockResponse = TestHelpers.makeRequest(
+                client(),
+                "PUT",
+                TestHelpers.RELEASE_LOCK_BASE_URI + "/" + expectedLockId,
+                ImmutableMap.of(),
+                null,
+                null
+            );
+            assertEquals("success", entityAsMap(releaseLockResponse).get("release-lock"));
 
             String lockId = validateResponseAndGetLockId(getLockResponse);
 
-            assertEquals(TestHelpers.generateExpectedLockId(String.valueOf(i), String.valueOf(i)), lockId);
+            assertEquals(expectedLockId, lockId);
         }
     }