-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Remove outdated assertion from #118214 #121435
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
Conversation
Asserting that we definitely saw the "received a single result" flag and can now deal with null responses, isn't applicable after a few recent fixes. New requests are sent out before responses are fully processed to keep data nodes in a tighter loop (as well as other relaxed ordering relative to when this assertion was added) so the flag is not guaranteed to show up as true for lower numbers of shard requests any longer. Lets just remove it, it was always best effort and accidental that this worked for the numbers the test randomizes over.
|
Pinging @elastic/es-search-foundations (Team:Search Foundations) |
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!
|
Just to be sure we are not forgetting, can you add "closes #118214" to the description? |
|
Thanks Matteo! |
Asserting that we definitely saw the "received a single result" flag and can now deal with null responses, isn't applicable after a few recent fixes. New requests are sent out before responses are fully processed to keep data nodes in a tighter loop (as well as other relaxed ordering relative to when this assertion was added) so the flag is not guaranteed to show up as true for lower numbers of shard requests any longer. Lets just remove it, it was always best effort and accidental that this worked for the numbers the test randomizes over.
💔 Backport failed
You can use sqren/backport to manually backport by running |
Asserting that we definitely saw the "received a single result" flag and can now deal with null responses, isn't applicable after a few recent fixes. New requests are sent out before responses are fully processed to keep data nodes in a tighter loop (as well as other relaxed ordering relative to when this assertion was added) so the flag is not guaranteed to show up as true for lower numbers of shard requests any longer. Lets just remove it, it was always best effort and accidental that this worked for the numbers the test randomizes over. Co-authored-by: Dimitris Rempapis <[email protected]>
Asserting that we definitely saw the "received a single result" flag and can now deal with null responses, isn't applicable after a few recent fixes. New requests are sent out before responses are fully processed to keep data nodes in a tighter loop (as well as other relaxed ordering relative to when this assertion was added) so the flag is not guaranteed to show up as true for lower numbers of shard requests any longer.
Lets just remove it, it was always best effort and accidental that this worked for the numbers the test randomizes over.