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: v2 - add worker mode #6739

Merged
merged 5 commits into from Mar 21, 2024
Merged

feat: v2 - add worker mode #6739

merged 5 commits into from Mar 21, 2024

Conversation

srindom
Copy link
Collaborator

@srindom srindom commented Mar 19, 2024

What

  • Adds support for starting a Medusa process with a worker mode.
  • The worker modes supported are "shared", "worker", "server"
  • In "worker" mode, API routes are not registered and modules that need to run workers (e.g., event bus redis) can use the flag to conditionally start workers.
  • In "server" mode, API routes are registered and workers are not started.
  • In "shared" mode, API routes are registered and workers are started. This is great for development.

@srindom srindom requested review from a team as code owners March 19, 2024 13:18
Copy link

changeset-bot bot commented Mar 19, 2024

🦋 Changeset detected

Latest commit: 7819a5a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@medusajs/medusa Patch
@medusajs/event-bus-redis Patch
@medusajs/modules-sdk Patch
@medusajs/types Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Mar 19, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
medusa-dashboard ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 21, 2024 0:35am
3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
api-reference ⬜️ Ignored (Inspect) Mar 21, 2024 0:35am
docs-ui ⬜️ Ignored (Inspect) Visit Preview Mar 21, 2024 0:35am
medusa-docs ⬜️ Ignored (Inspect) Visit Preview Mar 21, 2024 0:35am

@srindom
Copy link
Collaborator Author

srindom commented Mar 19, 2024

/snapshot-this

@srindom
Copy link
Collaborator Author

srindom commented Mar 19, 2024

@carlos-r-l-rodrigues, @adrien2p - I will need some help with conditionally starting workflow-engine-redis workers too.

@srindom
Copy link
Collaborator Author

srindom commented Mar 19, 2024

/snapshot-this

Copy link
Member

@adrien2p adrien2p left a comment

Choose a reason for hiding this comment

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

Finally !! 🎉 we can start subscribers only 🔥

@adrien2p
Copy link
Member

It can be an option of the module right? And you can access the options as the second argument of the constructor too

@adrien2p
Copy link
Member

It will free the node event loop a lot when handling subscribers separately from the api

@srindom
Copy link
Collaborator Author

srindom commented Mar 19, 2024

It can be an option of the module right? And you can access the options as the second argument of the constructor too

My thinking on this is that it makes more sense to define it at the MedusaApp-level and not the module-level. This gives a more standardized way for modules to do conditional stuff depending on the worker mode.

That said we could have a priority stack for this:

  1. Worker mode in module confg
  2. Worker mode in project config
  3. Worker mode in environment variables
  4. Default "shared"

--

Think it would be great if the mode always gets passed to the module as moduleDeclaration instead of an option. Again to standardize usage.

We could potentially do it like this in medusa-config.js

// An eventbus only worker
{
  resolve: "@medusajs/event-bus-redis",
  options: { redisUrl: REDIS_URL },
  worker_mode: "worker"
},
{
  resolve: "@medusajs/workflow-engine-redis",
  options: { redis: { url: REDIS_URL } },
  worker_mode: "server"
},

// A workflow engine only worker
{
  resolve: "@medusajs/event-bus-redis",
  options: { redisUrl: REDIS_URL },
  worker_mode: "server"
},
{
  resolve: "@medusajs/workflow-engine-redis",
  options: { redis: { url: REDIS_URL } },
  worker_mode: "worker"
},

@adrien2p
Copy link
Member

Yes definitely, i like the priority order, but the modules does not have access to the config as it is not injected and they do not require it as a dependency. But it can be changed and when using medusa app it can be added to the dependencies and getting injected in the module, so we can apply the ordering as you suggested

@srindom srindom requested review from a team as code owners March 21, 2024 09:37
@srindom srindom changed the base branch from fix/v2-subscribers to develop March 21, 2024 09:38
@srindom
Copy link
Collaborator Author

srindom commented Mar 21, 2024

@olivermrbl, @adrien2p - can we get this in as is for now and then come back to improvements later on? :)

Copy link
Member

@adrien2p adrien2p left a comment

Choose a reason for hiding this comment

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

yes we can @srindom if you want you can create a linear ticket to come back to it later with non automatic closing :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants