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: Many retries when an LRS returns a 409 error #298

Merged
merged 3 commits into from May 18, 2023

Conversation

bmtcril
Copy link
Contributor

@bmtcril bmtcril commented May 14, 2023

Description: The 409 error code is returned when an LRS determines a statement id already exists in the database, which is a use case we will see a lot with the new idempotent statement ids and upcoming tracking log replay. This change logs and eats that error.

This was introduced here:
https://github.com/openedx/event-routing-backends/pull/290/files

Issue: #295

Merge deadline: ASAP as this has a multiplicative effect for each failing event which (in older versions) is compounded by this issue: #296

Testing instructions:
Currently this is easy to test in a Nutmeg Tutor install given the other issue with the idempotent ids.

Merge checklist:

  • All reviewers approved
  • CI build is green
  • Version bumped
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Commits are squashed

Post merge:

  • Create a tag
  • Check new version is pushed to PyPI after tag-triggered build is
    finished.

Author concerns: This is a draft PR just to get @ziafazal 's eyes on it before I put more work into making the tests work. It may not be the correct way to go so I'd like his input.

The 409 error code is returned when an LRS determines a statement id already exists in the database, which is a use case we will see a lot with the new idempotent statement ids and upcoming tracking log replay. This change logs and eats that error.
The 409 error code is returned when an LRS determines a statement id already exists in the database, which is a use case we will see a lot with the new idempotent statement ids and upcoming tracking log replay. This change logs and eats that error.
Copy link
Contributor

@ziafazal ziafazal left a comment

Choose a reason for hiding this comment

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

@bmtcril thanks for this fix. LGTM.

@bmtcril bmtcril marked this pull request as ready for review May 15, 2023 14:55
@ziafazal ziafazal merged commit 8839a03 into master May 18, 2023
9 checks passed
@ziafazal ziafazal deleted the bmtcril/fix_duplicate_uuid_handling branch May 18, 2023 05:22
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.

None yet

2 participants