Scale out Medusa into multiple instances: Events are likely duplicated if we are unlucky with timing

[khanh-to-niteco]

Hi Medusa team,

We are using Medusajs 1.17.4 and planning to scale out the app to support the increasing load of our system. But looking into the code of the event-bus.ts, it does not seem confident for us to scale out

We have added await sleep(12000) on line 208 to make it "unlucky" with the timing so we can reproduce the issue consistently.

After adding that line of code, we observed that the same events were executed on multiple instances which could increase load unnecessarily on the instances and could increase the concurrency level on the system.
Even worse if the events trigger an import or export batch jobs, these are resource-intensive workflows in the application.

I know that we cannot achieve exactly once semantic 100% in distributed systems (or very difficult), so in the end, we have to rely on deduplication or idempotency to cover 0.001% of the edge cases. Idempotency also helps to solve the issue of events being dispatched out of order in this case.

But it would be great if the Medusa system has some synchronized control over this process or has any mechanism that this duplicated event executions are kept to the minimal.

[dwene]

Hey @khanh-to-niteco

I added a LockService using redis and the redlock npm package. It allows you to create a mutex lock for a particular key to ensure only a single instance is performing work at the same time.

You could always add that into this enqueuer function to ensure only a single instance is running this at a time.

import { ConfigModule } from "@medusajs/types"
import { Redis } from "ioredis"
import Redlock, { RedlockAbortSignal, Settings } from "redlock"

const initializeRedlock = (redis: Redis) => {
  const redlock = new Redlock([redis], {
    driftFactor: 0.01, // multiplied by lock ttl to determine drift time
    retryCount: 60,
    retryDelay: 1000, // time in ms
    retryJitter: 200, // time in ms
    automaticExtensionThreshold: 500, // time in ms
  })

  return redlock
}

interface ConstructorArgs {
  configModule: ConfigModule
  redisClient: Redis
}

class LockService {
  private redlock: Redlock
  constructor(args: ConstructorArgs) {
    this.redlock = initializeRedlock(args.redisClient)
  }

  async using<T>(
    {
      resources,
      duration,
      settings = {},
    }: { resources: string[]; duration: number; settings?: Partial<Settings> },
    routine: (signal: RedlockAbortSignal) => Promise<T>
  ) {
    return this.redlock.using(resources, duration, settings, routine)
  }
}

export default LockService

[khanh-to-niteco]

Hi @dwene ,

It seems that RedLock is not safe: https://martin.kleppmann.com/2016/02/08/how-to-do-distributed-locking.html, but in our case, if we need only efficiency instead of correctness, it may work if it can help to avoid running concurrent batch jobs.

And ensuring only single instance running is not an optimal solution, would it be better if we can partition the events for the instances so they are less likely to step on each other toes and execute the events more quickly?

[dwene]

I'm not sure that's easily possible with bullmq.

You could also try this if redlock is not safe enough

https://www.npmjs.com/package/advisory-lock

[dwene]

Or spin up a zookeeper / etcd instance and use it for locking.

[khanh-to-niteco]

@dwene maybe a quick workaround is to throw in some randomness for the await sleep(3000) in every iteration so in case we are "unlucky" once, we don't repeatedly run into this issue for next iterations, then we can implement idempotency in the event handlers to handle the "unlucky" cases.