Skip to content

Update settings with deepmerge #54

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
Pranoti27 merged 2 commits into from
Jan 7, 2026
Merged

Update settings with deepmerge #54

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fac035c
Update settings with deepmerge
Pranoti27 Dec 18, 2025
ad2abf7
fix unit test
Pranoti27 Dec 18, 2025
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
84 changes: 17 additions & 67 deletions src/main/java/io/clustercontroller/indices/IndexManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,38 +434,34 @@ public void updateSettings(String clusterId, String indexName, String settingsJs
throw new IllegalArgumentException("Index '" + indexName + "' does not exist in cluster '" + clusterId + "'");
}

// Parse and validate the new settings JSON
IndexSettings newSettings;
// Parse the new settings JSON to Map
Map<String, Object> newSettingsMap;
try {
newSettings = objectMapper.readValue(settingsJson, IndexSettings.class);
log.debug("Successfully parsed new settings JSON for index '{}': {}", indexName, newSettings);
newSettingsMap = objectMapper.readValue(settingsJson, Map.class);
log.debug("Successfully parsed new settings JSON for index '{}': {}", indexName, newSettingsMap);
} catch (Exception e) {
throw new IllegalArgumentException("Invalid JSON format for settings: " + e.getMessage(), e);
}

// Validate that new settings is not empty
if (newSettings == null) {
throw new IllegalArgumentException("Settings cannot be empty");
}

// Get existing settings and merge with new settings
IndexSettings existingSettings = null;
// Get existing settings (IndexSettings now preserves all fields via @JsonAnySetter)
Map<String, Object> mergedSettingsMap = new HashMap<>();
try {
existingSettings = metadataStore.getIndexSettings(clusterId, indexName);
log.debug("Retrieved existing settings for index '{}': {}", indexName, existingSettings);
IndexSettings existingSettings = metadataStore.getIndexSettings(clusterId, indexName);
if (existingSettings != null) {
mergedSettingsMap = objectMapper.convertValue(existingSettings, Map.class);
log.debug("Retrieved existing settings for index '{}': {}", indexName, mergedSettingsMap);
}
} catch (Exception e) {
log.warn("Failed to retrieve existing settings for index '{}', will create new settings: {}", indexName, e.getMessage());
log.warn("Failed to retrieve existing settings for index '{}', will use new settings only: {}", indexName, e.getMessage());
}

// Merge existing settings with new settings (new settings override existing ones)
IndexSettings mergedSettings = mergeIndexSettings(existingSettings, newSettings);

log.info("Merged settings for index '{}': existing={}, new={}, merged={}",
indexName, existingSettings, newSettings, mergedSettings);
// Deep merge: new settings override existing ones (partial update)
deepMerge(mergedSettingsMap, newSettingsMap);

@GeorgeZhai GeorgeZhai Dec 22, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question , is this deepMerge following the same logic as Opensearch setting update logic? or if's just a standard object deep merge.
if we want to remove an attribute, do we just pass a null value to remove it?

@Pranoti27 Pranoti27 Dec 22, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deepMerge is standard object deep merge. - https://github.com/TejasNaikk/cluster-controller/blob/main/src/main/java/io/clustercontroller/indices/IndexManager.java#L829

log.info("Merged settings for index '{}': {}", indexName, mergedSettingsMap);

// Update the settings in the metadata store with merged settings
// Save merged settings
try {
String mergedSettingsJson = objectMapper.writeValueAsString(mergedSettings);
String mergedSettingsJson = objectMapper.writeValueAsString(mergedSettingsMap);
metadataStore.setIndexSettings(clusterId, indexName, mergedSettingsJson);
log.info("Successfully updated settings for index '{}' in cluster '{}'", indexName, clusterId);
} catch (Exception e) {
Expand Down Expand Up @@ -702,52 +698,6 @@ private List<Integer> extractNumGroupsPerShard(Map<String, Object> settings, int
return new ArrayList<>(shardReplicaCount);
}

/**
* Merge two IndexSettings objects, with the second object's values taking precedence.
* Only non-null fields from newSettings are applied to oldSettings.
*/
private IndexSettings mergeIndexSettings(IndexSettings oldSettings, IndexSettings newSettings) {
// If both are null, return empty settings
if (oldSettings == null && newSettings == null) {
return new IndexSettings();
}

// If old is null, return new
if (oldSettings == null) {
return newSettings;
}

// If new is null, return old
if (newSettings == null) {
return oldSettings;
}

// Update fields in oldSettings with non-null values from newSettings
if (newSettings.getNumberOfShards() != null) {
oldSettings.setNumberOfShards(newSettings.getNumberOfShards());
log.debug("Updating number_of_shards to {}", newSettings.getNumberOfShards());
}

if (newSettings.getShardReplicaCount() != null) {
oldSettings.setShardReplicaCount(newSettings.getShardReplicaCount());
log.debug("Updating shard_replica_count");
}

if (newSettings.getNumGroupsPerShard() != null) {
oldSettings.setNumGroupsPerShard(newSettings.getNumGroupsPerShard());
log.debug("Updating num_groups_per_shard");
}

if (newSettings.getPausePullIngestion() != null) {
oldSettings.setPausePullIngestion(newSettings.getPausePullIngestion());
log.debug("Updating pause_pull_ingestion to {}", newSettings.getPausePullIngestion());
}

log.debug("Merged IndexSettings: result={}", oldSettings);

return oldSettings;
}

/**
* Update index metadata.
* Stores metadata as _meta field in index mappings.
Expand Down
36 changes: 30 additions & 6 deletions src/main/java/io/clustercontroller/models/IndexSettings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,23 @@
package io.clustercontroller.models;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonAnyGetter;
import com.fasterxml.jackson.annotation.JsonAnySetter;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
import lombok.AccessLevel;
import lombok.Data;
import lombok.Getter;
import lombok.NoArgsConstructor;
import java.util.ArrayList;

import java.util.HashMap;
import java.util.List;
import java.util.ArrayList;
import java.util.Map;

/**
* IndexSettings represents the settings for an index.
* Known fields are typed, unknown fields are captured in additionalProperties.
*/
@Data
@NoArgsConstructor
Expand All @@ -18,20 +27,35 @@ public class IndexSettings {
private Integer numberOfShards;

@JsonProperty("shard_replica_count")
private List<Integer> shardReplicaCount;
private List<Integer> shardReplicaCount = new ArrayList<>();

@JsonProperty("num_groups_per_shard")
private List<Integer> numGroupsPerShard;
private List<Integer> numGroupsPerShard = new ArrayList<>();

@JsonProperty("num_ingest_groups_per_shard")
private List<Integer> numIngestGroupsPerShard;
private List<Integer> numIngestGroupsPerShard = new ArrayList<>();

@JsonProperty("pause_pull_ingestion")
private Boolean pausePullIngestion;
private Boolean pausePullIngestion = false;

@JsonProperty("refresh_interval")
private String refreshInterval;

@JsonProperty("number_of_replicas")
private Integer numberOfReplicas;

// Capture all unknown fields (e.g., ingestion_source, knn, replication.type, etc.)
@JsonIgnore
@Getter(AccessLevel.NONE)
private Map<String, Object> additionalProperties = new HashMap<>();

@JsonAnyGetter
public Map<String, Object> getAdditionalProperties() {
return additionalProperties;
}

@JsonAnySetter
public void setAdditionalProperty(String key, Object value) {
additionalProperties.put(key, value);
}
}
8 changes: 4 additions & 4 deletions src/test/java/io/clustercontroller/store/EtcdMetadataStoreTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -670,11 +670,11 @@ public void testGetIndexSettingsWithEmptySettings() throws Exception {
// Execute
IndexSettings result = store.getIndexSettings("test-cluster", indexName);

// Verify - empty JSON {} will parse to an IndexSettings object with all null fields
// Verify - empty JSON {} will parse to an IndexSettings object with default values
assertThat(result).isNotNull();
assertThat(result.getNumberOfShards()).isNull(); // No default value
assertThat(result.getShardReplicaCount()).isNull();
assertThat(result.getPausePullIngestion()).isNull();
assertThat(result.getNumberOfShards()).isNull();
assertThat(result.getShardReplicaCount()).isEmpty(); // Default is empty list
assertThat(result.getPausePullIngestion()).isFalse(); // Default is false
}

// ------------------------- getIndexMappings tests -------------------------
Expand Down