From 5d3bfe54a5b630fd6c9de43f47c26675260035bc Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Sun, 9 Jul 2023 19:46:03 -0700 Subject: [PATCH 1/3] Force DOCUMENT replication on lock index Signed-off-by: Daniel Widdis --- .../jobscheduler/spi/utils/LockService.java | 9 ++++++++- .../multinode/GetLockMultiNodeRestIT.java | 16 +++++++++++++--- 2 files changed, 21 insertions(+), 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 index 96a0b87b..d133d7b1 100644 --- 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 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 index 986dfead..d5c3bbc3 100644 --- a/src/test/java/org/opensearch/jobscheduler/multinode/GetLockMultiNodeRestIT.java +++ b/src/test/java/org/opensearch/jobscheduler/multinode/GetLockMultiNodeRestIT.java @@ -8,12 +8,12 @@ */ package org.opensearch.jobscheduler.multinode; -import com.google.common.collect.ImmutableMap; - import java.io.IOException; +import org.apache.hc.core5.http.HttpEntity; import org.junit.Before; import org.opensearch.client.Response; +import org.opensearch.client.ResponseException; import org.opensearch.common.xcontent.LoggingDeprecationHandler; import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.xcontent.NamedXContentRegistry; @@ -23,6 +23,8 @@ 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 { @@ -30,6 +32,7 @@ public class GetLockMultiNodeRestIT extends ODFERestTestCase { private String initialJobIndexName; private Response initialGetLockResponse; + @Override @Before public void setUp() throws Exception { super.setUp(); @@ -53,14 +56,21 @@ public void testGetLockRestAPI() throws Exception { // Submit 10 requests to generate new lock models for different job indexes for (int i = 0; i < 10; i++) { + final HttpEntity httpEntity = TestHelpers.toHttpEntity( + TestHelpers.generateAcquireLockRequestBody(String.valueOf(i), String.valueOf(i)) + ); Response getLockResponse = TestHelpers.makeRequest( client(), "GET", TestHelpers.GET_LOCK_BASE_URI, ImmutableMap.of(), - TestHelpers.toHttpEntity(TestHelpers.generateAcquireLockRequestBody(String.valueOf(i), String.valueOf(i))), + httpEntity, null ); + // attempt to acquire same lock should fail + assertThrows(ResponseException.class, () -> { + TestHelpers.makeRequest(client(), "GET", TestHelpers.GET_LOCK_BASE_URI, ImmutableMap.of(), httpEntity, null); + }); String lockId = validateResponseAndGetLockId(getLockResponse); From d802e1ccc0ac53f4c5903cf0cabb871a4cc357aa Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Sun, 9 Jul 2023 22:11:43 -0700 Subject: [PATCH 2/3] Test releasing lock to avoid any query cache issues Signed-off-by: Daniel Widdis --- .../multinode/GetLockMultiNodeRestIT.java | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/test/java/org/opensearch/jobscheduler/multinode/GetLockMultiNodeRestIT.java b/src/test/java/org/opensearch/jobscheduler/multinode/GetLockMultiNodeRestIT.java index d5c3bbc3..bdb79521 100644 --- a/src/test/java/org/opensearch/jobscheduler/multinode/GetLockMultiNodeRestIT.java +++ b/src/test/java/org/opensearch/jobscheduler/multinode/GetLockMultiNodeRestIT.java @@ -10,10 +10,8 @@ import java.io.IOException; -import org.apache.hc.core5.http.HttpEntity; import org.junit.Before; import org.opensearch.client.Response; -import org.opensearch.client.ResponseException; import org.opensearch.common.xcontent.LoggingDeprecationHandler; import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.xcontent.NamedXContentRegistry; @@ -25,7 +23,7 @@ import com.google.common.collect.ImmutableMap; -@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.SUITE, numDataNodes = 2) +@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 21) public class GetLockMultiNodeRestIT extends ODFERestTestCase { private String initialJobId; @@ -55,26 +53,30 @@ public void testGetLockRestAPI() throws Exception { assertEquals(TestHelpers.generateExpectedLockId(initialJobIndexName, initialJobId), initialLockId); // Submit 10 requests to generate new lock models for different job indexes - for (int i = 0; i < 10; i++) { - final HttpEntity httpEntity = TestHelpers.toHttpEntity( - TestHelpers.generateAcquireLockRequestBody(String.valueOf(i), String.valueOf(i)) - ); + for (int i = 0; i < 10000; i++) { + String expectedLockId = TestHelpers.generateExpectedLockId(String.valueOf(i), String.valueOf(i)); Response getLockResponse = TestHelpers.makeRequest( client(), "GET", TestHelpers.GET_LOCK_BASE_URI, ImmutableMap.of(), - httpEntity, + 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 ); - // attempt to acquire same lock should fail - assertThrows(ResponseException.class, () -> { - TestHelpers.makeRequest(client(), "GET", TestHelpers.GET_LOCK_BASE_URI, ImmutableMap.of(), httpEntity, 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); } } From ab9e5bf31fd619f3dcac49b8152712663148ccda Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Sun, 9 Jul 2023 22:13:56 -0700 Subject: [PATCH 3/3] Restore original test config Signed-off-by: Daniel Widdis --- .../jobscheduler/multinode/GetLockMultiNodeRestIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/opensearch/jobscheduler/multinode/GetLockMultiNodeRestIT.java b/src/test/java/org/opensearch/jobscheduler/multinode/GetLockMultiNodeRestIT.java index bdb79521..65edd5e3 100644 --- a/src/test/java/org/opensearch/jobscheduler/multinode/GetLockMultiNodeRestIT.java +++ b/src/test/java/org/opensearch/jobscheduler/multinode/GetLockMultiNodeRestIT.java @@ -23,7 +23,7 @@ import com.google.common.collect.ImmutableMap; -@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST, numDataNodes = 21) +@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.SUITE, numDataNodes = 2) public class GetLockMultiNodeRestIT extends ODFERestTestCase { private String initialJobId; @@ -53,7 +53,7 @@ public void testGetLockRestAPI() throws Exception { assertEquals(TestHelpers.generateExpectedLockId(initialJobIndexName, initialJobId), initialLockId); // Submit 10 requests to generate new lock models for different job indexes - for (int i = 0; i < 10000; i++) { + for (int i = 0; i < 10; i++) { String expectedLockId = TestHelpers.generateExpectedLockId(String.valueOf(i), String.valueOf(i)); Response getLockResponse = TestHelpers.makeRequest( client(),