Skip to content
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

fix: Ensure Flux reads across all shards #20064

Merged
merged 3 commits into from
Nov 17, 2020
Merged

fix: Ensure Flux reads across all shards #20064

merged 3 commits into from
Nov 17, 2020

Conversation

stuartcarnie
Copy link
Contributor

@stuartcarnie stuartcarnie commented Nov 17, 2020

Closes #20035

This PR ensures all elements of the cursors.CursorIterator slice (itrs) are read when reading a multi-shard cursor. It is accompanied with a unit test to replicate the incorrect behaviour.

The root cause is that it is possible a single shard has no data for a given query and this resulted in the read stopping short, as it returned a nil cursor.

Unlike the createCursor function, which finds the next non-nil result from the itrs slice:

for cur == nil && len(row.Query) > 0 {
shard, row.Query = row.Query[0], row.Query[1:]
cur, _ = shard.Next(m.ctx, &m.req)
}

the existing implementation of nextArrayCursor was only checking one element of the itrs slice:

itr, c.itrs = c.itrs[0], c.itrs[1:]
cur, _ = itr.Next(c.ctx, c.req)

If the call to Next returned a nil Cursor, that signalled the end of the read.

This fix ensures itrs is read in a loop for the next non-nil cursor.

@stuartcarnie stuartcarnie changed the title fix: Continue reading until itrs is empty fix: Ensure Flux reads across all shards for ReadFilter requests Nov 17, 2020
@stuartcarnie stuartcarnie changed the title fix: Ensure Flux reads across all shards for ReadFilter requests fix: Ensure Flux reads across all shards Nov 17, 2020
@stuartcarnie
Copy link
Contributor Author

@nathanielc please feel free to add someone from your team for review.

@danxmoran I added you, so you could see what the fix was 👍

@danxmoran danxmoran merged commit 502ee78 into master Nov 17, 2020
@stuartcarnie stuartcarnie deleted the sgc/issues/20035 branch November 17, 2020 19:09
@danxmoran danxmoran removed their request for review February 10, 2021 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flux stops reading results on first shard with no data
3 participants