-
Notifications
You must be signed in to change notification settings - Fork 313
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
Remove buggy indices.exists() calls from code #1580
Conversation
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's ok by me! What a wild ride. Probably worth a comment about why using get
instead of exists
and maybe even an assertion that you are on the un-fixed version of the client. Once you upgrade I think you want to revert this commit, right?
@elasticmachine run rally/rally-tracks-compat |
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. Thanks so much for chasing this down.
Another point in favor of prioritizing the client upgrade.
Recently we've had a few errors surface in Rally executions when indices/data-streams are deleted. The errors would be reported as
connection timed out
for theDELETE /<index>
call, but when inspected (via a packet capture) the server was shown to have responded correctly and in a timely fashion to Rally.It turns out that in our version of
elasticsearch-py
,indices.exists()
calls were handled via GET rather than HEAD. It's unclear to me why this causes an issue, but any of the following fixes the issue when targeting very large indices with thedelete-index
operation:only-if-exists
tofalse
in the operation (to avoid theexists()
call)exists()
call to aget()
call.I'm led to believe the bytes from the body are left in the event loop and not dequeued properly by the client, hence blocking somehow the response from the very next transaction.
This PR changes from using the
exists()
method to using theget()
method to determine an index's existence.A test is updated to show the difference in behavior under-the-hood.