feat: refactor queue module
Closes goravel/goravel#153 Closes goravel/goravel#278
📑 Description
This is a draft, need more discussion.
I tried to reconstruct the queue module with reference to Laravel's implementation. There are some issues that need to be discussed and determined.
✅ Checks
- [ ] My pull request adheres to the code style of this project
- [ ] My code requires changes to the documentation
- [ ] I have updated the documentation as required
- [ ] All the tests have passed
ℹ Additional Information
Summary by CodeRabbit
-
New Features
- Introduced interface
Driverfor managing job queues with methods for pushing, popping, and scheduling jobs. - Added
MigrateMakeCommandstruct for creating migrations for failed queue jobs. - Implemented
Shutdownmethod for theWorkerto allow graceful shutdown. - Enhanced queue configuration with new methods like
Via()andFailedJobsQuery().
- Introduced interface
-
Bug Fixes
- Improved error handling in multiple methods across the application, including
RegisterandBoot. - Standardized import statements and references for consistency.
- Improved error handling in multiple methods across the application, including
-
Refactor
- Updated structs and method signatures across various files to use more generic types and new field names.
- Replaced
machinerywithdriverinWorkerandTaskstructs for better integration. - Modified SQL statements for database consistency.
-
Tests
- Adjusted test cases to reflect new configurations and updated struct fields.
- Simplified argument passing mechanisms within the test functions.
-
Documentation
- Updated references in import statements for clarity and consistency.
- Revised comments to reflect new method signatures and struct fields.
cc @hwbrzzl
@hwbrzzl Please help me check if there are any problems with the implementation idea. If there are no problems, I will continue to complete other driver development.
Thanks for this, was trying to implement AWS SQS into a potential replacement product with goravel. And found that adding my own driver wasn't an option. Anyway keep it up! 💪
Codecov Report
Attention: Patch coverage is 50.47022% with 158 lines in your changes missing coverage. Please review.
Project coverage is 70.26%. Comparing base (
f2b9567) to head (b71cfeb). Report is 34 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #314 +/- ##
==========================================
- Coverage 70.34% 70.26% -0.09%
==========================================
Files 180 183 +3
Lines 11212 11202 -10
==========================================
- Hits 7887 7871 -16
- Misses 2751 2763 +12
+ Partials 574 568 -6
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
This PR is basically completed, please let me know if there are any areas that need improvement.
Known issues:
- Database driver lacks test cases.
- Features related to task timeout have not been implemented yet.
- Features related to retry failed task have not been implemented yet.
- Database migration create command have not been implemented yet.
cc @hwbrzzl
@devhaozi Great job man, do we cover all current features?
@devhaozi Great job man, do we cover all current features?
Yes
@devhaozi Great job man, do we cover all current features?
Yes
Laravel support this https://learnku.com/docs/laravel/10.x/queues/14873#8dd0bc and https://learnku.com/docs/laravel/10.x/queues/14873#6a8bef.
Currently, I have reserved relevant interfaces and methods, but they have not been fully implemented yet.
Got it, will take a look.
@hwbrzzl If redis BLPop timeout setting to 60 seconds, this test case will fail.
@hwbrzzl If redis BLPop timeout setting to 60 seconds, this test case will fail.
Why?
@hwbrzzl If redis BLPop timeout setting to 60 seconds, this test case will fail.
Why?
The logic here is similar to Laravel. First, need to merge the tasks in the delay queue into the main queue.
Next, BLPop will block until there is a task in the main queue or the timeout is reached.
Therefore, if it is set to 60s, it will always be blocked, causing the test to fail.
The timeout parameter means an error will be thrown if something spends more time, right? Why will block?
The timeout parameter means an error will be thrown if something spends more time, right? Why will block?
No, no, no, this is a blocking method, it will block until there is a task in the queue list or the timeout is reached. This is not specific to task running time, in fact, no control over task running time is currently implemented.
@devhaozi I see, thanks!
Hi @devhaozi Do you still work on this, please? The PR is a bit complex and spent a lot of time, I think we only need to implement the Redis driver for now, the DB driver is optional.
Hi @devhaozi Do you still work on this, please? The PR is a bit complex and spent a lot of time, I think we only need to implement the Redis driver for now, the DB driver is optional.
I'm busy with other work, you can take over this PR if you have time.
Hi @devhaozi Do you still work on this, please? The PR is a bit complex and spent a lot of time, I think we only need to implement the Redis driver for now, the DB driver is optional.
I'm busy with other work, you can take over this PR if you have time.
I think only need to implement sync and async here, and consider splitting database and redis into two packages.
I will test this feature in my project.
After remove machinery and sonic, the binary size has been reduced by about 50MB.
After the discussion in the group, I think a goravel/machinery is necessary, we need to implement the driver interface in this PR through machinery, otherwise, users will hard to upgrade the framework in the future. Another way is createing a migrate command: machinery -> db or redis, but we cannot guarantee that it will be compatible with all situations.
After the discussion in the group, I think a goravel/machinery is necessary, we need to implement the driver interface in this PR through machinery, otherwise, users will hard to upgrade the framework in the future. Another way is createing a migrate command: machinery -> db or redis, but we cannot guarantee that it will be compatible with all situations.
I prefer the second way.
I prefer the second way.
If so, we need to check the machinery code for how to generate a structure from the queue, then transform the structure to ours.
[!IMPORTANT]
Review skipped
Auto reviews are limited to specific labels.
Labels to auto review (1)
- 🚀 Review Ready
Please check the settings in the CodeRabbit UI or the
.coderabbit.yamlfile in this repository. To trigger a single review, invoke the@coderabbitai reviewcommand.You can disable this status message by setting the
reviews.review_statustofalsein the CodeRabbit configuration file.
Walkthrough
The changes focus on enhancing the queue system by introducing new methods and modifying existing ones, refining job handling, improving error handling, and expanding the trigger events in the GitHub workflow. Additionally, significant updates to dependencies and database migration stubs were made to ensure consistency and support for asynchronous job processing.
Changes
| File(s) | Change Summary |
|---|---|
.github/workflows/pr-check-title.yml |
Switched trigger from pull_request to pull_request_target and refined event triggers. |
contracts/queue/driver.go |
Introduced Driver interface with methods for job queue management. |
contracts/queue/job.go |
Updated Jobs struct to include a Delay field. |
contracts/queue/queue.go |
Modified Queue interface methods and added new ones, including GetJob and Shutdown. |
contracts/queue/task.go |
Changed Delay method signature in Task interface from time.Time to uint. |
database/console/migrate_stubs.go |
Modified SQL statements for consistency across database platforms. |
event/application.go |
Added error handling in Register method of Application struct. |
event/service_provider.go |
Renamed package eventConsole to eventconsole and updated references. |
event/task.go |
Simplified conversion process from event.Arg to any type. |
event/task_test.go |
Updated test function to use any type for job handling. |
go.mod |
Updated dependencies, including removal of machinery and addition of do package. |
mail/application.go |
Modified argument passing in Queue method to use any type. |
mail/application_test.go |
Restructured test suite logic and configurations for mail and queue operations. |
mail/service_provider.go |
Added error handling in Boot method of ServiceProvider struct. |
queue/application.go |
Made changes to Application struct, including new methods and modifications to existing ones. |
queue/config.go |
Updated methods in Config struct for queue configuration. |
queue/config_test.go |
Modified configuration settings and test cases for queue connections. |
queue/console/migrate_make_command.go |
Added MigrateMakeCommand struct for creating migration for failed queue jobs database table. |
queue/driver_async.go |
Introduced functionality for asynchronous job processing. |
queue/task.go |
Major changes to Task struct, including field types and method signatures. |
queue/task_test.go |
Updated TestDispatchSync function to use JobImpl for job registration. |
queue/utils_test.go |
Renamed import queuecontract to contractsqueue and updated references. |
queue/worker.go |
Updated Worker struct fields and methods, including new Shutdown method and asynchronous job processing. |
Assessment against Linked Issues
| Objective | Addressed | Explanation |
|---|---|---|
| Refactor the queue facades (#153) | ✅ | |
| Implement async and sync job dispatch, Redis/Database/memory driver (#153) | ✅ | |
| Provide interfaces to get queue status (#153) | ✅ | |
| Fix queue always printing DEBUG messages with Redis driver (#278) | ❓ | Needs review to confirm if debug messages issue has been resolved. |
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
Tips
Chat
There are 3 ways to chat with CodeRabbit:
- Review comments: Directly reply to a review comment made by CodeRabbit. Example:
-
I pushed a fix in commit <commit_id>. -
Generate unit testing code for this file. -
Open a follow-up GitHub issue for this discussion.
-
- Files and specific lines of code (under the "Files changed" tab): Tag
@coderabbitaiin a new review comment at the desired location with your query. Examples:-
@coderabbitai generate unit testing code for this file. -
@coderabbitai modularize this function.
-
- PR comments: Tag
@coderabbitaiin a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:-
@coderabbitai generate interesting stats about this repository and render them as a table. -
@coderabbitai show all the console.log statements in this repository. -
@coderabbitai read src/utils.ts and generate unit testing code. -
@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format. -
@coderabbitai help me debug CodeRabbit configuration file.
-
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.
CodeRabbit Commands (invoked as PR comments)
-
@coderabbitai pauseto pause the reviews on a PR. -
@coderabbitai resumeto resume the paused reviews. -
@coderabbitai reviewto trigger an incremental review. This is useful when automatic reviews are disabled for the repository. -
@coderabbitai full reviewto do a full review from scratch and review all the files again. -
@coderabbitai summaryto regenerate the summary of the PR. -
@coderabbitai resolveresolve all the CodeRabbit review comments. -
@coderabbitai configurationto show the current CodeRabbit configuration for the repository. -
@coderabbitai helpto get help.
Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
CodeRabbit Configuration File (.coderabbit.yaml)
- You can programmatically configure CodeRabbit by adding a
.coderabbit.yamlfile to the root of your repository. - Please see the configuration documentation for more information.
- If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation:
# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
Documentation and Community
- Visit our Documentation for detailed information on how to use CodeRabbit.
- Join our Discord Community to get help, request features, and share feedback.
- Follow us on X/Twitter for updates and announcements.
Currently, machinery has resumed maintenance, so this PR may no longer be needed.
machinery still doesn't support the DB driver, right?
machinery still doesn't support the DB driver, right?
Yes
If so, I think it's better to implement the DB driver ourselves.