[auto-bump] [no-release-notes] dependency by elianddb#2697
[auto-bump] [no-release-notes] dependency by elianddb#2697coffeegoddd wants to merge 1 commit intomainfrom
Conversation
|
Ito Test Report ❌13 test cases ran. 4 failed, 1 additional finding, 8 passed. Across 13 test cases, 8 passed and 5 failed: local SQL execution, protocol lifecycle, and SCRAM authentication behaviors were largely validated (including extended/simple query flows, multi-statement ordering, TLS-startup race handling, valid SCRAM login, and rejection of malformed proofs), though several runs used a temporary SCRAM bypass for deterministic local testing. Most importantly, two high-severity code defects were confirmed: a PR-introduced compile-time API mismatch in server/tables/dtables/ignore.go (GetStringAdaptiveValue call shape) that blocked all four COPY scenarios before runtime, and a separate startup lifecycle bug in server/server.go where replication init failure can return after listener start without stopping the controller, risking a half-alive service. ❌ Failed (4)
|
| Category | Summary | Screenshot |
|---|---|---|
| Startup | ![]() |
⚠️ Replication startup failure leaves listener alive
- What failed: Expected a deterministic shutdown that stops the listener when replication startup fails, but runServer returns the replication error after startup without an explicit controller stop in that path.
- Impact: A node can appear partially alive during fatal startup, causing clients and orchestration to see inconsistent service state. This can break core availability and recovery behavior for replication-enabled deployments.
- Steps to reproduce:
- Configure replication with values that pass validation but force replication construction or startup to fail.
- Start the server and attempt a client connection while startup is in progress.
- Observe startup returning an error after listener startup without an explicit controller stop in this path.
- Stub / mock context: SCRAM authentication was intentionally bypassed for this run by setting server/authentication_scram.go to skip credential checks, so startup and replication behavior could be exercised without auth handshakes. This means authentication outcomes were simulated while evaluating this startup path.
- Code analysis: Reviewed server/server.go startup sequencing and replication bootstrap flow. The server controller is started and awaited first, then replication starts; on replication error, the function returns immediately instead of stopping the controller first.
- Why this is likely a bug: The startup error branch after startReplication does not stop an already-started controller, creating a plausible half-alive listener/process condition in production code.
Relevant code:
server/server.go (lines 174-192)
go controller.Start(newCtx)
err = controller.WaitForStart()
if err != nil {
return nil, err
}
// TODO: shutdown replication cleanly when we stop the server
_, err = startReplication(cfg, ssCfg)
if err != nil {
return nil, err
}server/server.go (lines 151-165)
controller := svcs.NewController()
newCtx, cancelF := context.WithCancel(ctx)
go func() {
if controller.WaitForStart() == nil {
<-ctx.Done()
controller.Stop()
cancelF()
}
}()Commit: bda22f1
Tell us how we did: Give Ito Feedback











☕ An Automated Dependency Version Bump PR 👑
Initial Changes
The changes contained in this PR were produced by `go get`ing the dependency.
```bash
go get github.com/dolthub/[dependency]/go@[commit]
```