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

Reset event checkpoint key property for non sub-page breaks #7638

Merged
merged 3 commits into from Jul 29, 2021

Conversation

xacrimon
Copy link
Contributor

@xacrimon xacrimon commented Jul 22, 2021

This PR fixes #7635 which was introduced in #7266 and is caused by incorrect event skipping logic for resuming from sub-page breaks in the events iterator in Dynamo. It doesn't remedy the incorrect event return amount since that is a designed cutoff to avoid hitting gRPC message limits but it does fix the somewhat rare pagination end bug that the issue also mentions.

@xacrimon xacrimon force-pushed the joel/fix-7635 branch 2 times, most recently from c18c091 to 4c211ad Compare July 26, 2021 12:58
@russjones
Copy link
Contributor

@xacrimon Please backport this to branch/v6.2 as well.

@xacrimon
Copy link
Contributor Author

@russjones will do, need approval

@russjones
Copy link
Contributor

@xacrimon Can you add test coverage?

@xacrimon
Copy link
Contributor Author

@russjones I tried. To break this you need a lot of events and it times out long before that finishes. I had Lisa test it manually too with the frontend.

@xacrimon xacrimon force-pushed the joel/fix-7635 branch 2 times, most recently from 4c211ad to a675389 Compare July 27, 2021 08:07
@russjones
Copy link
Contributor

@xacrimon Was that the local DynamoDB emulator?

@xacrimon
Copy link
Contributor Author

@russjones It was not, nothing in the test infra is set up to work with that atm.

@xacrimon xacrimon force-pushed the joel/fix-7635 branch 2 times, most recently from e376a21 to c129a0f Compare July 29, 2021 16:41
@xacrimon
Copy link
Contributor Author

@russjones Added test coverage, managed to make it work.

Copy link
Contributor

@russjones russjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bot.

@xacrimon xacrimon merged commit e842fbc into master Jul 29, 2021
@xacrimon xacrimon deleted the joel/fix-7635 branch July 29, 2021 17:47
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.

UI Audit log fetching does not return correct amount of events
4 participants