You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@seata.apache.org by "WyRainBow (via GitHub)" <gi...@apache.org> on 2025/11/23 08:31:36 UTC
[PR] fix Background Goroutines Cannot Exit Gracefully [incubator-seata-go]
WyRainBow opened a new pull request, #996:
URL: https://github.com/apache/incubator-seata-go/pull/996
fix #992
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] fix Background Goroutines Cannot Exit Gracefully [incubator-seata-go]
Posted by "WyRainBow (via GitHub)" <gi...@apache.org>.
WyRainBow commented on PR #996:
URL: https://github.com/apache/incubator-seata-go/pull/996#issuecomment-3619599527
> Perhaps you could clone the samples repository and replace its "seata go" dependency with your modified local version to verify if it works correctly.
Verified by cloning apache/seata-go-samples, replacing the dependency with my local incubator-seata-go via go mod edit -replace ..., running go mod tidy, and all go test ./... runs succeeded on both the samples repo and the main repo.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] fix Background Goroutines Cannot Exit Gracefully [incubator-seata-go]
Posted by "0x-infinity (via GitHub)" <gi...@apache.org>.
0x-infinity commented on code in PR #996:
URL: https://github.com/apache/incubator-seata-go/pull/996#discussion_r2554151563
##########
pkg/datasource/sql/async_worker.go:
##########
@@ -123,7 +129,9 @@ func (aw *AsyncWorker) BranchCommit(ctx context.Context, req rm.BranchResource)
}
func (aw *AsyncWorker) run() {
+ defer aw.wg.Done()
Review Comment:
Remove this line.
##########
pkg/datasource/sql/async_worker.go:
##########
@@ -68,6 +69,9 @@ type AsyncWorker struct {
doBranchCommitFailureTotal prometheus.Counter
receiveChanLength prometheus.Gauge
rePutBackToQueue prometheus.Counter
+
+ done chan struct{}
+ wg sync.WaitGroup
Review Comment:
The `done` channel and `wg` are not needed here.
##########
pkg/datasource/sql/async_worker.go:
##########
@@ -134,10 +142,18 @@ func (aw *AsyncWorker) run() {
}
case <-ticker.C:
aw.doBranchCommit(&phaseCtxs)
+ case <-aw.done:
+ return
}
}
}
+// Stop stops the AsyncWorker.
Review Comment:
Similarly, there is no need to add a new `Stop` method here.
##########
pkg/datasource/sql/async_worker.go:
##########
@@ -134,10 +142,18 @@ func (aw *AsyncWorker) run() {
}
case <-ticker.C:
aw.doBranchCommit(&phaseCtxs)
+ case <-aw.done:
Review Comment:
Just listen to `ctx.Done`.
##########
pkg/datasource/sql/async_worker.go:
##########
@@ -123,7 +129,9 @@ func (aw *AsyncWorker) BranchCommit(ctx context.Context, req rm.BranchResource)
}
func (aw *AsyncWorker) run() {
Review Comment:
This run method is private, changing it to `run(ctx context.Context)` will not affect the outside.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] fix Background Goroutines Cannot Exit Gracefully [incubator-seata-go]
Posted by "WyRainBow (via GitHub)" <gi...@apache.org>.
WyRainBow commented on code in PR #996:
URL: https://github.com/apache/incubator-seata-go/pull/996#discussion_r2554726704
##########
pkg/datasource/sql/datasource/base/meta_cache.go:
##########
@@ -127,34 +124,43 @@ func (c *BaseTableMetaCache) refresh(ctx context.Context) {
f()
- ticker := time.NewTicker(time.Duration(1 * time.Minute))
+ ticker := time.NewTicker(c.refreshInterval)
Review Comment:
Adding refreshInterval is mainly to improve test coverage and test efficiency.
Test efficiency:
If the interval is hard-coded to 1 minute, each test run would take far too long, which is impractical.
Test coverage:
If we don’t wait long enough, we can’t cover the logic that runs after ticker.C fires (i.e., the second execution of f()). To reach the case <-ticker.C: branch, the ticker must trigger quickly.
Therefore, exposing this field allows tests to control the interval, making it possible to exercise that branch and improve overall coverage.
In unit tests (like TestBaseTableMetaCache_refresh
and TestBaseTableMetaCache_GracefulShutdown ) we need to verify the behavior of the scheduled task.
If refreshInterval is hardcoded as a constant (eg 1 * time.Minute) inside the refresh method, the test would have to wait for 1 minute to trigger the logic, making the tests extremely slow.*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] fix Background Goroutines Cannot Exit Gracefully [incubator-seata-go]
Posted by "LucienShen-Liu (via GitHub)" <gi...@apache.org>.
LucienShen-Liu commented on PR #996:
URL: https://github.com/apache/incubator-seata-go/pull/996#issuecomment-3678451420
Hi, please fix the CI when you have time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] fix Background Goroutines Cannot Exit Gracefully [incubator-seata-go]
Posted by "WyRainBow (via GitHub)" <gi...@apache.org>.
WyRainBow commented on PR #996:
URL: https://github.com/apache/incubator-seata-go/pull/996#issuecomment-3619598172
Verified by cloning apache/seata-go-samples, replacing the dependency with my local incubator-seata-go via go mod edit -replace ..., running go mod tidy, and all go test ./... runs succeeded on both the samples repo and the main repo.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] fix: background goroutines cannot exit gracefully [incubator-seata-go]
Posted by "thunguo (via GitHub)" <gi...@apache.org>.
thunguo merged PR #996:
URL: https://github.com/apache/incubator-seata-go/pull/996
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] fix Background Goroutines Cannot Exit Gracefully [incubator-seata-go]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #996:
URL: https://github.com/apache/incubator-seata-go/pull/996#issuecomment-3567627544
## [Codecov](https://app.codecov.io/gh/apache/incubator-seata-go/pull/996?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `9.09091%` with `30 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 61.12%. Comparing base ([`174d2f8`](https://app.codecov.io/gh/apache/incubator-seata-go/commit/174d2f8544e654b6d9d10fab42575fa664d60d3f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`3be967c`](https://app.codecov.io/gh/apache/incubator-seata-go/commit/3be967cb6f23b83e90de279757424c268fdae2f8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/incubator-seata-go/pull/996?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [pkg/datasource/sql/datasource/base/meta\_cache.go](https://app.codecov.io/gh/apache/incubator-seata-go/pull/996?src=pr&el=tree&filepath=pkg%2Fdatasource%2Fsql%2Fdatasource%2Fbase%2Fmeta_cache.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL2RhdGFzb3VyY2Uvc3FsL2RhdGFzb3VyY2UvYmFzZS9tZXRhX2NhY2hlLmdv) | 12.50% | [21 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-seata-go/pull/996?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pkg/datasource/sql/async\_worker.go](https://app.codecov.io/gh/apache/incubator-seata-go/pull/996?src=pr&el=tree&filepath=pkg%2Fdatasource%2Fsql%2Fasync_worker.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL2RhdGFzb3VyY2Uvc3FsL2FzeW5jX3dvcmtlci5nbw==) | 0.00% | [9 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-seata-go/pull/996?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #996 +/- ##
==========================================
- Coverage 61.20% 61.12% -0.08%
==========================================
Files 213 213
Lines 13052 13073 +21
==========================================
+ Hits 7988 7991 +3
- Misses 4542 4561 +19
+ Partials 522 521 -1
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/incubator-seata-go/pull/996?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] fix Background Goroutines Cannot Exit Gracefully [incubator-seata-go]
Posted by "0x-infinity (via GitHub)" <gi...@apache.org>.
0x-infinity commented on code in PR #996:
URL: https://github.com/apache/incubator-seata-go/pull/996#discussion_r2553945209
##########
pkg/datasource/sql/async_worker.go:
##########
@@ -68,6 +69,10 @@ type AsyncWorker struct {
doBranchCommitFailureTotal prometheus.Counter
receiveChanLength prometheus.Gauge
rePutBackToQueue prometheus.Counter
+
+ ctx context.Context
Review Comment:
You might consider whether there is a better approach, using context as a member of a struct is not a good practice.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] fix Background Goroutines Cannot Exit Gracefully [incubator-seata-go]
Posted by "0x-infinity (via GitHub)" <gi...@apache.org>.
0x-infinity commented on PR #996:
URL: https://github.com/apache/incubator-seata-go/pull/996#issuecomment-3618114628
Perhaps you could clone the samples repository and replace its "seata go" dependency with your modified local version to verify if it works correctly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] fix Background Goroutines Cannot Exit Gracefully [incubator-seata-go]
Posted by "LucienShen-Liu (via GitHub)" <gi...@apache.org>.
LucienShen-Liu commented on PR #996:
URL: https://github.com/apache/incubator-seata-go/pull/996#issuecomment-3690800381
> Hi, please fix the CI when you have time.
There's currently an issue with the integration test; it should pass once we fix it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] fix Background Goroutines Cannot Exit Gracefully [incubator-seata-go]
Posted by "tew-axiom (via GitHub)" <gi...@apache.org>.
tew-axiom commented on PR #996:
URL: https://github.com/apache/incubator-seata-go/pull/996#issuecomment-3677635378
Plz check and resolve the conflicts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] fix Background Goroutines Cannot Exit Gracefully [incubator-seata-go]
Posted by "thunguo (via GitHub)" <gi...@apache.org>.
thunguo commented on PR #996:
URL: https://github.com/apache/incubator-seata-go/pull/996#issuecomment-3976765010
Plz push something to re-run the CI `integrate-test`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] fix Background Goroutines Cannot Exit Gracefully [incubator-seata-go]
Posted by "WyRainBow (via GitHub)" <gi...@apache.org>.
WyRainBow commented on PR #996:
URL: https://github.com/apache/incubator-seata-go/pull/996#issuecomment-3619616514
> Perhaps you could clone the samples repository and replace its "seata go" dependency with your modified local version to verify if it works correctly.
Verified by cloning apache/seata-go-samples, replacing the dependency with my local incubator-seata-go via go mod edit -replace ..., running go mod tidy, and all go test ./... runs succeeded on both the samples repo and the main repo.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] fix Background Goroutines Cannot Exit Gracefully [incubator-seata-go]
Posted by "0x-infinity (via GitHub)" <gi...@apache.org>.
0x-infinity commented on code in PR #996:
URL: https://github.com/apache/incubator-seata-go/pull/996#discussion_r2554676396
##########
pkg/datasource/sql/datasource/base/meta_cache.go:
##########
@@ -127,34 +124,43 @@ func (c *BaseTableMetaCache) refresh(ctx context.Context) {
f()
- ticker := time.NewTicker(time.Duration(1 * time.Minute))
+ ticker := time.NewTicker(c.refreshInterval)
Review Comment:
Why add refreshInterval? It seems this value is only used by this private method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org