Adding number of routing shards to index settings before passing into…

[ghost]

… GetSettingsResponse

Description

Adds number_of_routing_shards into indexSettings before passing into GetSettingsResponse. Alternative solution to PR-4443.

Related Issues

Resolves #14199

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

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.

[github-actions]

❌ Gradle check result for 950f0ad: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❌ Gradle check result for 82a9f3c: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❌ Gradle check result for ede87d5: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❌ Gradle check result for fa27c83: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❌ Gradle check result for 42f9754: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❌ Gradle check result for dcfe9d7: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❌ Gradle check result for 6e32fb5: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[Bukhtawar]

Do you intent to backport to 2.x, if yes please handle BWC

[github-actions]

❌ Gradle check result for 013105b: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❌ Gradle check result for 1b12a69: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❌ Gradle check result for 240d42d: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❌ Gradle check result for 1f865f4: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[ghost]

@gaobinlong Mentioning you here since I closed the previous PR in favor on this approach.

Sorry this is my first contribution so I'm having a bit a trouble tracking down failed tests. I see that org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT.test {p0=indices.get_settings/10_basic/Get /_settings}. I think this seems like some mixed version test? Do you know how I could go about trying to fix this test?

[github-actions]

❌ Gradle check result for d63ffbd: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❌ Gradle check result for 3cc3aac: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[ghost]

Do you intent to backport to 2.x, if yes please handle BWC

Sorry, new to contributing here! How would I handle the backwards compatibility for this? @Bukhtawar

[github-actions]

❌ Gradle check result for 8029d7d: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❌ Gradle check result for 3a85274: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❌ Gradle check result for 54d4952: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❌ Gradle check result for 24a4ecb: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

✅ Gradle check result for c14ce22: SUCCESS

[github-actions]

✅ Gradle check result for 02bd802: SUCCESS

[gaobinlong]

I have a concern that we always return the setting index.number_of_routing_shards when calling get index settings API, that setting is not commonly used(may only useful when splitting index), should we only show the setting when it was set explicitly? What do you think about this? @Bukhtawar @r1walz @reta @andrross

[reta]

I have a concern that we always return the setting index.number_of_routing_shards when calling get index settings API, that setting is not commonly used(may only useful when splitting index), should we only show the setting when it was set explicitly? What do you think about this? @Bukhtawar @r1walz @reta @andrross

Thanks @gaobinlong , I think this is very valid concern. This settings is only useful during the Resize API calls (and could be created temporarily for the index in question), it is not a regular index setting per se.

[r1walz]

I have a concern that we always return the setting index.number_of_routing_shards when calling get index settings API, that setting is not commonly used(may only useful when splitting index), should we only show the setting when it was set explicitly? What do you think about this? @Bukhtawar @r1walz @reta @andrross

It sounds like a better idea to only return the setting when explicitly set by the user. I think we can make this distinction by pushing the setting into the IndexMetadata#settings variable when it is provided by the user (passed into request while creating index or copied from sourceMetadata in case of resize action, etc). All settings pushed into IndexMetadata#settings are visible in the response of GET {index}/_settings call unless filtered.

Side Note: while looking into the issue, I found that INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING is only IndexScope settings while it should also be a Final settings as well because routing number of shards is also used during routing of docs in OperationRouting, it should not change after index creation

OpenSearch/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java

Line 475 in 4c25257

return Math.floorMod(hash, indexMetadata.getRoutingNumShards()) / indexMetadata.getRoutingFactor();

IndexScope settings can be updated when index is closed. Although, it's value is never read from the settings itself during operation routing. But, it created yet another discrepancy between these two variables representing same information (eg: in _cluster/state call).

[ghost]

I have a concern that we always return the setting index.number_of_routing_shards when calling get index settings API, that setting is not commonly used(may only useful when splitting index), should we only show the setting when it was set explicitly? What do you think about this? @Bukhtawar @r1walz @reta @andrross

It sounds like a better idea to only return the setting when explicitly set by the user. I think we can make this distinction by pushing the setting into the IndexMetadata#settings variable when it is provided by the user (passed into request while creating index or copied from sourceMetadata in case of resize action, etc). All settings pushed into IndexMetadata#settings are visible in the response of GET {index}/_settings call unless filtered.

Side Note: while looking into the issue, I found that INDEX_NUMBER_OF_ROUTING_SHARDS_SETTING is only IndexScope settings while it should also be a Final settings as well because routing number of shards is also used during routing of docs in OperationRouting, it should not change after index creation

OpenSearch/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java

Line 475 in 4c25257

return Math.floorMod(hash, indexMetadata.getRoutingNumShards()) / indexMetadata.getRoutingFactor();

IndexScope settings can be updated when index is closed. Although, it's value is never read from the settings itself during operation routing. But, it created yet another discrepancy between these two variables representing same information (eg: in _cluster/state call).

Hello! If I understand what you're saying correctly, I think what you're suggesting is similar to the approach that I initially implemented . I think the issue is that there seems to be a deliberate removal of the number_of_routing_shards in IndexMetadata on creation.

[opensearch-trigger-bot]

This PR is stalled because it has been open for 30 days with no activity.

[michlitz]

@gaobinlong, it looks like this PR was closed due to @PeacefulTortoise deleting their GitHub account. How close was this to being merged, and can someone from the AWS side pick this up if it's near resolved?

[gaobinlong]

@gaobinlong, it looks like this PR was closed due to @PeacefulTortoise deleting their GitHub account. How close was this to being merged, and can someone from the AWS side pick this up if it's near resolved?

This PR is not close to be merged, I'll take a look later.