-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Don't traverse immutable layer if deleteBelowTs > 0 #4204
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.
✅ A review job has been created and sent to the PullRequest network.
@pawanrawal you can click here to see the review status or cancel the code review job.
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.
Reviewed 1 of 1 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @manishrjain)
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.
Have Manish take a look at this just to be sure.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @manishrjain)
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.
Reviewed 1 of 1 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pawanrawal)
posting/list.go, line 105 at r2 (raw file):
func (it *pIterator) init(l *List, afterUid, deleteBelowTs uint64) error { if deleteBelowTs > 0 && deleteBelowTs <= l.minTs {
Please check when it broke. And add that PR which broke it in the description.
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.
}`, ` | ||
{ | ||
"me": [] | ||
}`)) |
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.
I think this format would be better for readability:
}`,
),
)
}) | ||
require.NoError(t, err) | ||
|
||
t.Run("Get list of friends", s.testCase(` |
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.
Let's have s.testCase(
to be on a new line for readability.
S P *
deletions were not working when data was loaded using bulk loader or was in the immutable layer. This PR makes sure we don't iterate over the immutable layer when we have a delete marker. Earlier after doing aS P *
deletion, we were still returning the first uid from the immutable layer.Fixes #4182
This bug was introduced in #3105.
This change is