-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[Pull-based Ingestion] Introduce ingestion management APIs #17631
base: main
Are you sure you want to change the base?
[Pull-based Ingestion] Introduce ingestion management APIs #17631
Conversation
❌ Gradle check result for dcccf22: 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? |
❌ Gradle check result for 77aa63a: 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? |
77aa63a
to
7402d31
Compare
Signed-off-by: Varun Bharadwaj <[email protected]>
❌ Gradle check result for 7402d31: 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? |
7402d31
to
b7617bf
Compare
Signed-off-by: Varun Bharadwaj <[email protected]>
b7617bf
to
5756fd0
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17631 +/- ##
============================================
- Coverage 72.40% 72.33% -0.07%
- Complexity 65828 65851 +23
============================================
Files 5316 5339 +23
Lines 305294 305905 +611
Branches 44289 44338 +49
============================================
+ Hits 221033 221266 +233
- Misses 66187 66495 +308
- Partials 18074 18144 +70 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
5e33b66
to
371e634
Compare
❌ Gradle check result for 371e634: 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? |
371e634
to
7eccc3c
Compare
❌ Gradle check result for 7eccc3c: 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: Varun Bharadwaj <[email protected]>
7eccc3c
to
060d76c
Compare
❕ Gradle check result for 060d76c: UNSTABLE
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall lgtm
@Override | ||
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { | ||
builder.startObject(); | ||
builder.field(SHARD, shard); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no index?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We skip index from XContent since shard level failures generally follow the format of indexName: {[<shard failures>]}
. So the indexName will be used as a key from the container class holding this failure.
public void onResponse(UpdateIngestionStateResponse updateIngestionStateResponse) { | ||
boolean shardsAcked = updateIngestionStateResponse.isAcknowledged() | ||
&& updateIngestionStateResponse.getTotalShards() > 0 | ||
&& updateIngestionStateResponse.getFailedShards() == 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this condition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are checking if the request was processed successfully on all individual shards in order to set shard level ack to true. If atleast 1 shard failed to explicitly update the state, we set this to false and add the shard in the failure list.
/** | ||
* Holds metadata required for updating ingestion state. | ||
* | ||
* <p> This is for internal use only and will not be exposed to the user. </p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how do we ensure user cannot access this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. Right now, I just create the transport action but do not register it as a REST action in the action module so users cannot call this externally.
if (in.getVersion().onOrAfter(Version.V_3_0_0)) { | ||
ingestionPaused = in.readBoolean(); | ||
} else { | ||
ingestionPaused = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this one? the default value of ingestionPaused is false?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a final variable and hence we have to initialize it explicitly to false. If we don't want to, we can make it non-final.
Description
This PR adds ingestion management APIs - mainly Pause, Resume and GetIngestionState APIs. This PR is a first version to introduce the APIs and subsequent PRs will build on this to support pagination and consumer reset options.
Related Issues
Part one of #17442
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.