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

Added job for email link click processing #21845

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

9larsons
Copy link
Contributor

@9larsons 9larsons commented Dec 10, 2024

ref https://linear.app/ghost/issue/ENG-1815/
ref https://linear.app/ghost/issue/ENG-1817/

  • created a job queue entry for processing email click redirect actions

Our existing handling relies on a sequence of DomainEvents, resulting a couple updates by different services. The challenge here is that while we maintain the walls around each particular service, this is creating performance issues for Ghost under heavy load.

This change provides a pathway for us to respond to the redirect lookup as far as possible while encapsulating the actions needed by Ghost's main process within the response lifecycle, such that we do not create additional load on Ghost by accepting a new request before all the processing is done with the previous.

In other words, we have a request queue and gate requests as a means of not overhwleming Ghost. We need to ensure that any processing due to a request is contained in that response lifecycle, even if it means very slight delays in returning the response to the user.

We have a new model here instead of creating and emitting events, we inject the job creation fn into the service package as a cleaner means of creating the job. This allows us to keep the job in the service wrapper (within ghost/core) which is necessary to use knex and other imports like the settings cache.

ref https://linear.app/ghost/issue/ENG-1815/
ref https://linear.app/ghost/issue/ENG-1817/

- created a job queue entry for processing email click redirect actions

Our existing handling relies on a sequence of DomainEvents, resulting a couple updates by different services. The challenge here is that while we maintain the walls around each particular service, this is creating performance issues for Ghost under heavy load.

This change provides a pathway for us to respond to the redirect lookup as far as possible *while* encapsulating the actions needed by Ghost's main process within the response lifecycle, such that we do not create additional load on Ghost by accepting a new request before all the processing is done with the previous.

In other words, we have a request queue and gate requests as a means of not overhwleming Ghost. We need to ensure that any processing due to a request is contained in that response lifecycle, even if it means very slight delays in returning the response to the user.

We have a new model here instead of creating and emitting events, we inject the job creation fn into the service package as a cleaner means of creating the job. This allows us to keep the job in the service wrapper (within ghost/core) which is necessary to use knex and other imports like the settings cache.
@9larsons 9larsons requested a review from ErisDS December 10, 2024 02:56
@9larsons
Copy link
Contributor Author

9larsons commented Dec 10, 2024

Todo:

  • Add support for node events (e.g. member.edited); this is in a separate PR that needs tests
  • Add/update tests
  • Add config for the job that isn't just based on the queue being enabled (AND logic)

Some thoughts:

  • Would it make more sense for these jobs to be in a separate package?
  • Should we split up this job so that we can insert a lastSeenAt job row that is member-uuid-specific, which would be a way to prevent duplicative work? [this would be an optimization, and one that may not be necessary]

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.

1 participant