server: return error on ping when in graceful shutdown #58008
Conversation
|
Hi @dveeden. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/ok-to-test |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #58008 +/- ##
================================================
+ Coverage 73.1633% 73.7072% +0.5438%
================================================
Files 1674 1678 +4
Lines 461261 471346 +10085
================================================
+ Hits 337474 347416 +9942
+ Misses 103041 102976 -65
- Partials 20746 20954 +208
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Small demo on how this works with Go: Setup a server with this PR and graceful wait set to 30s. Run the demo Now press Now let's look at the results: As you can see the lines starting with The lines with This shows that anything that does a The demo code: package main
import (
"context"
"database/sql"
"fmt"
"sync"
"time"
_ "github.com/go-sql-driver/mysql"
)
var wg sync.WaitGroup
func dbpinger(db *sql.DB, nr int) error {
defer wg.Done()
for {
fmt.Printf("p %d: OK %s\n", nr, time.Now())
err := db.Ping()
if err != nil {
fmt.Printf("p %d: ERROR %v\n", nr, err)
return err
}
time.Sleep(time.Second * 3)
}
}
func dbworker(db *sql.DB, nr int) error {
defer wg.Done()
var ts []uint8
ctx := context.Background()
tx, err := db.BeginTx(ctx, nil)
if err != nil {
fmt.Printf("w %d: ERROR %v\n", nr, err)
return err
}
defer tx.Rollback()
for {
err := tx.QueryRowContext(ctx, "SELECT NOW(6)").Scan(&ts)
if err != nil {
fmt.Printf("w %d: ERROR %v\n", nr, err)
return err
}
fmt.Printf("w %d: OK: %s\n", nr, ts)
time.Sleep(time.Second * 3)
}
}
func main() {
db, err := sql.Open("mysql", "root@tcp(127.0.0.1:4000)/test")
if err != nil {
panic(err)
}
for i := 0; i < 10; i++ {
wg.Add(1)
go dbpinger(db, i)
}
for i := 0; i < 10; i++ {
wg.Add(1)
go dbworker(db, i)
}
wg.Wait()
} |
|
/retest |
|
I'm afraid some clients may rely on COM_PING to decide whether the connection is created successfully, which will cause connection failure. However, I don't know such client drivers or proxies, and I can't enumerate all the drivers and proxies, so I choose to approve this PR. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: djshow832, lance6716 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Issue Number: ref #58007
Problem Summary:
What changed and how does it work?
Check List
Tests
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.
From ProxySQL: