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

[Extensions] Fix timeout error when adding a document to an index with extension running #6275

Merged
merged 4 commits into from
Feb 13, 2023

Conversation

ryanbogan
Copy link
Member

Signed-off-by: Ryan Bogan [email protected]

Description

Fixes a timeout exception that would occur when attempting to add a document to an index while an extension is running.

Issues Resolved

#5999

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

github-actions bot commented Feb 9, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.snapshots.DedicatedClusterSnapshotRestoreIT.testIndexDeletionDuringSnapshotCreationInQueue

@codecov-commenter
Copy link

codecov-commenter commented Feb 9, 2023

Codecov Report

Merging #6275 (0baf855) into main (dfe5d2b) will increase coverage by 0.15%.
The diff coverage is 0.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff              @@
##               main    #6275      +/-   ##
============================================
+ Coverage     70.62%   70.77%   +0.15%     
- Complexity    58786    58891     +105     
============================================
  Files          4789     4789              
  Lines        281788   281789       +1     
  Branches      40669    40670       +1     
============================================
+ Hits         199007   199437     +430     
+ Misses        66412    65950     -462     
- Partials      16369    16402      +33     
Impacted Files Coverage Δ
...a/org/opensearch/extensions/ExtensionsManager.java 47.08% <0.00%> (-3.47%) ⬇️
...rch/test/junit/listeners/ReproduceInfoPrinter.java 9.52% <0.00%> (-68.26%) ⬇️
...ava/org/opensearch/action/NoSuchNodeException.java 0.00% <0.00%> (-50.00%) ⬇️
...regations/metrics/AbstractHyperLogLogPlusPlus.java 51.72% <0.00%> (-44.83%) ⬇️
...ensearch/discovery/InitializeExtensionRequest.java 0.00% <0.00%> (-40.00%) ⬇️
...cluster/coordination/PublishClusterStateStats.java 33.33% <0.00%> (-37.51%) ⬇️
...r/src/main/java/org/opensearch/http/HttpUtils.java 33.33% <0.00%> (-33.34%) ⬇️
...search/aggregations/pipeline/HoltWintersModel.java 21.47% <0.00%> (-30.88%) ⬇️
...luster/awarenesshealth/ClusterAwarenessHealth.java 35.18% <0.00%> (-27.78%) ⬇️
...a/org/opensearch/index/mapper/MapperException.java 75.00% <0.00%> (-25.00%) ⬇️
... and 483 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

github-actions bot commented Feb 9, 2023

Gradle Check (Jenkins) Run Completed with:

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

@github-actions
Copy link
Contributor

github-actions bot commented Feb 9, 2023

Gradle Check (Jenkins) Run Completed with:

Copy link
Member

@dbwiddis dbwiddis left a comment

Choose a reason for hiding this comment

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

Approved 'cause it works but suggest adding an else. LGTM!

if (e.getCause() instanceof TimeoutException) {
logger.info("No response from extension to request.");
}
if (e.getCause() instanceof RuntimeException) {
Copy link
Member

Choose a reason for hiding this comment

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

curious why this isn't an else if like line 512.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure - I should fix that

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually I think it's intentional, because no exception handling happens in the first if statement: there's only an additional logging statement. The other if/else ifs actually handle the exception.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see. A timeout is a runtime exception so we log and throw.

@ryanbogan ryanbogan added the backport 2.x Backport to 2.x branch label Feb 13, 2023
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.indices.replication.SegmentReplicationStatsIT.testSegmentReplicationStatsResponse

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@owaiskazi19 owaiskazi19 merged commit 0eee9a9 into opensearch-project:main Feb 13, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Feb 13, 2023
…h extension running (#6275)

* Fix timeout error when adding a document to an index with extension running

Signed-off-by: Ryan Bogan <[email protected]>

* Add CHANGELOG entry

Signed-off-by: Ryan Bogan <[email protected]>

---------

Signed-off-by: Ryan Bogan <[email protected]>
(cherry picked from commit 0eee9a9)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
saratvemulapalli pushed a commit that referenced this pull request Feb 15, 2023
…h extension running (#6275) (#6307)

* Fix timeout error when adding a document to an index with extension running

(cherry picked from commit 0eee9a9)

Signed-off-by: Ryan Bogan <[email protected]>
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.

4 participants