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

🐛 Bug Report: Cancelling Scheduled Event Edge Case #5050

Closed
2 tasks done
Khongchai opened this issue Jan 5, 2024 · 1 comment
Closed
2 tasks done

🐛 Bug Report: Cancelling Scheduled Event Edge Case #5050

Khongchai opened this issue Jan 5, 2024 · 1 comment
Labels
bug Something isn't working worker Created by Linear-GitHub Sync

Comments

@Khongchai
Copy link
Contributor

Khongchai commented Jan 5, 2024

📜 Description

Hi, Novu team! Thanks for the amazing product.

When scheduling an event a lot of users (multicast or broadcast to huge number of subscrbers), let's say the time this takes is 10 minutes (maybe a million users). If we were to call the cancel endpoint in the middle of that 10 minutes span, the cancel endpoint would only cancel jobs that have already been created (scheduled).

👟 Reproduction steps

Run a script to create a lot of users (like > 10k)

Schedule it to a certain point in the future.

As soon as you schedule, cancel immediately.

Some of the messages will still arrive because they were created after the cancel event.

👍 Expected behavior

Perhaps we should keep track of which transaction id got cancelled and check that along with the job status enum?

In the transaction cancel-delayed-usecase.ts

    await this.jobRepository.update(
      {
        _environmentId: command.environmentId,
        _id: {
          $in: transactionJobs.map((job) => job._id),
        },
      },
      {
        $set: {
          status: JobStatusEnum.CANCELED,
        },
      }
    );

   // HERE
   markTransactionIdAsCancelled(command.transactionId);

and then in run-job.usecase.ts, we check the cancel status like so

  @Instrument()
  private async delayedEventIsCanceled(
    job: JobEntity
  ): Promise<{ canceled: boolean; activeDigestFollower: JobEntity | null }> {
    let activeDigestFollower: JobEntity | null = null;

    if (job.type !== StepTypeEnum.DIGEST && job.type !== StepTypeEnum.DELAY) {
      return { canceled: false, activeDigestFollower };
    }

   // this
    const canceled = job.status === JobStatusEnum.CANCELED || checkTransactionIdCancelled(job.transactionId);

// ...

👎 Actual Behavior with Screenshots

Currently, even with checking both pending, delayed, and merged jobs don't suffice.

image

Because as I mentioned earlier, some of the jobs haven't even been scheduled. The loop inside trigger-broadcast-usecase.ts might still being run and things are still in the process of being sent to subscriberProcessQueueService.

image

Novu version

0.22.0

npm version

No response

node version

No response

📃 Provide any additional context for the Bug.

No response

👀 Have you spent some time to check if this bug has been raised before?

  • I checked and didn't find a similar issue

🏢 Have you read the Contributing Guidelines?

Are you willing to submit PR?

Yes I am willing to submit a PR!

Copy link

linear bot commented Jan 5, 2024

@github-actions github-actions bot added the triage label Jan 5, 2024
@scopsy scopsy removed the triage label Jan 22, 2024
@scopsy scopsy added bug Something isn't working worker Created by Linear-GitHub Sync labels Jan 22, 2024 — with Linear
@linear linear bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working worker Created by Linear-GitHub Sync
Projects
None yet
Development

No branches or pull requests

2 participants