Add capability to stop async query on demand #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
smalyshev
force-pushed
the
partial-result-on-demand
branch
from
b76b0ad to
037564d
Compare
dnhatn
reviewed
Dec 5, 2024
| * This action will stop running async request and collect the results. | ||
| * If the request is already finished, it will do the same thing as the regular async get. | ||
| */ | ||
| public class TransportEsqlAsyncStopAction extends HandledTransportAction<AsyncStopRequest, EsqlQueryResponse> { |
There was a problem hiding this comment.
Can we add a new parameter to GetAsyncResultRequest instead?
There was a problem hiding this comment.
GetAsyncResultRequest is GET, and this is POST. We could add POST entry point to GetAsyncResultRequest but it feels like bolting it on since this code shares nothing with GetAsyncResultRequest. We could also make it GET but it makes me feel bad to have non-repeatable state-modifying GET, it's against all REST rules.
There was a problem hiding this comment.
Another option would be to make POST /_query/async/{id} to do this action (without additional params) and switch on the method, it feels a little bit unobvious but can work too... Though I feel since it doesn't share other params with the GET (unless we want to make it so?) it is cleaner to keep them separate.
There was a problem hiding this comment.
My comment is to avoid introducing yet another action.
There was a problem hiding this comment.
But is it bad in this case? It's a different action that never existed before, so I'd think it may be ok in this case to have separate action class.
There was a problem hiding this comment.
I believe what we need is the capability to retrieve the available result. In the first iteration, we lack the ability to continue execution after retrieving the result; therefore, we stop and fail the query afterward.
smalyshev
force-pushed
the
partial-result-on-demand
branch
from
6fc83f7 to
de1039b
Compare
smalyshev
force-pushed
the
partial-result-on-demand
branch
from
de1039b to
f7e1d9c
Compare
smalyshev
force-pushed
the
partial-result-on-demand
branch
from
afddc5f to
124e070
Compare
smalyshev
force-pushed
the
partial-result-on-demand
branch
from
81f2e81 to
49c768e
Compare
The method in which it was parsed was unused.
Also provide caching support for buildparams provider * Extract BuildParameterExtension public api into interface * Make tests better readable * Fix test flakyness
… Learning jobs (elastic#118549) Since in 9.0 we don't need to support snapshots prior to 7.17, we can simplify the changes made in elastic#81039 and re-introduce a single contant to manage the minimum supported snapshot version.
Fix elastic#117698 by enabling push down of `LIMIT` past `LEFT JOIN`s. There is a subtle point here: our `LOOKUP JOIN` currently _exactly preserves the number of rows from the left hand side_. This is different from SQL, where `LEFT JOIN` will return _at least one row for each row from the left_, but may return multiple rows in case of multiple matches. We, instead, throw multiple matches into multi-values, instead. (C.f. [tests that I'm about to add](https://github.com/elastic/elasticsearch/pull/118471/files#diff-334f3328c5f066a093ed8a5ea4a62cd6bcdb304b660b15763bb4f64d0e87ed7cR365-R369) that demonstrate this.) If we were to change our semantics to match SQL's, we'd have to adjust the pushdown, too.
…18260) * [Connector API] Update yaml tests * Update tests --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Make the dependency checker for query plans take into account binary plans and make sure that fields required from the left hand side are actually obtained from there (and analogously for the right).
This PR adds support for ST_EXTENT_AGG aggregation, i.e., computing a bounding box over a set of points/shapes (Cartesian or geo). Note the difference between this aggregation and the already implemented scalar function ST_EXTENT. This isn't a very efficient implementation, and future PRs will attempt to read these extents directly from the doc values. We currently always use longitude wrapping, i.e., we may wrap around the dateline for a smaller bounding box. Future PRs will let the user control this behavior. Fixes elastic#104659.
… testPlanSanityCheckWithBinaryPlans elastic#118656
* Introduce TranslationAware interface * Serialize query builder * Fix EsqlNodeSubclassTests * Add javadoc * Address review comments * Revert changes on making constructors private
Add a csv dataset and tests for `LOOKUP JOIN` where the join keys are not unique. In particular, add tests that include MVs and nulls to see how `LOOKUP JOIN` treats these.
…ing (elastic#118624) We need to load the two fields from the same response. Otherwise, we can have a sort of race where we load "endpoints" from pre-8.15 as empty and then load "models" from a post-8.15 node also empty, resulting in an empty list because we took the wrong info from either response. closes elastic#118163
…o 1b51ff6 (elastic#117902) Co-authored-by: elastic-renovate-prod[bot] <174716857+elastic-renovate-prod[bot]@users.noreply.github.com> Co-authored-by: Rene Groeschke <rene@elastic.co>
- Add link to Python notebook - Fix heading level
This adds support for running the bucket function over a date nanos field. Code wise, this just delegates to DateTrunc, which already supports date nanos, so most of the PR is just tests and the auto-generated docs. Resolves elastic#118031
…rchStatusException (elastic#118618) * Replace ElasticsearchTimeoutException with ElasticsearchTimeoutException with 408 status to avoid cuasing 503s
* WIP: Tool to find all public caller from a starting list of (JDK) methods. * Add public-callers-finder tool, extract common stuff to common module * Adjustments to visibility/functions and classes and modules to print out * Spotless * Missing gradle configuration * Add details in README as requested in PR * Update ASM version * Including protected methods
This prevents two tests in `grok` and `dissect` suites - `overwriteName` and `overwriteNameWhere` and one in the `stats` suite - `byStringAndLongWithAlias` - to run against pre-8.13.0 versions. Reason being that coordinators prior to that version can generate invalid node plans, that'd fail (verification) on 8.18+ nodes.
Disabled the NoImds test on AWS EC2 instance where it fails because the AWS metadata are available, which is not expected by this test.
elastic#118058) Previously, Kibana was authorizing (and granting application privileges) to create reports, simply based on the `reporting_user` role name. This PR makes these application privileges explicitly granted to the `reporting_user` role.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WIP!