Skip to content

Refactor WLM settings to use Setting objects and rename field #21143

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.

Sign up for GitHub

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

Merged
cwperks merged 1 commit into from
May 5, 2026
Merged

Refactor WLM settings to use Setting objects and rename field #21143

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
70 changes: 29 additions & 41 deletions ...rkload-management/src/javaRestTest/java/org/opensearch/rest/WorkloadManagementRestIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,77 +148,65 @@ public void testOperationWhenWlmDisabled() throws Exception {
}

public void testSearchSettings() throws Exception {
// Create with search_settings
// Create with settings
String createJson = """
{
"name": "search_test",
"resiliency_mode": "enforced",
"resource_limits": {"cpu": 0.3, "memory": 0.3},
"search_settings": {
"timeout": "30s"
"settings": {
"search.default_search_timeout": "30s"
}
}""";
Response response = performOperation("PUT", "_wlm/workload_group", createJson);
assertEquals(200, response.getStatusLine().getStatusCode());

// Verify search_settings in GET response
// Verify settings in GET response
Response getResponse = performOperation("GET", "_wlm/workload_group/search_test", null);
String responseBody = EntityUtils.toString(getResponse.getEntity());
assertTrue(responseBody.contains("\"search_settings\""));
assertTrue(responseBody.contains("\"timeout\":\"30s\""));
assertTrue(responseBody.contains("\"settings\""));
assertTrue(responseBody.contains("\"search.default_search_timeout\":\"30s\""));

// Update search_settings
// Update settings
String updateJson = """
{
"search_settings": {
"timeout": "1m"
"settings": {
"search.default_search_timeout": "1m"
}
}""";
Response updateResponse = performOperation("PUT", "_wlm/workload_group/search_test", updateJson);
assertEquals(200, updateResponse.getStatusLine().getStatusCode());

// Verify updated search_settings
// Verify updated settings
Response getResponse2 = performOperation("GET", "_wlm/workload_group/search_test", null);
String responseBody2 = EntityUtils.toString(getResponse2.getEntity());
assertTrue(responseBody2.contains("\"timeout\":\"1m\""));
assertTrue(responseBody2.contains("\"search.default_search_timeout\":\"1m\""));

performOperation("DELETE", "_wlm/workload_group/search_test", null);
}

static String getCreateJson(String name, String resiliencyMode, double cpu, double memory) {
return "{\n"
+ " \"name\": \""
+ name
+ "\",\n"
+ " \"resiliency_mode\": \""
+ resiliencyMode
+ "\",\n"
+ " \"resource_limits\": {\n"
+ " \"cpu\" : "
+ cpu
+ ",\n"
+ " \"memory\" : "
+ memory
+ "\n"
+ " },\n"
+ " \"search_settings\": {}\n"
+ "}";
return String.format(Locale.ROOT, """
{
"name": "%s",
"resiliency_mode": "%s",
"resource_limits": {
"cpu" : %s,
"memory" : %s
},
"settings": {}
}""", name, resiliencyMode, cpu, memory);
}

static String getUpdateJson(String resiliencyMode, double cpu, double memory) {
return "{\n"
+ " \"resiliency_mode\": \""
+ resiliencyMode
+ "\",\n"
+ " \"resource_limits\": {\n"
+ " \"cpu\" : "
+ cpu
+ ",\n"
+ " \"memory\" : "
+ memory
+ "\n"
+ " }\n"
+ "}";
return String.format(Locale.ROOT, """
{
"resiliency_mode": "%s",
"resource_limits": {
"cpu" : %s,
"memory" : %s
}
}""", resiliencyMode, cpu, memory);
}

Response performOperation(String method, String uriPath, String json) throws IOException {
Expand Down
2 changes: 1 addition & 1 deletion ...kload-management/src/test/java/org/opensearch/plugin/wlm/WorkloadManagementTestUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public class WorkloadManagementTestUtils {
public static final long TIMESTAMP_ONE = 4513232413L;
public static final long TIMESTAMP_TWO = 4513232415L;
public static final long TIMESTAMP_THREE = 4513232417L;
public static final Map<String, String> TEST_SEARCH_SETTINGS = Map.of("timeout", "30s");
public static final Settings TEST_SEARCH_SETTINGS = Settings.builder().put("search.default_search_timeout", "30s").build();
public static final WorkloadGroup workloadGroupOne = builder().name(NAME_ONE)
._id(_ID_ONE)
.mutableWorkloadGroupFragment(
Expand Down
46 changes: 24 additions & 22 deletions ...ment/src/test/java/org/opensearch/plugin/wlm/action/CreateWorkloadGroupResponseTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,17 @@ public void testToXContentCreateWorkloadGroup() throws IOException {
XContentBuilder builder = JsonXContent.contentBuilder().prettyPrint();
CreateWorkloadGroupResponse response = new CreateWorkloadGroupResponse(WorkloadManagementTestUtils.workloadGroupOne, RestStatus.OK);
String actual = response.toXContent(builder, mock(ToXContent.Params.class)).toString();
String expected = "{\n"
+ " \"_id\" : \"AgfUO5Ja9yfsYlONlYi3TQ==\",\n"
+ " \"name\" : \"workload_group_one\",\n"
+ " \"resiliency_mode\" : \"monitor\",\n"
+ " \"resource_limits\" : {\n"
+ " \"memory\" : 0.3\n"
+ " },\n"
+ " \"search_settings\" : { },\n"
+ " \"updated_at\" : 4513232413\n"
+ "}";
String expected = """
{
"_id" : "AgfUO5Ja9yfsYlONlYi3TQ==",
"name" : "workload_group_one",
"resiliency_mode" : "monitor",
"resource_limits" : {
"memory" : 0.3
},
"settings" : { },
"updated_at" : 4513232413
}""";
assertEquals(expected, actual);
}

Expand All @@ -75,18 +76,19 @@ public void testToXContentCreateWorkloadGroupWithSearchSettings() throws IOExcep
RestStatus.OK
);
String actual = response.toXContent(builder, mock(ToXContent.Params.class)).toString();
String expected = "{\n"
+ " \"_id\" : \"H6jVP6Kb0zgtZmPOmZj4UQ==\",\n"
+ " \"name\" : \"workload_group_three\",\n"
+ " \"resiliency_mode\" : \"enforced\",\n"
+ " \"resource_limits\" : {\n"
+ " \"memory\" : 0.5\n"
+ " },\n"
+ " \"search_settings\" : {\n"
+ " \"timeout\" : \"30s\"\n"
+ " },\n"
+ " \"updated_at\" : 4513232417\n"
+ "}";
String expected = """
{
"_id" : "H6jVP6Kb0zgtZmPOmZj4UQ==",
"name" : "workload_group_three",
"resiliency_mode" : "enforced",
"resource_limits" : {
"memory" : 0.5
},
"settings" : {
"search.default_search_timeout" : "30s"
},
"updated_at" : 4513232417
}""";
assertEquals(expected, actual);
}
}
10 changes: 5 additions & 5 deletions ...agement/src/test/java/org/opensearch/plugin/wlm/action/GetWorkloadGroupResponseTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public void testToXContentGetSingleWorkloadGroup() throws IOException {
"resource_limits" : {
"memory" : 0.3
},
"search_settings" : { },
"settings" : { },
"updated_at" : 4513232413
}
]
Expand Down Expand Up @@ -125,7 +125,7 @@ public void testToXContentGetMultipleWorkloadGroup() throws IOException {
"resource_limits" : {
"memory" : 0.3
},
"search_settings" : { },
"settings" : { },
"updated_at" : 4513232413
},
{
Expand All @@ -135,7 +135,7 @@ public void testToXContentGetMultipleWorkloadGroup() throws IOException {
"resource_limits" : {
"memory" : 0.6
},
"search_settings" : { },
"settings" : { },
"updated_at" : 4513232415
}
]
Expand Down Expand Up @@ -176,8 +176,8 @@ public void testToXContentGetWorkloadGroupWithSearchSettings() throws IOExceptio
"resource_limits" : {
"memory" : 0.5
},
"search_settings" : {
"timeout" : "30s"
"settings" : {
"search.default_search_timeout" : "30s"
},
"updated_at" : 4513232417
}
Expand Down
46 changes: 24 additions & 22 deletions ...ment/src/test/java/org/opensearch/plugin/wlm/action/UpdateWorkloadGroupResponseTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,17 @@ public void testToXContentUpdateSingleWorkloadGroup() throws IOException {
XContentBuilder builder = JsonXContent.contentBuilder().prettyPrint();
UpdateWorkloadGroupResponse otherResponse = new UpdateWorkloadGroupResponse(workloadGroupOne, RestStatus.OK);
String actual = otherResponse.toXContent(builder, mock(ToXContent.Params.class)).toString();
String expected = "{\n"
+ " \"_id\" : \"AgfUO5Ja9yfsYlONlYi3TQ==\",\n"
+ " \"name\" : \"workload_group_one\",\n"
+ " \"resiliency_mode\" : \"monitor\",\n"
+ " \"resource_limits\" : {\n"
+ " \"memory\" : 0.3\n"
+ " },\n"
+ " \"search_settings\" : { },\n"
+ " \"updated_at\" : 4513232413\n"
+ "}";
String expected = """
{
"_id" : "AgfUO5Ja9yfsYlONlYi3TQ==",
"name" : "workload_group_one",
"resiliency_mode" : "monitor",
"resource_limits" : {
"memory" : 0.3
},
"settings" : { },
"updated_at" : 4513232413
}""";
assertEquals(expected, actual);
}

Expand All @@ -76,18 +77,19 @@ public void testToXContentUpdateWorkloadGroupWithSearchSettings() throws IOExcep
RestStatus.OK
);
String actual = response.toXContent(builder, mock(ToXContent.Params.class)).toString();
String expected = "{\n"
+ " \"_id\" : \"H6jVP6Kb0zgtZmPOmZj4UQ==\",\n"
+ " \"name\" : \"workload_group_three\",\n"
+ " \"resiliency_mode\" : \"enforced\",\n"
+ " \"resource_limits\" : {\n"
+ " \"memory\" : 0.5\n"
+ " },\n"
+ " \"search_settings\" : {\n"
+ " \"timeout\" : \"30s\"\n"
+ " },\n"
+ " \"updated_at\" : 4513232417\n"
+ "}";
String expected = """
{
"_id" : "H6jVP6Kb0zgtZmPOmZj4UQ==",
"name" : "workload_group_three",
"resiliency_mode" : "enforced",
"resource_limits" : {
"memory" : 0.5
},
"settings" : {
"search.default_search_timeout" : "30s"
},
"updated_at" : 4513232417
}""";
assertEquals(expected, actual);
}
}
30 changes: 16 additions & 14 deletions server/src/main/java/org/opensearch/cluster/metadata/WorkloadGroup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.opensearch.common.UUIDs;
import org.opensearch.common.annotation.ExperimentalApi;
import org.opensearch.common.annotation.PublicApi;
import org.opensearch.common.settings.Settings;
import org.opensearch.core.common.io.stream.StreamInput;
import org.opensearch.core.common.io.stream.StreamOutput;
import org.opensearch.core.xcontent.ToXContentObject;
Expand Down Expand Up @@ -73,12 +74,12 @@ public WorkloadGroup(String name, String _id, MutableWorkloadGroupFragment mutab
throw new IllegalArgumentException("WorkloadGroup.updatedAtInMillis is not a valid epoch");
}

// Normalize null searchSettings to empty map for storage
if (mutableWorkloadGroupFragment.getSearchSettings() == null) {
// Normalize null settings to empty Settings for storage
if (mutableWorkloadGroupFragment.getSettings() == null) {
mutableWorkloadGroupFragment = new MutableWorkloadGroupFragment(
mutableWorkloadGroupFragment.getResiliencyMode(),
mutableWorkloadGroupFragment.getResourceLimits(),
new HashMap<>()
Settings.EMPTY
);
}

Expand Down Expand Up @@ -113,23 +114,23 @@ public static WorkloadGroup updateExistingWorkloadGroup(
}
final ResiliencyMode mode = Optional.ofNullable(mutableWorkloadGroupFragment.getResiliencyMode())
.orElse(existingGroup.getResiliencyMode());
// Handle search_settings update:
// Handle settings update:
// null = not specified (keep existing)
// empty map = explicitly clear (set to empty)
// non-empty map = replace with new values
final Map<String, String> mutableFragmentSearchSettings = mutableWorkloadGroupFragment.getSearchSettings();
final Map<String, String> updatedSearchSettings;
if (mutableFragmentSearchSettings == null) {
// empty Settings = explicitly clear (set to empty)
// non-empty Settings = replace with new values
final Settings mutableFragmentSettings = mutableWorkloadGroupFragment.getSettings();
final Settings updatedSettings;
if (mutableFragmentSettings == null) {
// Not specified - keep existing
updatedSearchSettings = new HashMap<>(existingGroup.getSearchSettings());
updatedSettings = Settings.builder().put(existingGroup.getSettings()).build();
} else {
// Specified (empty or non-empty) - use the new value
updatedSearchSettings = new HashMap<>(mutableFragmentSearchSettings);
updatedSettings = Settings.builder().put(mutableFragmentSettings).build();
}
return new WorkloadGroup(
existingGroup.getName(),
existingGroup.get_id(),
new MutableWorkloadGroupFragment(mode, updatedResourceLimits, updatedSearchSettings),
new MutableWorkloadGroupFragment(mode, updatedResourceLimits, updatedSettings),
Instant.now().getMillis()
);
}
Expand Down Expand Up @@ -201,8 +202,9 @@ public Map<ResourceType, Double> getResourceLimits() {
return getMutableWorkloadGroupFragment().getResourceLimits();
}

public Map<String, String> getSearchSettings() {
return getMutableWorkloadGroupFragment().getSearchSettings();
Comment thread
dzane17 marked this conversation as resolved.
@ExperimentalApi
public Settings getSettings() {
return getMutableWorkloadGroupFragment().getSettings();
}

public String get_id() {
Expand Down
24 changes: 24 additions & 0 deletions server/src/main/java/org/opensearch/common/settings/Settings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,30 @@ public static void writeSettingsToStream(Settings settings, StreamOutput out) th
}
}

/**
* Reads an optional {@link Settings} from the stream. Returns {@code null} if no settings were written.
* Counterpart to {@link #writeOptionalSettingsToStream(Settings, StreamOutput)}.
*/
public static Settings readOptionalSettingsFromStream(StreamInput in) throws IOException {
if (in.readBoolean()) {
return readSettingsFromStream(in);
}
return null;
}

/**
* Writes an optional {@link Settings} to the stream. A {@code null} value is permitted.
* Counterpart to {@link #readOptionalSettingsFromStream(StreamInput)}.
*/
public static void writeOptionalSettingsToStream(Settings settings, StreamOutput out) throws IOException {
if (settings != null) {
out.writeBoolean(true);
writeSettingsToStream(settings, out);
} else {
out.writeBoolean(false);
}
}

/**
* Returns a builder to be used in order to build settings.
*/
Expand Down
Loading
Loading