-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
[Transform] add debug log for configuration index #49484
[Transform] add debug log for configuration index #49484
Conversation
Pinging @elastic/ml-core (:ml/Transform) |
@@ -327,6 +329,7 @@ public void getTransformConfigurationForUpdate( | |||
// use sort to get the last | |||
.addSort("_index", SortOrder.DESC) | |||
.setSize(1) | |||
.setAllowPartialSearchResults(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.
What does it mean "partial search results"? Results from a subset of nodes?
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 means that it will throw if a subset of the shards fail the search.
If true
, the search will return as normal but indicate shard failures.
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.
subset of shards, which might be on different nodes, yes. In the default setting ES returns with 200 but actually might not have all results. This is somewhat "search engine behavior", if you care about consistency you can either ask for getFailedShards()
in the response object or you disallow partial results which will make it throw. I prefer the more explicit behavior.
Note that per default the number of shards is 1 for this config index, therefore it should not make a difference in this case, because it should throw if all shards == 1 shard failed. But I think it's a good setting as the default might change.
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.
Ok, thanks for explanations.
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.
LGTM
7c519fc
to
f7e559b
Compare
run elasticsearch-ci/packaging-sample-matrix |
add debug log for transform creation and disallow partial results for retrieval
add debug log for transform creation and disallow partial results for retrieval