Skip to content

Update settings with deepmerge - #54

Merged
Pranoti27 merged 2 commits into
mainfrom
pranoti/fix_settings_merge
Jan 7, 2026
Merged

Pranoti27 merged 2 commits into
mainfrom
pranoti/fix_settings_merge

Conversation

@Pranoti27

@Pranoti27 Pranoti27 commented Dec 18, 2025 •

Copy link
Copy Markdown
Collaborator

Update updateSettings method to use existing deepmerge for function to correctly merge settings before updating.

Tested locally -


etcdctl put "/esdock-opensearch-os-cn-tier2-production-cluster-dca/indices/os-cn-mp13n-v1-1001/settings" '{"index":{"pause_pull_ingestion":false,"ingestion_source":{"type":"kafka","pointer.init.reset":"reset_by_timestamp","param":{"topic":"mp13n-os-cloudnative","bootstrap_servers":"kafka-i.phx.uber.internal:9092"},"pointer.init.reset.value":20909090},"refresh_interval":"30s","number_of_shards":1,"replication.type":"SEGMENT","max_result_window":10000,"remote_store.enabled":true,"knn":true,"remote_store.segment.repository":"gcs-snapshot-repository","number_of_replicas":1}}'

Call update settings api -
curl -X PUT "http://localhost:8080/esdock-opensearch-os-cn-tier2-production-cluster-dca/os-cn-mp13n-v1-1001/_settings" \
  -H "Content-Type: application/json" \
  -d '{
    "pause_pull_ingestion": true
  }'

/esdock-opensearch-os-cn-tier2-production-cluster-dca/indices/os-cn-mp13n-v1-1001/settings
{"index":{"number_of_shards":1,"shard_replica_count":[],"num_groups_per_shard":[],"num_ingest_groups_per_shard":[],"pause_pull_ingestion":true,"refresh_interval":"30s","number_of_replicas":1,"ingestion_source":{"type":"kafka","pointer.init.reset":"reset_by_timestamp","param":{"topic":"mp13n-os-cloudnative","bootstrap_servers":"kafka-i.phx.uber.internal:9092"},"pointer.init.reset.value":20909090},"replication.type":"SEGMENT","max_result_window":10000,"remote_store.enabled":true,"knn":true,"remote_store.segment.repository":"gcs-snapshot-repository"}}
`

@GeorgeZhai GeorgeZhai left a comment

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.

LGTM. left a question

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);

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?

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.

@Pranoti27
Pranoti27 force-pushed the pranoti/fix_settings_merge branch from adec8cf to ad2abf7 Compare January 7, 2026 23:09
@Pranoti27
Pranoti27 merged commit c71a3cf into main Jan 7, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants