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

feat: use a more robust queue system, with retry #54

Closed
wants to merge 18 commits into from

Conversation

wa0x6e
Copy link
Contributor

@wa0x6e wa0x6e commented Apr 25, 2023

Issues

Handle all queue-like events with a true dedicated queue system, instead of using recursive run() function with sleep.

The queue system will allow more robust queue processing, such as dedicated process for each job and retry feature (fix #53), as well as easier testing.

Changes

  • Moving the basic queue function in replay.ts and event.ts to a bull worker.
  • Add retry strategy for worker
  • Add dedicated worker for HTTP notifications
  • Add dedicated worker for Discord notifications
  • Add dedicated worker for push notifications

Refactoring has been kept to the minimum, as workers are just used to trigger the function in the existing files, instead of moving the logic inside those workers file.

Todo

  • Handle all HTTP errors, and decide which kind should trigger a retry
  • Handle all Push errors, and decide which kind should trigger a retry
  • Handle all Discord errors, and decide which kind should trigger a retry

Notes

This PR is based on #60 and #61, and may contains unrelated changes until these are merged

@wa0x6e wa0x6e marked this pull request as ready for review April 26, 2023 19:03
@wa0x6e wa0x6e requested a review from ChaituVR April 26, 2023 19:03
@@ -8,3 +8,4 @@ SERVICE_PUSHER_BEAMS_INSTANCE_ID=
SERVICE_PUSHER_BEAMS_SECRET_KEY=
HUB_URL=
SNAPSHOT_URI=
REDIS_URL=
Copy link
Member

Choose a reason for hiding this comment

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

Hi @wa0x6e It won't work without Redis? Redis is usually expensive, retry can't be in memory or on mysql DB?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of the light queue libraries uses Redis as backend, it's built for that.

Redis is not used to store the retry, but is used as storage solution for the whole queue system.

As for the redis instance, only a small instance will be required, as data will be cleared as queue is cleared, only data for failed/waiting for retry jobs will be stored.
Another repo (envelop) is also using the queue system, we could share the same redis instance for all repo using a queue system

Copy link
Member

Choose a reason for hiding this comment

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

For that we need @bonustrack 's approval 😢 because he denied Redis a few times as far as I remember

@bonustrack what you think? we could use Redis here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, I've seen REDIS_URL in a few places, so I just assumed that it was common.

@wa0x6e wa0x6e closed this Apr 29, 2024
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.

feat: add retries
2 participants