-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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 support of default replica count change #5610
Changes from 17 commits
65a1d52
f563943
ebe089a
58d9f7c
dba0e3e
5b7ec7a
d28ae15
2eac09c
bd851a3
472c9d6
7ebab23
63c39e5
bfc3915
0aa504f
045617b
b9c406e
40e8706
615e81c
099a7d1
3a1afc0
4fa42d8
231858d
80f8da0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -161,6 +161,13 @@ public interface Custom extends NamedDiffable<Custom>, ToXContentFragment, Clust | |
EnumSet<XContentContext> context(); | ||
} | ||
|
||
public static final Setting<Integer> DEFAULT_REPLICA_COUNT_SETTING = Setting.intSetting( | ||
"cluster.default_replica_count", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we use |
||
1, | ||
Property.Dynamic, | ||
Property.NodeScope | ||
); | ||
|
||
public static final Setting<Boolean> SETTING_READ_ONLY_SETTING = Setting.boolSetting( | ||
"cluster.blocks.read_only", | ||
false, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -123,6 +123,7 @@ | |
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_INDEX_UUID; | ||
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS; | ||
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS; | ||
import static org.opensearch.cluster.metadata.Metadata.DEFAULT_REPLICA_COUNT_SETTING; | ||
|
||
/** | ||
* Service responsible for submitting create index requests | ||
|
@@ -554,7 +555,6 @@ private ClusterState applyCreateIndexRequestWithV1Templates( | |
xContentRegistry | ||
) | ||
); | ||
|
||
final Settings aggregatedIndexSettings = aggregateIndexSettings( | ||
currentState, | ||
request, | ||
|
@@ -862,7 +862,7 @@ static Settings aggregateIndexSettings( | |
indexSettingsBuilder.put(SETTING_NUMBER_OF_SHARDS, INDEX_NUMBER_OF_SHARDS_SETTING.get(settings)); | ||
} | ||
if (INDEX_NUMBER_OF_REPLICAS_SETTING.exists(indexSettingsBuilder) == false) { | ||
indexSettingsBuilder.put(SETTING_NUMBER_OF_REPLICAS, INDEX_NUMBER_OF_REPLICAS_SETTING.get(settings)); | ||
indexSettingsBuilder.put(SETTING_NUMBER_OF_REPLICAS, DEFAULT_REPLICA_COUNT_SETTING.get(currentState.metadata().settings())); | ||
} | ||
if (settings.get(SETTING_AUTO_EXPAND_REPLICAS) != null && indexSettingsBuilder.get(SETTING_AUTO_EXPAND_REPLICAS) == null) { | ||
indexSettingsBuilder.put(SETTING_AUTO_EXPAND_REPLICAS, settings.get(SETTING_AUTO_EXPAND_REPLICAS)); | ||
|
@@ -1189,10 +1189,10 @@ List<String> getIndexSettingsValidationErrors( | |
validationErrors.addAll(validatePrivateSettingsNotExplicitlySet(settings, indexScopedSettings)); | ||
} | ||
if (indexName.isEmpty() || indexName.get().charAt(0) != '.') { | ||
// Apply aware replica balance only to non system indices | ||
// Apply aware replica balance validation only to non system indices | ||
int replicaCount = settings.getAsInt( | ||
IndexMetadata.SETTING_NUMBER_OF_REPLICAS, | ||
INDEX_NUMBER_OF_REPLICAS_SETTING.getDefault(Settings.EMPTY) | ||
DEFAULT_REPLICA_COUNT_SETTING.get(this.clusterService.state().metadata().settings()) | ||
Comment on lines
-1192
to
+1195
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to ensure system indices do not break There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This validation will not be applied on the system index
|
||
); | ||
AutoExpandReplicas autoExpandReplica = AutoExpandReplicas.SETTING.get(settings); | ||
Optional<String> error = awarenessReplicaBalance.validate(replicaCount, autoExpandReplica); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,10 +13,10 @@ | |
import org.opensearch.common.settings.Setting; | ||
import org.opensearch.common.settings.Settings; | ||
|
||
import java.util.HashMap; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Optional; | ||
import java.util.HashMap; | ||
|
||
import static java.lang.Math.max; | ||
import static org.opensearch.cluster.routing.allocation.decider.AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_ATTRIBUTE_SETTING; | ||
|
@@ -31,8 +31,9 @@ | |
* Helps in balancing shards across all awareness attributes and ensuring high availability of data. | ||
*/ | ||
public class AwarenessReplicaBalance { | ||
public static final String SETTING_CLUSTER_ROUTING_ALLOCATION_AWARENESS_BALANCE = "cluster.routing.allocation.awareness.balance"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: do we need this? |
||
public static final Setting<Boolean> CLUSTER_ROUTING_ALLOCATION_AWARENESS_BALANCE_SETTING = Setting.boolSetting( | ||
"cluster.routing.allocation.awareness.balance", | ||
SETTING_CLUSTER_ROUTING_ALLOCATION_AWARENESS_BALANCE, | ||
false, | ||
Setting.Property.Dynamic, | ||
Setting.Property.NodeScope | ||
|
@@ -54,14 +55,17 @@ public AwarenessReplicaBalance(Settings settings, ClusterSettings clusterSetting | |
); | ||
setAwarenessBalance(CLUSTER_ROUTING_ALLOCATION_AWARENESS_BALANCE_SETTING.get(settings)); | ||
clusterSettings.addSettingsUpdateConsumer(CLUSTER_ROUTING_ALLOCATION_AWARENESS_BALANCE_SETTING, this::setAwarenessBalance); | ||
|
||
} | ||
|
||
private void setAwarenessBalance(Boolean awarenessBalance) { | ||
this.awarenessBalance = awarenessBalance; | ||
} | ||
|
||
private void setForcedAwarenessAttributes(Settings forceSettings) { | ||
this.forcedAwarenessAttributes = getForcedAwarenessAttributes(forceSettings); | ||
} | ||
|
||
public static Map<String, List<String>> getForcedAwarenessAttributes(Settings forceSettings) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same do we need this change |
||
Map<String, List<String>> forcedAwarenessAttributes = new HashMap<>(); | ||
Map<String, Settings> forceGroups = forceSettings.getAsGroups(); | ||
for (Map.Entry<String, Settings> entry : forceGroups.entrySet()) { | ||
|
@@ -70,7 +74,7 @@ private void setForcedAwarenessAttributes(Settings forceSettings) { | |
forcedAwarenessAttributes.put(entry.getKey(), aValues); | ||
} | ||
} | ||
this.forcedAwarenessAttributes = forcedAwarenessAttributes; | ||
return forcedAwarenessAttributes; | ||
} | ||
|
||
private void setAwarenessAttributes(List<String> awarenessAttributes) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correct this .