-
Notifications
You must be signed in to change notification settings - Fork 344
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
Use ES single redundancy by default #531
Use ES single redundancy by default #531
Conversation
Signed-off-by: Pavol Loffay <[email protected]>
@@ -133,7 +133,7 @@ func normalizeElasticsearch(spec *v1.ElasticsearchSpec) { | |||
spec.NodeCount = 1 | |||
} | |||
if spec.RedundancyPolicy == "" { | |||
spec.RedundancyPolicy = esv1.ZeroRedundancy | |||
spec.RedundancyPolicy = esv1.SingleRedundancy |
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.
This string option is used to specify replica shards. Instead of exposing an integer ECL decided to expose a human readable string. The number of nodes is taken into the account when calculating the replica shards.
jaeger-operator/pkg/storage/elasticsearch.go
Lines 200 to 212 in 0d6bd5f
func calculateReplicaShards(policyType esv1.RedundancyPolicyType, dataNodes int) int { | |
switch policyType { | |
case esv1.FullRedundancy: | |
return dataNodes - 1 | |
case esv1.MultipleRedundancy: | |
return (dataNodes - 1) / 2 | |
case esv1.SingleRedundancy: | |
return 1 | |
case esv1.ZeroRedundancy: | |
return 0 | |
default: | |
return 1 | |
} |
This should be defaulting to SingleRedundancy
to keep backwards compatibility.
@@ -133,7 +133,7 @@ func normalizeElasticsearch(spec *v1.ElasticsearchSpec) { | |||
spec.NodeCount = 1 | |||
} | |||
if spec.RedundancyPolicy == "" { |
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.
Here we should be also checking the number of nodes. If it is a single node cluster the redundancy should be zero - https://angristan.xyz/elasticsearch-6-shard-replica-settings-for-single-node-cluster/ otherwise the health of the cluster will be yellow (not green :)).
I will create a separate issue for it.
Codecov Report
@@ Coverage Diff @@
## master #531 +/- ##
=======================================
Coverage 91.97% 91.97%
=======================================
Files 69 69
Lines 3501 3501
=======================================
Hits 3220 3220
Misses 196 196
Partials 85 85
Continue to review full report at Codecov.
|
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.
LGTM
Although wouldn't it be easier just to add the node count check aswell and set to zero redundancy as part of this PR? Your call.
I want to test it first. |
Related to #501 (comment)
Signed-off-by: Pavol Loffay [email protected]