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

Add update settings allowlist for searchable snapshot #5907

Merged
merged 5 commits into from
Jan 20, 2023
Merged

Add update settings allowlist for searchable snapshot #5907

merged 5 commits into from
Jan 20, 2023

Conversation

adnapibar
Copy link
Contributor

Signed-off-by: Rabi Panda adnapibar@gmail.com

Description

Allow search related settings to be updated for searchable snapshots.

Issues Resolved

Resolves #4966

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
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

codecov-commenter commented Jan 19, 2023

Codecov Report

Merging #5907 (79a211e) into main (f7dc32e) will decrease coverage by 0.05%.
The diff coverage is 9.52%.

@@             Coverage Diff              @@
##               main    #5907      +/-   ##
============================================
- Coverage     70.85%   70.80%   -0.05%     
- Complexity    58711    58754      +43     
============================================
  Files          4770     4770              
  Lines        280744   280765      +21     
  Branches      40539    40550      +11     
============================================
- Hits         198911   198798     -113     
- Misses        65455    65611     +156     
+ Partials      16378    16356      -22     
Impacted Files Coverage Δ
...es/settings/put/TransportUpdateSettingsAction.java 50.00% <9.52%> (-25.87%) ⬇️
...g/opensearch/index/analysis/CharFilterFactory.java 0.00% <0.00%> (-100.00%) ⬇️
...adonly/AddIndexBlockClusterStateUpdateRequest.java 0.00% <0.00%> (-75.00%) ⬇️
.../java/org/opensearch/node/NodeClosedException.java 50.00% <0.00%> (-50.00%) ⬇️
...ch/index/snapshots/IndexShardRestoreException.java 0.00% <0.00%> (-42.86%) ⬇️
...pensearch/indices/breaker/CircuitBreakerStats.java 27.77% <0.00%> (-41.67%) ⬇️
...readonly/TransportVerifyShardIndexBlockAction.java 9.75% <0.00%> (-41.47%) ⬇️
...indices/readonly/TransportAddIndexBlockAction.java 20.68% <0.00%> (-37.94%) ⬇️
...java/org/opensearch/threadpool/ThreadPoolInfo.java 56.25% <0.00%> (-37.50%) ⬇️
...arch/search/aggregations/pipeline/LinearModel.java 23.07% <0.00%> (-34.62%) ⬇️
... and 453 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Allow search related settings to be updated for searchable snapshots.

Signed-off-by: Rabi Panda <adnapibar@gmail.com>
Signed-off-by: Rabi Panda <adnapibar@gmail.com>
Signed-off-by: Rabi Panda <adnapibar@gmail.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.indices.replication.SegmentReplicationIT.testMultipleShards
      1 org.opensearch.cluster.routing.allocation.decider.DiskThresholdDeciderIT.testIndexCreateBlockWhenAllNodesExceededHighWatermark

Comment on lines 73 to 83
private final static String[] ALLOWLIST_REMOTE_SNAPSHOT_SETTINGS = {
"index.max_result_window",
"index.max_inner_result_window",
"index.max_rescore_window",
"index.max_docvalue_fields_search",
"index.max_script_fields",
"index.max_terms_count",
"index.max_regex_length",
"index.highlight.max_analyzed_offset" };

private final static String[] ALLOWLIST_REMOTE_SNAPSHOT_SETTINGS_PREFIXES = { "index.search.slowlog" };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick, but for both of these I'd use Set.of(...) to create immutable sets. It'll save you the step of doing Arrays.asList below.

@@ -332,6 +335,13 @@ private void assertIndexSettingChangeBlocked(String index) {
}
}

private void assertUpdateIndexSettingsAllowList(String index) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a case for a request that contains both an allowed setting and a disallowed setting?

Signed-off-by: Rabi Panda <adnapibar@gmail.com>
CHANGELOG.md Outdated
@@ -18,6 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Allow mmap to use new JDK-19 preview APIs in Apache Lucene 9.4+ ([#5151](https://github.com/opensearch-project/OpenSearch/pull/5151))
- Add support for ppc64le architecture ([#5459](https://github.com/opensearch-project/OpenSearch/pull/5459))
- Add support to disallow search request with preference parameter with strict weighted shard routing([#5874](https://github.com/opensearch-project/OpenSearch/pull/5874))
- Add update settings allowlist for searchable snapshot ([#5907](https://github.com/opensearch-project/OpenSearch/pull/5907))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to go in the [Unreleased 2.x] section since it will be released on the 2.x line.

Signed-off-by: Rabi Panda <adnapibar@gmail.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.cluster.routing.allocation.decider.DiskThresholdDeciderIT.testIndexCreateBlockWithAReadOnlyBlock

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.search.SearchWeightedRoutingIT.testSearchAggregationWithNetworkDisruption_FailOpenEnabled
      1 org.opensearch.cluster.routing.allocation.decider.DiskThresholdDeciderIT.testIndexCreateBlockIsRemovedWhenAnyNodesNotExceedHighWatermark

@andrross
Copy link
Member

New issues for the flaky tests:

#5957
#5956

@andrross andrross merged commit 011ff63 into opensearch-project:main Jan 20, 2023
@andrross andrross added the backport 2.x Backport to 2.x branch label Jan 20, 2023
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

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

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
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-5907-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 011ff6353237fd3c23a5be1487dff67e8374a84d
# Push it to GitHub
git push --set-upstream origin backport/backport-5907-to-2.x
# Go back to the original working tree
popd
# 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-5907-to-2.x.

@adnapibar adnapibar added backport 2.x Backport to 2.x branch and removed backport 2.x Backport to 2.x branch labels Jan 21, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jan 21, 2023
* Add update settings allowlist for searchable snapshot

Allow search related settings to be updated for searchable snapshots.

Signed-off-by: Rabi Panda <adnapibar@gmail.com>

* Update Changelog

Signed-off-by: Rabi Panda <adnapibar@gmail.com>

* Add feature flag check

Signed-off-by: Rabi Panda <adnapibar@gmail.com>

* Use set instead of string arrays for allowed list

Signed-off-by: Rabi Panda <adnapibar@gmail.com>

* Update Changelog

Signed-off-by: Rabi Panda <adnapibar@gmail.com>

Signed-off-by: Rabi Panda <adnapibar@gmail.com>
(cherry picked from commit 011ff63)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
kotwanikunal pushed a commit that referenced this pull request Jan 22, 2023
* Add update settings allowlist for searchable snapshot

Allow search related settings to be updated for searchable snapshots.

Signed-off-by: Rabi Panda <adnapibar@gmail.com>

* Update Changelog

Signed-off-by: Rabi Panda <adnapibar@gmail.com>

* Add feature flag check

Signed-off-by: Rabi Panda <adnapibar@gmail.com>

* Use set instead of string arrays for allowed list

Signed-off-by: Rabi Panda <adnapibar@gmail.com>

* Update Changelog

Signed-off-by: Rabi Panda <adnapibar@gmail.com>

Signed-off-by: Rabi Panda <adnapibar@gmail.com>
(cherry picked from commit 011ff63)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Signed-off-by: Rabi Panda <adnapibar@gmail.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
kotwanikunal pushed a commit that referenced this pull request Jan 25, 2023
* Add update settings allowlist for searchable snapshot

Allow search related settings to be updated for searchable snapshots.

Signed-off-by: Rabi Panda <adnapibar@gmail.com>

* Update Changelog

Signed-off-by: Rabi Panda <adnapibar@gmail.com>

* Add feature flag check

Signed-off-by: Rabi Panda <adnapibar@gmail.com>

* Use set instead of string arrays for allowed list

Signed-off-by: Rabi Panda <adnapibar@gmail.com>

* Update Changelog

Signed-off-by: Rabi Panda <adnapibar@gmail.com>

Signed-off-by: Rabi Panda <adnapibar@gmail.com>
(cherry picked from commit 011ff63)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Signed-off-by: Rabi Panda <adnapibar@gmail.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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.

[Searchable Snapshot] Finalize behavior of settings API
4 participants