shiftonelabs / laravel-sqs-fifo-queue

Adds a Laravel queue driver for Amazon SQS FIFO queues.
MIT License
150 stars 61 forks source link

Inconsistent Message Group Ids #13

Closed jesseschutt closed 2 years ago

jesseschutt commented 2 years ago

Hello and thank you for your efforts on this package! It's much appreciated.

I'm working in Laravel 8 on Vapor and am having some trouble getting the queued notifications to use different message group ids. I'm queueing notifications using the following as an example:

(new ReadyNotification($assignment))
    ->onConnection('fifo')
    ->onMessageGroup($assignment->getUuid())

My queue configuration looks like this:

'fifo' => [
    'driver' => 'sqs-fifo',
    'key' => env('SQS_KEY'),
    'secret' => env('SQS_SECRET'),
    'prefix' => env('SQS_PREFIX'),
    'suffix' => env('SQS_SUFFIX', '-develop'),
    'queue' => 'fifo.fifo',
    'region' => env('AWS_DEFAULT_REGION', 'us-east-1'),
    'group' => 'fifo',
    'deduplicator' => 'sqs',
    'allow_delay' => env('SQS_ALLOW_DELAY'),
],

As I've inspected jobs on the queue occasionally one will be on the {assignmentUuid} group but they are typically falling onto the default group as specified by the config above.

I've attempted to set the message group in the constructor of the notification but it results in the same inconsistent behavior.

What advice might you have as I continue to debug this?

Again, thank you for your time!

jesseschutt commented 2 years ago

I figured out what caused the problem - It has to do with how Vapor retries failed jobs. It does not use my custom queue overrides as it uses a VaporJob class instead.

fourstacks commented 1 year ago

@jesseschutt - thanks for following up with your findings. Did you eventually find a solution to this?

Also, if you'd be able to briefly describe your process for getting Laravel Vapor to use a the fifo queue driver (and any other gotchas other than the above) that would be amazing.

I'm trying to figure out the best approach to a project that requires strict job ordering for jobs that are spawned from incoming webhooks and it's proving tricky!

PS - as a total aside, just wanted to say that your article on polymorphism from 2016 was the one that really helped the concept 'click' for me back then - big thanks for sharing that!)

jesseschutt commented 1 year ago

Hello @fourstacks - I did not find a solution to this specific problem due to how Vapor handles failed jobs. Unfortunately Vapor retries failed jobs using its own VaporJob class and there wasn't a good way of overriding that (at least at the time I was working on this).

What I ended up doing was to only allow the FIFO queable jobs one attempt and aborting if they failed. It required making the job classes more resilient and handle their own failures instead of just dumping them back onto the queue like I would do with a normal job.

Sorry I don't have a better solution for you!

PS - as a total aside, just wanted to say that your article on polymorphism from 2016 was the one that really helped the concept 'click' for me back then - big thanks for sharing that!)

It's neat to hear it was helpful! Thanks for sharing :-)

fourstacks commented 1 year ago

Thanks @jesseschutt ! Appreciate the tip on handling failures - will bear that in mind. I realise this is a bit off topic for this particular issue but did you run into any particular issues getting Vapor to work with a FIFO queue that Vapor itself wasn't responsible for provisioning?

jesseschutt commented 1 year ago

did you run into any particular issues getting Vapor to work with a FIFO queue that Vapor itself wasn't responsible for provisioning?

Nope, no problems at all. Just needed to add it under the queues: key in the vapor.yml file.

fourstacks commented 1 year ago

Oh nice! Was expecting far more wiring up in AWS than that. I only just fired up a project today to test it all out so it's good to know it might be (mostly) smooth sailing. Thanks again for the info!

devaygun commented 1 year ago

Hi guys, sorry to bump this closed discussion but I keep getting a AWS: Can only include alphanumeric characters, hyphens, or underscores. 1 to 80 in length error, my vapor.yml looks like:

    queues:
      - default-dev.fifo
      - high-dev.fifo
      - medium-dev.fifo
      - low-dev.fifo
      - search-dev.fifo
devaygun commented 1 year ago

@fourstacks @jesseschutt would you mind sharing your vapor.yml queues section?

jesseschutt commented 1 year ago

@aygunia - I think the only difference is that my queues section includes a typical vapor queue on the first line like this:

queues:
  - app-develop
  - fifo-develop.fifo
devaygun commented 1 year ago

Hey @jesseschutt, I appreciate the quick response!

But even with:

Screenshot 2023-07-26 at 13 56 37

I keep getting:

Screenshot 2023-07-26 at 13 56 16
devaygun commented 1 year ago

Which is odd because I'm able to generate queues with the name of default-dev.fifo manually in SQS just fine

jesseschutt commented 1 year ago

Did you manually create the FIFO queue or are you trying to have Vapor create it?

devaygun commented 1 year ago

@jesseschutt I haven't manually created the specific ones my vapor.yml references just yet, should I?

jesseschutt commented 1 year ago

Yes, I manually created the fifo queues via AWS console and then just added the name to the vapor.yml file.

devaygun commented 1 year ago

@jesseschutt It's working now, thank you!

Finally, just wondering if you used a similar configuration?

Screenshot 2023-07-26 at 14 06 02

Thanks again for your help!

jesseschutt commented 1 year ago

This is what we used:

Screenshot 2023-07-26 at 08 11 40

devaygun commented 1 year ago

@jesseschutt thanks, I've tried your config and mine and in both cases I keep receiving MissingParameter (client): The request must contain the parameter MessageGroupId and The request must contain the parameter MessageGroupId.

I've tried a few things such as


        (new \App\Jobs\SayHello())
            ->onMessageGroup('quarter')
            ->withDeduplicator('unique')
    );```
jesseschutt commented 1 year ago

@aygunia - I've moved away from this package as I ran into similar troubles. However, it does look like there has been development since I last used it.

You'll have to talk to the maintainers to get help on this issue.

devaygun commented 1 year ago

@jesseschutt good to know, so the package doesn't really work with Vapor

jesseschutt commented 1 year ago

I don't know that I'd say that it is specific to Vapor, but is more in how group ids are passed for deduplication.

devaygun commented 1 year ago

I think I've got it working, but think I might just avoid FIFO and see how my app holds up.

Did you have many specific FIFO cases?

jesseschutt commented 1 year ago

Did you have many specific FIFO cases?

Ours was primarily for broadcasting a sequence of messages to a user. So we used the message group id to identify a group of messages and the FIFO queue kept them in order. This approach allowed us to have the FIFO queue process sequences for multiple users at once while keeping the individual sequences intact.