Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Create QueryGroup API Logic #14680

Merged
merged 20 commits into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
add coverage
Signed-off-by: Ruirui Zhang <mariazrr@amazon.com>
  • Loading branch information
ruai0511 committed Aug 12, 2024
commit 8e123f7b504843bc98a4038de497c45c92631fde
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@
ThreadPool threadPool,
QueryGroupPersistenceService queryGroupPersistenceService
) {
super(CreateQueryGroupAction.NAME, transportService, actionFilters, CreateQueryGroupRequest::new);
this.threadPool = threadPool;
this.queryGroupPersistenceService = queryGroupPersistenceService;
}

Check warning on line 50 in plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportCreateQueryGroupAction.java

View check run for this annotation

Codecov / codecov/patch

plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportCreateQueryGroupAction.java#L47-L50

Added lines #L47 - L50 were not covered by tests

@Override
protected void doExecute(Task task, CreateQueryGroupRequest request, ActionListener<CreateQueryGroupResponse> listener) {
threadPool.executor(ThreadPool.Names.GENERIC)
threadPool.executor(ThreadPool.Names.SAME)
.execute(() -> queryGroupPersistenceService.persistInClusterStateMetadata(request.getQueryGroup(), listener));
}

Check warning on line 56 in plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportCreateQueryGroupAction.java

View check run for this annotation

Codecov / codecov/patch

plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/action/TransportCreateQueryGroupAction.java#L54-L56

Added lines #L54 - L56 were not covered by tests
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
* This class defines the functions for QueryGroup persistence
*/
public class QueryGroupPersistenceService {
private static final String SOURCE = "query-group-persistence-service";
static final String SOURCE = "query-group-persistence-service";
private static final String CREATE_QUERY_GROUP_THROTTLING_KEY = "create-query-group";
private static final Logger logger = LogManager.getLogger(QueryGroupPersistenceService.class);
/**
Expand Down Expand Up @@ -62,7 +62,7 @@
);
private final ClusterService clusterService;
private volatile int maxQueryGroupCount;
private final ThrottlingKey createQueryGroupThrottlingKey;
final ThrottlingKey createQueryGroupThrottlingKey;

/**
* Constructor for QueryGroupPersistenceService
Expand Down Expand Up @@ -111,7 +111,7 @@
clusterService.submitStateUpdateTask(SOURCE, new ClusterStateUpdateTask(Priority.NORMAL) {
@Override
public ClusterState execute(ClusterState currentState) throws Exception {
return saveQueryGroupInClusterState(queryGroup, currentState);

Check warning on line 114 in plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java

View check run for this annotation

Codecov / codecov/patch

plugins/workload-management/src/main/java/org/opensearch/plugin/wlm/service/QueryGroupPersistenceService.java#L114

Added line #L114 was not covered by tests
}

@Override
Expand Down Expand Up @@ -191,4 +191,11 @@
}
return map;
}

/**
* maxQueryGroupCount getter
*/
public int getMaxQueryGroupCount() {
return maxQueryGroupCount;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,16 @@

import org.opensearch.cluster.ClusterName;
import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.ClusterStateUpdateTask;
import org.opensearch.cluster.metadata.Metadata;
import org.opensearch.cluster.metadata.QueryGroup;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.collect.Tuple;
import org.opensearch.common.settings.ClusterSettings;
import org.opensearch.common.settings.Settings;
import org.opensearch.core.action.ActionListener;
import org.opensearch.plugin.wlm.QueryGroupTestUtils;
import org.opensearch.plugin.wlm.action.CreateQueryGroupResponse;
import org.opensearch.search.ResourceType;
import org.opensearch.test.OpenSearchTestCase;
import org.opensearch.threadpool.ThreadPool;
Expand All @@ -26,6 +30,8 @@
import java.util.List;
import java.util.Map;

import org.mockito.ArgumentCaptor;

import static org.opensearch.cluster.metadata.QueryGroup.builder;
import static org.opensearch.plugin.wlm.QueryGroupTestUtils.MEMORY_STRING;
import static org.opensearch.plugin.wlm.QueryGroupTestUtils.MONITOR_STRING;
Expand All @@ -34,18 +40,26 @@
import static org.opensearch.plugin.wlm.QueryGroupTestUtils._ID_ONE;
import static org.opensearch.plugin.wlm.QueryGroupTestUtils._ID_TWO;
import static org.opensearch.plugin.wlm.QueryGroupTestUtils.assertEqualQueryGroups;
import static org.opensearch.plugin.wlm.QueryGroupTestUtils.clusterSettings;
import static org.opensearch.plugin.wlm.QueryGroupTestUtils.clusterSettingsSet;
import static org.opensearch.plugin.wlm.QueryGroupTestUtils.preparePersistenceServiceSetup;
import static org.opensearch.plugin.wlm.QueryGroupTestUtils.queryGroupList;
import static org.opensearch.plugin.wlm.QueryGroupTestUtils.queryGroupOne;
import static org.opensearch.plugin.wlm.QueryGroupTestUtils.queryGroupTwo;
import static org.opensearch.plugin.wlm.service.QueryGroupPersistenceService.QUERY_GROUP_COUNT_SETTING_NAME;
import static org.opensearch.plugin.wlm.service.QueryGroupPersistenceService.SOURCE;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.anyString;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;

public class QueryGroupPersistenceServiceTests extends OpenSearchTestCase {

/**
* Test case to validate the creation logic of a single QueryGroup
* Test case to validate the creation logic of a QueryGroup
*/
public void testCreateQueryGroup() {
ruai0511 marked this conversation as resolved.
Show resolved Hide resolved
Tuple<QueryGroupPersistenceService, ClusterState> setup = preparePersistenceServiceSetup(new HashMap<>());
Expand Down Expand Up @@ -136,6 +150,9 @@ public void testCreateQueryGroupOverflowCount() {
assertThrows(RuntimeException.class, () -> queryGroupPersistenceService1.saveQueryGroupInClusterState(toCreate, clusterState));
}

/**
* Tests the invalid value of {@code node.query_group.max_count}
*/
public void testInvalidMaxQueryGroupCount() {
Settings settings = Settings.builder().put(QUERY_GROUP_COUNT_SETTING_NAME, 2).build();
ClusterSettings clusterSettings = new ClusterSettings(settings, clusterSettingsSet());
Expand All @@ -147,4 +164,84 @@ public void testInvalidMaxQueryGroupCount() {
);
assertThrows(IllegalArgumentException.class, () -> queryGroupPersistenceService.setMaxQueryGroupCount(-1));
}

/**
* Tests the valid value of {@code node.query_group.max_count}
*/
public void testValidMaxSandboxCountSetting() {
Settings settings = Settings.builder().put(QUERY_GROUP_COUNT_SETTING_NAME, 100).build();
ClusterService clusterService = new ClusterService(settings, clusterSettings(), mock(ThreadPool.class));
QueryGroupPersistenceService queryGroupPersistenceService = new QueryGroupPersistenceService(
clusterService,
settings,
clusterSettings()
);
queryGroupPersistenceService.setMaxQueryGroupCount(50);
assertEquals(50, queryGroupPersistenceService.getMaxQueryGroupCount());
}

/**
* Tests PersistInClusterStateMetadata function
*/
public void testPersistInClusterStateMetadata() {
ClusterService clusterService = mock(ClusterService.class);
@SuppressWarnings("unchecked")
ActionListener<CreateQueryGroupResponse> listener = mock(ActionListener.class);
QueryGroupPersistenceService queryGroupPersistenceService = new QueryGroupPersistenceService(
clusterService,
QueryGroupTestUtils.settings(),
clusterSettings()
);
queryGroupPersistenceService.persistInClusterStateMetadata(queryGroupOne, listener);
verify(clusterService).submitStateUpdateTask(eq(SOURCE), any());
}

/**
* Tests PersistInClusterStateMetadata function with inner functions
*/
public void testPersistInClusterStateMetadataInner() {
ClusterService clusterService = mock(ClusterService.class);
@SuppressWarnings("unchecked")
ActionListener<CreateQueryGroupResponse> listener = mock(ActionListener.class);
QueryGroupPersistenceService queryGroupPersistenceService = new QueryGroupPersistenceService(
clusterService,
QueryGroupTestUtils.settings(),
clusterSettings()
);
ArgumentCaptor<ClusterStateUpdateTask> captor = ArgumentCaptor.forClass(ClusterStateUpdateTask.class);
queryGroupPersistenceService.persistInClusterStateMetadata(queryGroupOne, listener);
verify(clusterService, times(1)).submitStateUpdateTask(eq(SOURCE), captor.capture());
ClusterStateUpdateTask capturedTask = captor.getValue();
assertEquals(queryGroupPersistenceService.createQueryGroupThrottlingKey, capturedTask.getClusterManagerThrottlingKey());

doAnswer(invocation -> {
ClusterStateUpdateTask task = invocation.getArgument(1);
task.clusterStateProcessed(SOURCE, mock(ClusterState.class), mock(ClusterState.class));
return null;
}).when(clusterService).submitStateUpdateTask(anyString(), any());
queryGroupPersistenceService.persistInClusterStateMetadata(queryGroupOne, listener);
verify(listener).onResponse(any(CreateQueryGroupResponse.class));
}

/**
* Tests PersistInClusterStateMetadata function with failure
*/
public void testPersistInClusterStateMetadataFailure() {
ClusterService clusterService = mock(ClusterService.class);
@SuppressWarnings("unchecked")
ActionListener<CreateQueryGroupResponse> listener = mock(ActionListener.class);
QueryGroupPersistenceService queryGroupPersistenceService = new QueryGroupPersistenceService(
clusterService,
QueryGroupTestUtils.settings(),
clusterSettings()
);
doAnswer(invocation -> {
ClusterStateUpdateTask task = invocation.getArgument(1);
Exception exception = new RuntimeException("Test Exception");
task.onFailure(SOURCE, exception);
return null;
}).when(clusterService).submitStateUpdateTask(anyString(), any());
queryGroupPersistenceService.persistInClusterStateMetadata(queryGroupOne, listener);
verify(listener).onFailure(any(RuntimeException.class));
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"test create a QueryGroup API successfully":
"test create QueryGroup API":
- skip:
version: " - 2.16.99"
reason: "QueryGroup WorkloadManagement feature was added in 2.17"
Expand All @@ -20,12 +20,6 @@
- match: { resource_limits.cpu: 0.4 }
- match: { resource_limits.memory: 0.2 }

---
"test bad QueryGroup API arguments":
- skip:
version: " - 2.16.99"
reason: "QueryGroup WorkloadManagement feature was added in 2.17"

- do:
catch: /illegal_argument_exception/
create_query_group_context:
Expand Down Expand Up @@ -78,12 +72,6 @@
}
}

---
"test create another QueryGroup API successfully":
- skip:
version: " - 2.16.99"
reason: "QueryGroup WorkloadManagement feature was added in 2.17"

- do:
create_query_group_context:
body:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,29 @@ public void testIllegalQueryGroupMode() {
);
}

public void testQueryGroupInitiation() {
QueryGroup queryGroup = new QueryGroup("analytics", randomMode(), Map.of(ResourceType.MEMORY, 0.4));
assertNotNull(queryGroup.getName());
assertNotNull(queryGroup.get_id());
assertNotNull(queryGroup.getResourceLimits());
assertFalse(queryGroup.getResourceLimits().isEmpty());
assertEquals(1, queryGroup.getResourceLimits().size());
assertTrue(allowedModes.contains(queryGroup.getResiliencyMode()));
assertTrue(queryGroup.getUpdatedAtInMillis() != 0);
}

public void testIllegalQueryGroupName() {
assertThrows(
NullPointerException.class,
() -> new QueryGroup("a".repeat(51), "_id", null, Map.of(ResourceType.MEMORY, 0.4), Instant.now().getMillis())
);
assertThrows(
NullPointerException.class,
() -> new QueryGroup("", "_id", null, Map.of(ResourceType.MEMORY, 0.4), Instant.now().getMillis())
);

}

public void testInvalidResourceLimitWhenInvalidSystemResourceValueIsGiven() {
assertThrows(
IllegalArgumentException.class,
Expand Down
Loading