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

Add support for laravel octane #12

Merged
merged 6 commits into from
Apr 20, 2024
Merged

Add support for laravel octane #12

merged 6 commits into from
Apr 20, 2024

Conversation

abublihi
Copy link
Contributor

@abublihi abublihi commented Apr 8, 2024

This pull request is related to ssi-anik/amqp#31, as this simple change will fix the issue while using laravel octane,

This change will make laravel only resolve the connection once after the application boots (warm), the user of the package should add the connection to warm parameter in Octane configuration file. ( this been added to readme )

Making only one connection to rabbitmq for each Worker of laravel octane will boost the performance.

Best regards.

…Consumer service container will keep the connection active for all requests coming to the same worker
@abublihi
Copy link
Contributor Author

@ssi-anik have you checked the PR?

@ssi-anik
Copy link
Owner

@abublihi Assalamu alaikum brother. Yes, I saw the PR the next day you submitted it. I was so busy that I couldn't reply to you. I am sorry for that.

Firstly, the consumer change is not necessary. Because the consumer is only booted once when you run the command and it keeps running. So, the connection issue for the consumer is not a problem for each consumption. Also, as you're keeping the reference of the consumer when passing options to the different consumers, it will return the old consumer as you have implemented if I am not wrong.

Secondly, for the producer, I am currently checking it. trying to make sure that it doesn't break. I will keep you posted here by tomorrow EoD insha-Allah.

@abublihi
Copy link
Contributor Author

abublihi commented Apr 17, 2024

وعليكم السلام ورحمة الله وبركاته،،

Thank you brother @ssi-anik for Reviewing the PR,,

For the consumer you are right I will discard the change, but for the producer I have a question does php-amqplib reconnect if the connection is disconnected for some reason?

@ssi-anik
Copy link
Owner

@abublihi I am again sorry for the delayed response. I am having trouble managing my time.

  • due to the warm config in octane and for your change in the getProducer method the connection is kept alive and listed under the connections tab.
  • If the producer's connection gets disconnected, it throws a connection broken exception.

So, please remove the consumer connection related changes, I will be merging it and then will work on the broken connection issue if possible.

Thanks for the PR.

@ssi-anik ssi-anik merged commit d31c0aa into ssi-anik:master Apr 20, 2024
8 checks passed
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.

2 participants