Skip to content
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

Fix get index settings API doesn't show number_of_routing_shards when it was explicitly set on index creation #16294

Merged
merged 4 commits into from
Oct 23, 2024

Conversation

gaobinlong
Copy link
Collaborator

@gaobinlong gaobinlong commented Oct 12, 2024

Description

If the index level setting index.number_of_routing_shards is set explicitly at the index creation time, we don't show the setting in the response of get settings API, and we can only get the value by GET _cluster/state, this PR makes that setting shown in the response of the get settings API when it's explicitly set.

Related Issues

#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.

… it was explicitly set on index creation

Signed-off-by: Gao Binlong <gbinlong@amazon.com>
@gaobinlong
Copy link
Collaborator Author

@r1walz @Bukhtawar could you help to review this PR? Thanks!

dbwiddis
dbwiddis previously approved these changes Oct 12, 2024
Copy link
Contributor

❌ Gradle check result for 7eaef9c: 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?

Signed-off-by: Gao Binlong <gbinlong@amazon.com>
Copy link
Contributor

❌ Gradle check result for 0f6f4e5: 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?

Signed-off-by: Gao Binlong <gbinlong@amazon.com>
Copy link
Contributor

✅ Gradle check result for a981af4: SUCCESS

Copy link

codecov bot commented Oct 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.98%. Comparing base (9ddee61) to head (69fa5c6).
Report is 43 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #16294      +/-   ##
============================================
- Coverage     72.03%   71.98%   -0.05%     
- Complexity    64782    64810      +28     
============================================
  Files          5307     5307              
  Lines        302545   302542       -3     
  Branches      43703    43703              
============================================
- Hits         217925   217778     -147     
- Misses        66712    66914     +202     
+ Partials      17908    17850      -58     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Gao Binlong <gbinlong@amazon.com>
Copy link
Contributor

✅ Gradle check result for 69fa5c6: SUCCESS

@gaobinlong
Copy link
Collaborator Author

@reta, could you help to take a second look at this PR? Thanks! This change doesn't target for 2.18.0.

@reta reta merged commit 5941a7e into opensearch-project:main Oct 23, 2024
37 of 38 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 23, 2024
… it was explicitly set on index creation (#16294)

* Fix get index settings API doesn't show number_of_routing_shards when it was explicitly set on index creation

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Update skip version in rest yaml test file

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Fix test failure

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

---------

Signed-off-by: Gao Binlong <gbinlong@amazon.com>
(cherry picked from commit 5941a7e)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
reta added a commit that referenced this pull request Oct 23, 2024
…ing_shards when it was explicitly set on index creation (#16453)

* Fix get index settings API doesn't show number_of_routing_shards when it was explicitly set on index creation (#16294)

* Fix get index settings API doesn't show number_of_routing_shards when it was explicitly set on index creation

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Update skip version in rest yaml test file

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

* Fix test failure

Signed-off-by: Gao Binlong <gbinlong@amazon.com>

---------

Signed-off-by: Gao Binlong <gbinlong@amazon.com>
(cherry picked from commit 5941a7e)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* Update 40_number_of_routing_shards.yml

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

---------

Signed-off-by: Gao Binlong <gbinlong@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Andriy Redko <andriy.redko@aiven.io>
@dblock
Copy link
Member

dblock commented Oct 24, 2024

I think this caused a YAML REST test in the python client to begin failing. I don't quite understand what those tests do, but what I know is that they fetch the YAML REST test list from main, then run those tests via the Python client. I fixed CI by skipping OpenSearch-main/rest-api-spec/src/main/resources/rest-api-spec/test/indices/get_settings/40_number_of_routing_shards[0] in opensearch-project/opensearch-py#838, but this can't be right. Maybe someone (@gaobinlong) could help me understand what's going on there?

@gaobinlong
Copy link
Collaborator Author

I think this caused a YAML REST test in the python client to begin failing. I don't quite understand what those tests do, but what I know is that they fetch the YAML REST test list from main, then run those tests via the Python client. I fixed CI by skipping OpenSearch-main/rest-api-spec/src/main/resources/rest-api-spec/test/indices/get_settings/40_number_of_routing_shards[0] in opensearch-project/opensearch-py#838, but this can't be right. Maybe someone (@gaobinlong) could help me understand what's going on there?

Sure, I'll take a look, how do I check the test failure? Is there any logs? @dblock

@dblock
Copy link
Member

dblock commented Oct 25, 2024

There's a failed job in https://github.com/opensearch-project/opensearch-py/actions/runs/11502167192/job/32016500189

2024-10-24T15:12:19.9391709Z 
2024-10-24T15:12:19.9391933Z =================================== FAILURES ===================================
2024-10-24T15:12:19.9393640Z _ test_rest_api_spec[OpenSearch-main/rest-api-spec/src/main/resources/rest-api-spec/test/indices/get_settings/40_number_of_routing_shards[0]] _
2024-10-24T15:12:19.9394521Z 
2024-10-24T15:12:19.9394642Z test_spec = {}
2024-10-24T15:12:19.9395268Z sync_runner = <test_opensearchpy.test_server.test_rest_api_spec.YamlRunner object at 0x7f85404d49a0>
2024-10-24T15:12:19.9395882Z 
2024-10-24T15:12:19.9396210Z     @pytest.mark.parametrize("test_spec", YAML_TEST_SPECS)  # type: ignore
2024-10-24T15:12:19.9397624Z     def test_rest_api_spec(test_spec: Any, sync_runner: Any) -> None:
2024-10-24T15:12:19.9398377Z         if test_spec.get("skip", False):
2024-10-24T15:12:19.9399065Z             pytest.skip("Manually skipped in 'SKIP_TESTS'")
2024-10-24T15:12:19.9399710Z         sync_runner.use_spec(test_spec)
2024-10-24T15:12:19.9400211Z >       sync_runner.run()
2024-10-24T15:12:19.9400486Z 
2024-10-24T15:12:19.9400768Z test_opensearchpy/test_server/test_rest_api_spec.py:502: 
2024-10-24T15:12:19.9401602Z _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
2024-10-24T15:12:19.9402498Z test_opensearchpy/test_server/test_rest_api_spec.py:158: in run
2024-10-24T15:12:19.9403229Z     self.run_code(self._run_code)
2024-10-24T15:12:19.9403977Z test_opensearchpy/test_server/test_rest_api_spec.py:173: in run_code
2024-10-24T15:12:19.9404834Z     getattr(self, "run_" + action_type)(action)
2024-10-24T15:12:19.9405333Z test_opensearchpy/test_server/test_rest_api_spec.py:325: in run_match
2024-10-24T15:12:19.9405789Z     value = self._lookup(path)
2024-10-24T15:12:19.9406174Z _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
2024-10-24T15:12:19.9406493Z 
2024-10-24T15:12:19.9406820Z self = <test_opensearchpy.test_server.test_rest_api_spec.YamlRunner object at 0x7f85404d49a0>
2024-10-24T15:12:19.9407819Z path = 'test-index1.settings.index\x01number_of_routing_shards'
2024-10-24T15:12:19.9408164Z 
2024-10-24T15:12:19.9408327Z     def _lookup(self, path: str) -> Any:
2024-10-24T15:12:19.9408732Z         # fetch the possibly nested value from last_response
2024-10-24T15:12:19.9409145Z         value: Any = self.last_response
2024-10-24T15:12:19.9409469Z         if path == "$body":
2024-10-24T15:12:19.9409736Z             return value
2024-10-24T15:12:19.9410028Z         path = path.replace(r"\.", "\1")
2024-10-24T15:12:19.9410344Z         step: Any
2024-10-24T15:12:19.9410599Z         for step in path.split("."):
2024-10-24T15:12:19.9410910Z             if not step:
2024-10-24T15:12:19.9411159Z                 continue
2024-10-24T15:12:19.9411441Z             step = step.replace("\1", ".")
2024-10-24T15:12:19.9411789Z             step = self._resolve(step)
2024-10-24T15:12:19.9412086Z     
2024-10-24T15:12:19.9412281Z             if (
2024-10-24T15:12:19.9412543Z                 isinstance(step, string_types)
2024-10-24T15:12:19.9412892Z                 and step.isdigit()
2024-10-24T15:12:19.9413216Z                 and isinstance(value, list)
2024-10-24T15:12:19.9413538Z             ):
2024-10-24T15:12:19.9413769Z                 step = int(step)
2024-10-24T15:12:19.9414086Z                 assert isinstance(value, list)
2024-10-24T15:12:19.9414439Z                 assert len(value) > step
2024-10-24T15:12:19.9414782Z             elif step == "_arbitrary_key_":
2024-10-24T15:12:19.9415140Z                 return list(value.keys())[0]
2024-10-24T15:12:19.9415461Z             else:
2024-10-24T15:12:19.9415703Z >               assert step in value
2024-10-24T15:12:19.9416860Z E               AssertionError: assert 'index.number_of_routing_shards' in {'index.creation_date': '1729782656764', 'index.number_of_replicas': '1', 'index.number_of_shards': '2', 'index.provided_name': 'test-index1', ...}
2024-10-24T15:12:19.9417709Z 
2024-10-24T15:12:19.9417944Z test_opensearchpy/test_server/test_rest_api_spec.py:403: AssertionError
2024-10-24T15:12:19.9418561Z ----------------------------- Captured stdout call -----------------------------
2024-10-24T15:12:19.9418975Z ========== setup ==========
2024-10-24T15:12:19.9419528Z skip {'version': ' - 2.99.99', 'reason': 'introduced in 3.0.0'}
2024-10-24T15:12:19.9420462Z do {'indices.create': {'body': {'settings': {'index': {'number_of_routing_shards': 4, 'number_of_shards': 2, 'number_of_replicas': 1}}}, 'index': 'test-index'}}
2024-10-24T15:12:19.9421575Z do {'indices.create': {'body': {'settings': {'index': {'number_of_shards': 2, 'number_of_replicas': 1}}}, 'index': 'test-index1'}}
2024-10-24T15:12:19.9422194Z ========== test ==========
2024-10-24T15:12:19.9422606Z skip {'version': ' - 2.99.99', 'reason': 'introduced in 3.0.0'}
2024-10-24T15:12:19.9423206Z do {'indices.get_settings': {'flat_settings': True, 'index': 'test-index'}}
2024-10-24T15:12:19.9423837Z match {'test-index.settings.index\\.number_of_routing_shards': '4'}
2024-10-24T15:12:19.9424479Z do {'indices.get_settings': {'flat_settings': True, 'index': 'test-index1'}}
2024-10-24T15:12:19.9425125Z match {'test-index1.settings.index\\.number_of_routing_shards': None}
2024-10-24T15:12:19.9425731Z ------------------------------ Captured log call -------------------------------
2024-10-24T15:12:19.9426551Z DEBUG    urllib3.connectionpool:connectionpool.py:546 http://localhost:9200 "PUT /test-index HTTP/11" 200 0
2024-10-24T15:12:19.9427485Z INFO     opensearch:base.py:258 PUT http://localhost:9200/test-index [status:200 request:0.041s]
2024-10-24T15:12:19.9428370Z DEBUG    opensearch:base.py:197 > {"settings":{"index":{"number_of_routing_shards":4,"number_of_shards":2,"number_of_replicas":1}}}
2024-10-24T15:12:19.9429319Z DEBUG    opensearch:base.py:199 < {"acknowledged":true,"shards_acknowledged":true,"index":"test-index"}
2024-10-24T15:12:19.9430393Z DEBUG    urllib3.connectionpool:connectionpool.py:546 http://localhost:9200 "PUT /test-index1 HTTP/11" 200 0
2024-10-24T15:12:19.9432037Z INFO     opensearch:base.py:258 PUT http://localhost:9200/test-index1 [status:200 request:0.035s]
2024-10-24T15:12:19.9432813Z DEBUG    opensearch:base.py:197 > {"settings":{"index":{"number_of_shards":2,"number_of_replicas":1}}}
2024-10-24T15:12:19.9433686Z DEBUG    opensearch:base.py:199 < {"acknowledged":true,"shards_acknowledged":true,"index":"test-index1"}
2024-10-24T15:12:19.9434743Z DEBUG    urllib3.connectionpool:connectionpool.py:546 http://localhost:9200 "GET /test-index/_settings?flat_settings=true HTTP/11" 200 0
2024-10-24T15:12:19.9435883Z INFO     opensearch:base.py:258 GET http://localhost:9200/test-index/_settings?flat_settings=true [status:200 request:0.001s]
2024-10-24T15:12:19.9436552Z DEBUG    opensearch:base.py:197 > None
2024-10-24T15:12:19.9438199Z DEBUG    opensearch:base.py:199 < {"test-index":{"settings":{"index.creation_date":"1729782656722","index.number_of_replicas":"1","index.number_of_routing_shards":"4","index.number_of_shards":"2","index.provided_name":"test-index","index.replication.type":"DOCUMENT","index.uuid":"coUxAE_KRVmk-VluSr7now","index.version.created":"137217827"}}}
2024-10-24T15:12:19.9440113Z DEBUG    urllib3.connectionpool:connectionpool.py:546 http://localhost:9200 "GET /test-index1/_settings?flat_settings=true HTTP/11" 200 0
2024-10-24T15:12:19.9441262Z INFO     opensearch:base.py:258 GET http://localhost:9200/test-index1/_settings?flat_settings=true [status:200 request:0.001s]
2024-10-24T15:12:19.9441928Z DEBUG    opensearch:base.py:197 > None
2024-10-24T15:12:19.9443426Z DEBUG    opensearch:base.py:199 < {"test-index1":{"settings":{"index.creation_date":"1729782656764","index.number_of_replicas":"1","index.number_of_shards":"2","index.provided_name":"test-index1","index.replication.type":"DOCUMENT","index.uuid":"vBmoOSCzQWWr0m21wuj1nA","index.version.created":"137217827"}}}
2024-10-24T15:12:19.9444901Z ---------------------------- Captured log teardown -----------------------------
2024-10-24T15:12:19.9445622Z DEBUG    urllib3.connectionpool:connectionpool.py:546 http://localhost:9200 "GET /_snapshot/_all HTTP/11" 200 0
2024-10-24T15:12:19.9446474Z INFO     opensearch:base.py:258 GET http://localhost:9200/_snapshot/_all [status:200 request:0.001s]
2024-10-24T15:12:19.9447043Z DEBUG    opensearch:base.py:197 > None
2024-10-24T15:12:19.9447554Z DEBUG    opensearch:base.py:199 < {}
2024-10-24T15:12:19.9448388Z DEBUG    urllib3.connectionpool:connectionpool.py:546 http://localhost:9200 "DELETE /*,-.ds-ilm-history-*?expand_wildcards=all HTTP/11" 200 0
2024-10-24T15:12:19.9449559Z INFO     opensearch:base.py:258 DELETE http://localhost:9200/*,-.ds-ilm-history-*?expand_wildcards=all [status:200 request:0.014s]
2024-10-24T15:12:19.9450245Z DEBUG    opensearch:base.py:197 > None
2024-10-24T15:12:19.9450634Z DEBUG    opensearch:base.py:199 < {"acknowledged":true}
2024-10-24T15:12:19.9451312Z DEBUG    urllib3.connectionpool:connectionpool.py:546 http://localhost:9200 "DELETE /_template/* HTTP/11" 200 0
2024-10-24T15:12:19.9452160Z INFO     opensearch:base.py:258 DELETE http://localhost:9200/_template/* [status:200 request:0.001s]
2024-10-24T15:12:19.9452731Z DEBUG    opensearch:base.py:197 > None
2024-10-24T15:12:19.9453113Z DEBUG    opensearch:base.py:199 < {"acknowledged":true}
2024-10-24T15:12:19.9453813Z DEBUG    urllib3.connectionpool:connectionpool.py:546 http://localhost:9200 "DELETE /_index_template/* HTTP/11" 200 0
2024-10-24T15:12:19.9454708Z INFO     opensearch:base.py:258 DELETE http://localhost:9200/_index_template/* [status:200 request:0.001s]
2024-10-24T15:12:19.9455296Z DEBUG    opensearch:base.py:197 > None
2024-10-24T15:12:19.9455681Z DEBUG    opensearch:base.py:199 < {"acknowledged":true}
2024-10-24T15:12:19.9456393Z DEBUG    urllib3.connectionpool:connectionpool.py:546 http://localhost:9200 "DELETE /_component_template/* HTTP/11" 200 0
2024-10-24T15:12:19.9457325Z INFO     opensearch:base.py:258 DELETE http://localhost:9200/_component_template/* [status:200 request:0.001s]
2024-10-24T15:12:19.9457934Z DEBUG    opensearch:base.py:197 > None
2024-10-24T15:12:19.9458474Z DEBUG    opensearch:base.py:199 < {"acknowledged":true}
2024-10-24T15:12:19.9459168Z DEBUG    urllib3.connectionpool:connectionpool.py:546 http://localhost:9200 "GET /_cluster/settings HTTP/11" 200 0
2024-10-24T15:12:19.9460050Z INFO     opensearch:base.py:258 GET http://localhost:9200/_cluster/settings [status:200 request:0.001s]
2024-10-24T15:12:19.9460620Z DEBUG    opensearch:base.py:197 > None
2024-10-24T15:12:19.9461051Z DEBUG    opensearch:base.py:199 < {"persistent":{},"transient":{}}
2024-10-24T15:12:19.9461817Z DEBUG    urllib3.connectionpool:connectionpool.py:546 http://localhost:9200 "GET /_cluster/pending_tasks HTTP/11" 200 0
2024-10-24T15:12:19.9462736Z INFO     opensearch:base.py:258 GET http://localhost:9200/_cluster/pending_tasks [status:200 request:0.001s]
2024-10-24T15:12:19.9463335Z DEBUG    opensearch:base.py:197 > None
2024-10-24T15:12:19.9463686Z DEBUG    opensearch:base.py:199 < {"tasks":[]}
2024-10-24T15:12:19.9464464Z - generated xml file: /home/runner/work/opensearch-py/opensearch-py/opensearch-py/junit/opensearch-py-junit.xml -
2024-10-24T15:12:19.9465033Z 
2024-10-24T15:12:19.9465290Z ---------- coverage: platform linux, python 3.10.12-final-0 ----------
2024-10-24T15:12:19.9466172Z Coverage XML written to file /home/runner/work/opensearch-py/opensearch-py/opensearch-py/junit/opensearch-py-codecov.xml
2024-10-24T15:12:19.9466769Z 
2024-10-24T15:12:19.9466956Z =========================== short test summary info ============================
2024-10-24T15:12:19.9469066Z FAILED test_opensearchpy/test_server/test_rest_api_spec.py::test_rest_api_spec[OpenSearch-main/rest-api-spec/src/main/resources/rest-api-spec/test/indices/get_settings/40_number_of_routing_shards[0]] - AssertionError: assert 'index.number_of_routing_shards' in {'index.creation_date': '1729782656764', 'index.number_of_replicas': '1', 'index.number_of_shards': '2', 'index.provided_name': 'test-index1', ...}
2024-10-24T15:12:19.9471186Z =========== 1 failed, 1313 passed, 336 skipped in 128.46s (0:02:08) ============
2024-10-24T15:12:20.1775416Z ##[error]Process completed with exit code 1.
2024-10-24T15:12:20.1857746Z Post job cleanup.
2024-10-24T15:12:20.2812294Z [command]/usr/bin/git version
2024-10-24T15:12:20.2848469Z git version 2.47.0

@gaobinlong
Copy link
Collaborator Author

@dblock The failure is caused by this line which assert that the key exists in the map, but the failed test case just tests that the key doesn't exists(if number_of_routing_shards is not set explicitly when creating index, then get index settings API will not return that setting):

# do not show `index.number_of_routing_shards` if it was not explicitly set when creating

, so how about remove the assertion here and set value to None if the key doesn't exist?

                #assert step in value
            if step in value:
                value = value[step]
            else:
                value = None

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants