This repository was archived by the owner on Sep 30, 2024. It is now read-only.
feat/sg: handle interrupts more gracefully#62772
Merged
Conversation
Member
|
Thank you! This is great See example output here, this works very well for testing gitserver shutdown behavior for example. It seems that some of our services don't really like to shutdown well, which is not an issue of this PR, but great that we now get visibility into this. |
jhchabran
suggested changes
May 22, 2024
Contributor
jhchabran
left a comment
jhchabran
left a comment
There was a problem hiding this comment.
Got some panic while doing a test run:
worker] INFO worker.own-repo-indexing-queue.own.background.index.scheduler.analytics background/scheduler.go:108 skipping own indexing job, job disabled {"job-name": "analytics"}
[ worker] INFO worker.insights-job.background-insight-enqueuer background/insight_enqueuer.go:94 enqueuing indexed insight snapshots
[ worker] INFO worker.telemetrygateway-exporter telemetrygatewayexporter/exporter.go:131 events exported {"TraceId": "ea3b2341882d73159b39d0419a9628e5", "SpanId": "2405d65938edf604", "maxBatchSize": 10000, "succeeded": 2}
[ repo-updater] INFO repo-updater.syncer repos/syncer.go:450 syncing external service {"externalServiceID": 1}
[ repo-updater] INFO repoPurgeWorker purge/purge.go:144 repository purge finished {"deletedBefore": 1716388261010998000, "total": 0, "removed": 0, "failed": 0, "duration": "158.906459ms"}
^C⚠️ Interrupt received, executing 22 hooks for graceful shutdown...
[ symbols] Bazel caught interrupt signal; cancelling pending invocation.
[ symbols] ERROR: command interrupted while computing main repo mapping
[ symbols] Error: bazel exited with exit code: 8
panic: panic: failed kill process group ID 43092 for cmd repo-updater : operation not permitted
stacktrace:
goroutine 249 [running]:
runtime/debug.Stack()
/Users/tech/.asdf/installs/golang/1.22.1/go/src/runtime/debug/stack.go:24 +0x64
github.com/sourcegraph/conc/panics.NewRecovered(0x1, {0x106cdf3c0, 0x140007e8c90})
/Users/tech/work/other/vendor/github.com/sourcegraph/conc/panics/panics.go:59 +0x74
github.com/sourcegraph/conc/panics.(*Catcher).tryRecover(0x140026ac110)
/Users/tech/work/other/vendor/github.com/sourcegraph/conc/panics/panics.go:28 +0x58
panic({0x106cdf3c0?, 0x140007e8c90?})
/Users/tech/.asdf/installs/golang/1.22.1/go/src/runtime/panic.go:770 +0x124
github.com/sourcegraph/conc/pool.(*ContextPool).Go.func1.1()
/Users/tech/work/other/vendor/github.com/sourcegraph/conc/pool/context_pool.go:33 +0x60
panic({0x106cdf3c0?, 0x140007e8c90?})
/Users/tech/.asdf/installs/golang/1.22.1/go/src/runtime/panic.go:770 +0x124
github.com/sourcegraph/sourcegraph/dev/sg/internal/run.startCmd.func1()
/Users/tech/work/other/dev/sg/internal/run/command.go:342 +0x168
github.com/sourcegraph/sourcegraph/dev/sg/internal/run.(*cmdRunner).run.func1({0x107150ae8, 0x140009be000})
/Users/tech/work/other/dev/sg/internal/run/run.go:85 +0x9d8
github.com/sourcegraph/conc/pool.(*ContextPool).Go.func1()
/Users/tech/work/other/vendor/github.com/sourcegraph/conc/pool/context_pool.go:38 +0x70
github.com/sourcegraph/conc/pool.(*ContextPool).Go.(*ErrorPool).Go.func2()
/Users/tech/work/other/vendor/github.com/sourcegraph/conc/pool/error_pool.go:30 +0x30
github.com/sourcegraph/conc/pool.(*Pool).worker(0x140026ac100, 0x4d4f430a0a3b272e?)
/Users/tech/work/other/vendor/github.com/sourcegraph/conc/pool/pool.go:156 +0x5c
github.com/sourcegraph/conc/pool.(*Pool).Go.func1()
/Users/tech/work/other/vendor/github.com/sourcegraph/conc/pool/pool.go:51 +0x24
github.com/sourcegraph/conc/panics.(*Catcher).Try(0x0?, 0x100cc69d8?)
/Users/tech/work/other/vendor/github.com/sourcegraph/conc/panics/panics.go:23 +0x50
github.com/sourcegraph/conc.(*WaitGroup).Go.func1()
/Users/tech/work/other/vendor/github.com/sourcegraph/conc/waitgroup.go:32 +0x58
created by github.com/sourcegraph/conc.(*WaitGroup).Go in goroutine 132
/Users/tech/work/other/vendor/github.com/sourcegraph/conc/waitgroup.go:30 +0x7c
goroutine 132 [running]:
github.com/sourcegraph/conc/panics.(*Catcher).Repanic(...)
/Users/tech/work/other/vendor/github.com/sourcegraph/conc/panics/panics.go:38
github.com/sourcegraph/conc.(*WaitGroup).Wait(0x140026ac100)
/Users/tech/work/other/vendor/github.com/sourcegraph/conc/waitgroup.go:42 +0x5c
github.com/sourcegraph/conc/pool.(*Pool).Wait(0x140026ac100)
/Users/tech/work/other/vendor/github.com/sourcegraph/conc/pool/pool.go:81 +0x60
github.com/sourcegraph/conc/pool.(*ErrorPool).Wait(0x140026ac100)
/Users/tech/work/other/vendor/github.com/sourcegraph/conc/pool/error_pool.go:37 +0x24
github.com/sourcegraph/conc/pool.(*ContextPool).Wait(0x140026ac100?)
/Users/tech/work/other/vendor/github.com/sourcegraph/conc/pool/context_pool.go:57 +0x38
github.com/sourcegraph/sourcegraph/dev/sg/internal/run.(*cmdRunner).run(0x14002680510, {0x107150ae8?, 0x14001c99810?})
/Users/tech/work/other/dev/sg/internal/run/run.go:133 +0x130
github.com/sourcegraph/sourcegraph/dev/sg/internal/run.Commands({0x107150ae8, 0x14001c99810}, 0x1400204adb0, 0x0, {0x1400247a100, 0x10, 0x10})
/Users/tech/work/other/dev/sg/internal/run/run.go:54 +0x2cc
main.(*Commands).start(0x14001c996d0, {0x107150ae8, 0x14001c99810})
/Users/tech/work/other/dev/sg/sg_start.go:420 +0x298
main.start.func1()
/Users/tech/work/other/dev/sg/sg_start.go:311 +0x64
created by main.start in goroutine 1
/Users/tech/work/other/dev/sg/sg_start.go:307 +0x390
exit status 2
Contributor
Author
|
@jhchabran did you do anything particular here? I haven't been able to reproduce |
Contributor
Nothing, it was just |
jhchabran
approved these changes
May 23, 2024
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
Currently sending a SIGINT to sg will actually exit immediately and not run cleanup. This comes from two issues: we weren't actually waiting for processes to exit after we interrupt them, and we weren't waiting for our interrupt handlers to run. For a bonus, we also add support for SIGHUP which is what happens when you close a terminal.
Test plan
Created a script which waited for 10 seconds on SIGINT and verified that SG actually waited for it to clean up and logged it's output.