Making shard copy count a multiple of attribute count by gbbafna · Pull Request #3462 · opensearch-project/OpenSearch

gbbafna · 2022-05-27T10:10:05Z

Description

A boolean cluster level setting cluster.routing.allocation.awareness.balance which is false by default . When true, we would validate that total copies is always a multiple of awareness attribute value count . If not, we will throw a validation exception. If there are multiple awareness attributes, the balance needs to ensure that largest varying attribute of awareness_attribute is equally balance. For ex, if there are 2 Awareness Attributes, zones and rack ids, each having 2 possible values , total copies will to be multiple of 2.

Tested

Create/Update Index APIs
Snapshot restore
Index Template creation.

Issues Resolved

#3461

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

opensearch-ci-bot · 2022-05-27T10:32:22Z

❌ Gradle Check failure 5870d5aca952b773ac4632ebf44f0093bc3193bb
Log 5628

Reports 5628

opensearch-ci-bot · 2022-06-08T11:05:25Z

❌ Gradle Check failure 7fb9e069ea6c1bdcc109505f33366645630cd018
Log 5853

Reports 5853

dblock · 2022-06-13T23:40:37Z

@gbbafna Are you trying to finish this? Care to explain a bit more what this accomplishes above?

gbbafna · 2022-06-14T04:03:58Z

@gbbafna Are you trying to finish this? Care to explain a bit more what this accomplishes above?

Hi @dblock , yes, I am working on this.

Please check the details in #3461 . Let me know on which part I need to give more context on .

opensearch-ci-bot · 2022-06-14T10:10:59Z

❌ Gradle Check failure 0f3ee95a0fa74f3c764aef072ab98f0855e03575
Log 5975

Reports 5975

opensearch-ci-bot · 2022-06-15T09:09:08Z

❌ Gradle Check failure db0cc2be50e27d2eafe05a1a8d88cca16610d159
Log 6020

Reports 6020

opensearch-ci-bot · 2022-06-15T09:09:34Z

❌ Gradle Check failure 870d6dc03ce34ae13537cc45c036cddb04512878
Log 6021

Reports 6021

opensearch-ci-bot · 2022-06-15T09:45:43Z

✅ Gradle Check success 332855374dbfcf2924bfb1c990ba1772a57ee0cc
Log 6023

Reports 6023

server/src/main/java/org/opensearch/cluster/routing/allocation/AwarenessReplicaBalance.java

opensearch-ci-bot · 2022-06-16T14:18:38Z

✅ Gradle Check success 41e8297a97cbd3bd96c71675e10e99d19fad49b0
Log 6063

Reports 6063

server/src/main/java/org/opensearch/cluster/routing/allocation/AwarenessReplicaBalance.java

Bukhtawar · 2022-06-24T17:52:23Z

server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java

Do we also need to mention which attribute zone/rack etc?

Good point. It will add the complexity of returning attribute as well from awarenessAttributes function , just for logging purpose. So I prefer not doing it . On seeing the error message with and without the attribute, user would anyways need to revisit awareness attributes values . Do you think this behavior is okay ?

Bukhtawar · 2022-06-24T17:55:12Z

server/src/main/java/org/opensearch/cluster/metadata/MetadataUpdateSettingsService.java

Can we put the multiple logic as a Predicate in the AwarenessReplicaBalance

Done. But we still need to expose awarenessAttributes() for the validation exception .

...er/src/test/java/org/opensearch/cluster/routing/allocation/AwarenessReplicaBalanceTests.java

opensearch-ci-bot · 2022-06-27T08:22:30Z

✅ Gradle Check success 8b401e5901a19de0bd5c36e7ac0798318e9255aa
Log 6358

Reports 6358

server/src/main/java/org/opensearch/cluster/routing/allocation/AwarenessReplicaBalance.java

server/src/main/java/org/opensearch/cluster/metadata/MetadataUpdateSettingsService.java

server/src/main/java/org/opensearch/cluster/routing/allocation/AwarenessReplicaBalance.java

github-actions · 2022-07-11T05:26:46Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/371/
CommitID: 7d390779244a385c49054d4e9fc35d3d4969c66a

Bukhtawar · 2022-07-14T09:03:56Z

server/src/main/java/org/opensearch/node/Node.java

+            final AwarenessReplicaBalance awarenessReplicaBalance = new AwarenessReplicaBalance(
+                settings,
+                clusterService.getClusterSettings()
+            );


Do we need spl handling for system indices. How does this change modify dashboard, security and other indices

Thanks for catching this . We definitely need to skip validation for system indices . However I realized that in OpenSearch , only OpenSearch-Dashboards create system indices. All the plugins don't extend SystemIndexPlugin and extend ActionPlugin instead , hence not creating system index . I have created an issue to discuss this .

For now, the alternate is not to apply the validation on dot indices . By that , we will not be applying it on data streams, but that should be taken care at the time of index template creation . I am implementing that for now. Let me know if you see any concerns .

Bukhtawar · 2022-07-14T09:51:15Z

server/src/main/java/org/opensearch/cluster/routing/allocation/AwarenessReplicaBalance.java

+    public Optional<String> validate(int replicaCount) {
+        if ((replicaCount + 1) % maxAwarenessAttributes() != 0) {
+            String errorMessage = "expected total copies needs to be a multiple of total awareness attributes ["
+                + maxAwarenessAttributes()
+                + "]";
+            return Optional.of(errorMessage);
+        }
+        return Optional.empty();
+    }
+
+}


Ideally this should throw a validation exception if the validation fails and the exception can carry the message as well. The interface looks inconsistent otherwise

I have modeled this on lines of checkShardLimit. The good thing about this is it can let the caller collect all validation errors and then throw validation exception.

… path Signed-off-by: Gaurav Bafna <gbbafna@amazon.com>

github-actions · 2022-07-19T02:50:48Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/764/
CommitID: ac848dd

Bukhtawar · 2022-07-29T06:07:23Z

Would be great to see rollover tests as well.

github-actions · 2022-07-29T06:27:45Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1093/
CommitID: c165581

gbbafna · 2022-07-29T07:00:26Z

Both the failing tests are passing locally and are unrelated to the change here.

org.opensearch.cluster.routing.allocation.decider.DiskThresholdDeciderIT.testRestoreSnapshotAllocationDoesNotExceedWatermark
org.opensearch.search.DeletePitMultiNodeTests.testDeletePitWhileNodeDrop

github-actions · 2022-07-29T07:11:50Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1095/
CommitID: be424a8f61817bde4addba252a95cd5671f9665c

Signed-off-by: Gaurav Bafna <gbbafna@amazon.com>

github-actions · 2022-07-29T09:45:26Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/1097/
CommitID: a0c8fdbe66308d59e0139ff87da1442660cf801c

github-actions · 2022-07-29T09:51:24Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/1098/
CommitID: e81d63a

opensearch-trigger-bot · 2022-07-29T16:15:06Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-3462-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 82ff463fd1bc07c241c8674b72182f2875a60425
# Push it to GitHub
git push --set-upstream origin backport/backport-3462-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-3462-to-2.x.

…ject#3462) * Making all the copies a multiple of attribute count Signed-off-by: Gaurav Bafna <gbbafna@amazon.com> (cherry picked from commit 82ff463)

* [Backport 2.x] Making all the copies a multiple of attribute count Signed-off-by: Gaurav Bafna <gbbafna@amazon.com>

gbbafna requested review from a team and reta as code owners May 27, 2022 10:10

gbbafna changed the title ~~Making all copies a multiple of attribute count~~ Making shard copy count a multiple of attribute count May 27, 2022

saratvemulapalli requested a review from Bukhtawar June 6, 2022 22:57

gbbafna force-pushed the replica-count branch from 5870d5a to 7fb9e06 Compare June 8, 2022 10:42

dblock requested a review from andrross June 13, 2022 23:40

gbbafna marked this pull request as draft June 14, 2022 04:00

gbbafna force-pushed the replica-count branch from 7fb9e06 to 0f3ee95 Compare June 14, 2022 09:50

gbbafna force-pushed the replica-count branch from db0cc2b to 870d6dc Compare June 15, 2022 08:49

gbbafna force-pushed the replica-count branch 2 times, most recently from 1e40a15 to 3328553 Compare June 15, 2022 09:15

Bukhtawar reviewed Jun 15, 2022

View reviewed changes

server/src/main/java/org/opensearch/cluster/routing/allocation/AwarenessReplicaBalance.java Outdated Show resolved Hide resolved

gbbafna marked this pull request as ready for review June 17, 2022 04:12

gbbafna requested a review from Bukhtawar June 17, 2022 06:04

Bukhtawar reviewed Jun 24, 2022

View reviewed changes

server/src/main/java/org/opensearch/cluster/routing/allocation/AwarenessReplicaBalance.java Outdated Show resolved Hide resolved

Bukhtawar reviewed Jun 24, 2022

View reviewed changes

gbbafna requested a review from Bukhtawar June 27, 2022 07:50

Bukhtawar reviewed Jul 9, 2022

View reviewed changes

gbbafna force-pushed the replica-count branch from 7d39077 to 1d1c6da Compare July 13, 2022 04:33

Bukhtawar reviewed Jul 14, 2022

View reviewed changes

Ignoring system index in replica count validation creation & updation…

ac848dd

… path Signed-off-by: Gaurav Bafna <gbbafna@amazon.com>

gbbafna mentioned this pull request Jul 19, 2022

[Discusss] Plugins should use system indices instead of dot indices #3942

Open

Bukhtawar approved these changes Jul 29, 2022

View reviewed changes

Merge branch 'main' into replica-count

c165581

gbbafna force-pushed the replica-count branch from be424a8 to a0c8fdb Compare July 29, 2022 09:21

Adding rollover Integ Test

e81d63a

Signed-off-by: Gaurav Bafna <gbbafna@amazon.com>

gbbafna force-pushed the replica-count branch from a0c8fdb to e81d63a Compare July 29, 2022 09:22

Bukhtawar merged commit 82ff463 into opensearch-project:main Jul 29, 2022

Bukhtawar added the backport 2.x Backport to 2.x branch label Jul 29, 2022

Bukhtawar pushed a commit that referenced this pull request Aug 2, 2022

Making shard copy count a multiple of attribute count (#3462) (#4086)

ed0af68

* [Backport 2.x] Making all the copies a multiple of attribute count Signed-off-by: Gaurav Bafna <gbbafna@amazon.com>

kartg mentioned this pull request Aug 3, 2022

[META] Making all copies of shards spread evenly across all Awareness Attribute #3367

Closed

Bukhtawar mentioned this pull request Dec 31, 2022

Adding @gbbafna to OpenSearch maintainers #5668

Merged

2 tasks

ohltyler mentioned this pull request Feb 13, 2023

Sample data ingestions fails when cluster.routing.allocation.awareness.balance enabled opensearch-project/anomaly-detection-dashboards-plugin#417

Closed

Conversation

gbbafna commented May 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Tested

Issues Resolved

Check List

Uh oh!

opensearch-ci-bot commented May 27, 2022

Uh oh!

opensearch-ci-bot commented Jun 8, 2022

Uh oh!

dblock commented Jun 13, 2022

Uh oh!

gbbafna commented Jun 14, 2022

Uh oh!

opensearch-ci-bot commented Jun 14, 2022

Uh oh!

opensearch-ci-bot commented Jun 15, 2022

Uh oh!

opensearch-ci-bot commented Jun 15, 2022

Uh oh!

opensearch-ci-bot commented Jun 15, 2022

Uh oh!

Uh oh!

opensearch-ci-bot commented Jun 16, 2022

Uh oh!

Uh oh!

Bukhtawar Jun 24, 2022

Choose a reason for hiding this comment

Uh oh!

gbbafna Jun 27, 2022

Choose a reason for hiding this comment

Uh oh!

Bukhtawar Jun 24, 2022

Choose a reason for hiding this comment

Uh oh!

gbbafna Jun 27, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

opensearch-ci-bot commented Jun 27, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jul 11, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

Bukhtawar Jul 14, 2022

Choose a reason for hiding this comment

Uh oh!

gbbafna Jul 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bukhtawar Jul 14, 2022

Choose a reason for hiding this comment

Uh oh!

gbbafna Jul 14, 2022

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 19, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

Bukhtawar commented Jul 29, 2022

Uh oh!

github-actions bot commented Jul 29, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

gbbafna commented Jul 29, 2022

Uh oh!

github-actions bot commented Jul 29, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Jul 29, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Jul 29, 2022

Gradle Check (Jenkins) Run Completed with:

gbbafna commented May 27, 2022 •

edited

Loading

gbbafna Jul 18, 2022 •

edited

Loading