fix: [#807] queue.Shutdown doesn't stop the queue as expected#1252
Merged
fix: [#807] queue.Shutdown doesn't stop the queue as expected#1252
Conversation
hwbrzzl
commented
Oct 29, 2025
| } | ||
|
|
||
| timestamp := carbon.FromStdTime(entry.Time).ToDateTimeMilliString() | ||
| timestamp := carbon.FromStdTime(entry.Time, carbon.DefaultTimezone()).ToDateTimeMilliString() |
Contributor
Author
There was a problem hiding this comment.
It was local previously, should use the DefaultTimezone.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: 794c7e3 | Previous: 34d675d | Ratio |
|---|---|---|---|
Benchmark_DecryptString |
6122 ns/op 2032 B/op 16 allocs/op |
2098 ns/op 2032 B/op 16 allocs/op |
2.92 |
Benchmark_DecryptString - ns/op |
6122 ns/op |
2098 ns/op |
2.92 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v1.16.x #1252 +/- ##
==========================================
Coverage ? 67.99%
==========================================
Files ? 216
Lines ? 11608
Branches ? 0
==========================================
Hits ? 7893
Misses ? 3335
Partials ? 380 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the worker shutdown process and improves timestamp formatting across the codebase. It ensures that worker goroutines properly wait for completion before shutting down, and adds a new DefaultTimezone() function to the carbon wrapper.
- Adds proper synchronization to worker shutdown by waiting for all goroutines to complete
- Improves timestamp precision by switching from
ToDateTimeString()toToDateTimeMilliString() - Adds explicit timezone handling for log timestamp formatting
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| support/carbon/carbon.go | Adds DefaultTimezone() wrapper function to expose the carbon library's default timezone |
| queue/worker.go | Enhances Shutdown() to wait for goroutines, updates timestamp formatting to millisecond precision |
| queue/worker_test.go | Adds comprehensive shutdown tests covering various scenarios (multiple workers, failed jobs, blocking behavior) |
| log/formatter/general.go | Updates timestamp formatting to use explicit default timezone parameter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
hwbrzzl
added a commit
that referenced
this pull request
Oct 31, 2025
* correctly set cc and bcc headers (#1144) * correct the error return from SendMailJob handle (#1147) * fix: [#743] package make command generates correct code (#1151) (#1152) (cherry picked from commit 93dc8a3) * fix: [#749] The path is incorrect when publishing package files (#1157) * chore: optimize assertions for package installation (#1160) * fix: The configuredServiceProviders, publishes and publishGroups are not reset when Booting (#1162) * fix: The configuredServiceProviders, publishes and publishGroups are not reset when Booting * optimize * fix: [#738] The Orm Creating event can be triggered when the query with the Model method (#1166) * fix: [#738] The Orm Creating event can be triggered when the query with the Model method * fix tests * upgrade: v1.16.1 * fix: [#762] handle panic when using transaction (#1183) * fix: [#762] handle panic when using transaction * v1.16.2 * optimize * fix lint * fix: [#768] facades.DB will panic when migrating a new column (#1185) * fix: [#768] facades.DB will panic when migrating a new column * optimize * optimize * feat: [#770] Add a SelectRaw function for the ORM (#1186) * fix: [#770] Add a SelectRaw function for the ORM * fix: [#770] Add a SelectRaw function for the ORM * fix ci * upgrade: v1.16.3 * fix: comand cannot be run concurrently (#1243) * fix: comand cannot be run concurrently * fix ci * fix ci * optimize * optimize * optimize * optimize * optimize global options * fix ci * upgrade v1.16.4 * fix: [#807] queue.Shutdown doesn't stop the queue as expected (#1252) * fix: [#807] queue.Shutdown doesn't stop the queue as expected * optimize * upgrade: v1.16.5 * fix * Update queue/worker_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * optimize --------- Co-authored-by: krishan kumar <84431594+kkumar-gcc@users.noreply.github.com> Co-authored-by: ALMAS <almas.cc@icloud.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
hwbrzzl
added a commit
that referenced
this pull request
Oct 31, 2025
* correctly set cc and bcc headers (#1144) * correct the error return from SendMailJob handle (#1147) * fix: [#743] package make command generates correct code (#1151) (#1152) (cherry picked from commit 93dc8a3) * fix: [#749] The path is incorrect when publishing package files (#1157) * chore: optimize assertions for package installation (#1160) * fix: The configuredServiceProviders, publishes and publishGroups are not reset when Booting (#1162) * fix: The configuredServiceProviders, publishes and publishGroups are not reset when Booting * optimize * fix: [#738] The Orm Creating event can be triggered when the query with the Model method (#1166) * fix: [#738] The Orm Creating event can be triggered when the query with the Model method * fix tests * upgrade: v1.16.1 * fix: [#762] handle panic when using transaction (#1183) * fix: [#762] handle panic when using transaction * v1.16.2 * optimize * fix lint * fix: [#768] facades.DB will panic when migrating a new column (#1185) * fix: [#768] facades.DB will panic when migrating a new column * optimize * optimize * feat: [#770] Add a SelectRaw function for the ORM (#1186) * fix: [#770] Add a SelectRaw function for the ORM * fix: [#770] Add a SelectRaw function for the ORM * fix ci * upgrade: v1.16.3 * fix: comand cannot be run concurrently (#1243) * fix: comand cannot be run concurrently * fix ci * fix ci * optimize * optimize * optimize * optimize * optimize global options * fix ci * upgrade v1.16.4 * fix: [#807] queue.Shutdown doesn't stop the queue as expected (#1252) * fix: [#807] queue.Shutdown doesn't stop the queue as expected * optimize * upgrade: v1.16.5 * fix * Update queue/worker_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * optimize --------- Co-authored-by: krishan kumar <84431594+kkumar-gcc@users.noreply.github.com> Co-authored-by: ALMAS <almas.cc@icloud.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📑 Description
Closes goravel/goravel#807
This pull request improves the reliability and logging accuracy of the worker queue system, particularly around shutdown behavior and log timestamp precision. It also enhances test coverage for worker shutdown scenarios and introduces a helper for timezone retrieval.
Worker shutdown reliability and test coverage:
Shutdownmethod inqueue/worker.goto ensure proper closure of the failed job channel and waiting for all worker goroutines to finish before shutting down, preventing premature termination and potential race conditions.queue/worker_test.goto verify that shutdown waits for workers and job processing to complete, including scenarios with multiple jobs and failed job logging.mockpackage import to support new test cases using mock expectations.Logging improvements:
log/formatter/general.goand worker log methods inqueue/worker.goto include milliseconds and respect the default timezone, improving log precision for debugging and monitoring. [1] [2] [3] [4]Timezone utilities:
DefaultTimezonehelper function insupport/carbon/carbon.goto retrieve the default timezone, supporting timezone-aware operations.✅ Checks