Skip to content

Commit 46cd10e

Browse files
MimeLyczanmato1984
MimeLyc
and
zanmato1984
authored
fix: set tidb tunning variables only when connected to valid tidb (#188)
* fix: set tidb tunning variables only when connected to valid tidb * fix typo * refactor * Update cmd/go-tpc/tpch.go Co-authored-by: Rossi Sun <zanmato1984@gmail.com> * refactor: log for hint unsupported sys var, and reusable semver compare func * refactor: rename func signum * more logs for hintting query tunning --------- Co-authored-by: Rossi Sun <zanmato1984@gmail.com>
1 parent 6cd9f74 commit 46cd10e

3 files changed

Lines changed: 269 additions & 10 deletions

File tree

‎cmd/go-tpc/tpch.go‎

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package main
22

33
import (
44
"context"
5+
"database/sql"
56
"fmt"
67
"os"
78
"runtime"
@@ -26,19 +27,13 @@ var queryTuningVars = []struct {
2627
{"tidb_prefer_broadcast_join_by_exchange_data_size", "ON"},
2728
}
2829

29-
func appendQueryTuningVarsToConnParams() {
30-
for _, v := range queryTuningVars {
31-
if !strings.Contains(connParams, v.name) {
32-
connParams = fmt.Sprintf("%s&%s=%s", connParams, v.name, v.value)
33-
}
34-
}
30+
// isSysVarSupported determines if a system variable is supported in given TiDB version
31+
// TODO: Every known sys var should have a minimal supported version and be checked individually. For now we just assume all sys vars are supported since 7.1.0.
32+
func isSysVarSupported(ver util.SemVersion, sysVar string) bool {
33+
return ver.Compare(util.SemVersion{Major: 7, Minor: 1, Patch: 0}) >= 0
3534
}
3635

3736
func executeTpch(action string) {
38-
if action == "run" && driver == "mysql" && tpchConfig.EnableQueryTuning {
39-
appendQueryTuningVarsToConnParams()
40-
}
41-
4237
openDB()
4338
defer closeDB()
4439

@@ -50,6 +45,23 @@ func executeTpch(action string) {
5045
runtime.GOMAXPROCS(maxProcs)
5146
}
5247

48+
if action == "run" && driver == mysqlDriver && tpchConfig.EnableQueryTuning {
49+
serverVer, err := getServerVersion(globalDB)
50+
if err != nil {
51+
panic(fmt.Errorf("get server version failed: %v", err))
52+
}
53+
fmt.Printf("Server version: %s\n", serverVer)
54+
55+
if semVer, ok := util.NewTiDBSemVersion(serverVer); ok {
56+
fmt.Printf("Enabling query tuning for TiDB version %s.\n", semVer.String())
57+
if err := setTiDBQueryTuningVars(globalDB, semVer); err != nil {
58+
panic(fmt.Errorf("set session variables failed: %v", err))
59+
}
60+
} else {
61+
fmt.Printf("Query tuning is enabled(by default) but server version doesn't appear to be TiDB, skipping tuning.\n")
62+
}
63+
}
64+
5365
tpchConfig.PlanReplayerConfig.Host = hosts[0]
5466
tpchConfig.PlanReplayerConfig.StatusPort = statusPort
5567

@@ -67,6 +79,25 @@ func executeTpch(action string) {
6779
w.OutputStats(true)
6880
}
6981

82+
func getServerVersion(db *sql.DB) (string, error) {
83+
var version string
84+
err := db.QueryRow("SELECT VERSION()").Scan(&version)
85+
return version, err
86+
}
87+
88+
func setTiDBQueryTuningVars(db *sql.DB, ver util.SemVersion) error {
89+
for _, v := range queryTuningVars {
90+
if isSysVarSupported(ver, v.name) {
91+
if _, err := db.Exec(fmt.Sprintf("SET SESSION %s = %s", v.name, v.value)); err != nil {
92+
return err
93+
}
94+
} else {
95+
fmt.Printf("Unsupported query tunning var %s for TiDB version %s \n", v.name, ver.String())
96+
}
97+
}
98+
return nil
99+
}
100+
70101
func registerTpch(root *cobra.Command) {
71102
cmd := &cobra.Command{
72103
Use: "tpch",

‎pkg/util/version.go‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
package util
2+
3+
import (
4+
"strconv"
5+
"strings"
6+
)
7+
8+
type SemVersion struct {
9+
Major int
10+
Minor int
11+
Patch int
12+
}
13+
14+
// @version is the `SELECT VERSION()` output of TiDB
15+
func NewTiDBSemVersion(version string) (SemVersion, bool) {
16+
isTiDB := strings.Contains(strings.ToLower(version), "tidb")
17+
if !isTiDB {
18+
return SemVersion{}, false
19+
}
20+
21+
verItems := strings.Split(version, "-v")
22+
if len(verItems) < 2 {
23+
return SemVersion{}, false
24+
}
25+
verStr := strings.Split(verItems[1], "-")[0]
26+
27+
parts := strings.Split(verStr, ".")
28+
if len(parts) < 3 {
29+
return SemVersion{}, false
30+
}
31+
32+
major, err := strconv.Atoi(parts[0])
33+
if err != nil {
34+
return SemVersion{}, false
35+
}
36+
minor, err := strconv.Atoi(parts[1])
37+
if err != nil {
38+
return SemVersion{}, false
39+
}
40+
41+
patch, err := strconv.Atoi(parts[2])
42+
if err != nil {
43+
return SemVersion{}, false
44+
}
45+
46+
return SemVersion{
47+
Major: major,
48+
Minor: minor,
49+
Patch: patch,
50+
}, true
51+
}
52+
53+
func (s SemVersion) String() string {
54+
return strconv.Itoa(s.Major) + "." + strconv.Itoa(s.Minor) + "." + strconv.Itoa(s.Patch)
55+
}
56+
57+
func (s SemVersion) Compare(other SemVersion) int {
58+
signum := func(x int) int {
59+
if x > 0 {
60+
return 1
61+
}
62+
if x < 0 {
63+
return -1
64+
}
65+
return 0
66+
}
67+
68+
if diff := s.Major - other.Major; diff != 0 {
69+
return signum(diff)
70+
}
71+
if diff := s.Minor - other.Minor; diff != 0 {
72+
return signum(diff)
73+
}
74+
if diff := s.Patch - other.Patch; diff != 0 {
75+
return signum(diff)
76+
}
77+
return 0
78+
}

‎pkg/util/version_test.go‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
package util
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
)
8+
9+
func TestNewTiDBSemVersion(t *testing.T) {
10+
testCases := []struct {
11+
name string
12+
input string
13+
expected SemVersion
14+
ok bool
15+
}{
16+
{
17+
name: "normal case with addition",
18+
input: "5.7.25-TiDB-v7.1.0-alpha",
19+
expected: SemVersion{Major: 7, Minor: 1, Patch: 0},
20+
ok: true,
21+
},
22+
{
23+
name: "version without addition",
24+
input: "5.7.25-TiDB-v7.4.1",
25+
expected: SemVersion{Major: 7, Minor: 4, Patch: 1},
26+
ok: true,
27+
},
28+
{
29+
name: "multi-part addition",
30+
input: "5.7.25-TiDB-v6.5.3-beta.2",
31+
expected: SemVersion{Major: 6, Minor: 5, Patch: 3},
32+
ok: true,
33+
},
34+
{
35+
name: "empty addition due to trailing hyphen",
36+
input: "5.7.25-TiDB-v7.1.0-",
37+
expected: SemVersion{Major: 7, Minor: 1, Patch: 0},
38+
ok: true,
39+
},
40+
{
41+
name: "non-tidb database",
42+
input: "MySQL 8.0.35",
43+
expected: SemVersion{},
44+
ok: false,
45+
},
46+
{
47+
name: "missing version prefix",
48+
input: "TiDB-7.2.0",
49+
expected: SemVersion{},
50+
ok: false,
51+
},
52+
{
53+
name: "invalid patch version",
54+
input: "5.7.25-TiDB-v7.1.x",
55+
expected: SemVersion{},
56+
ok: false,
57+
},
58+
{
59+
name: "insufficient version parts",
60+
input: "5.7.25-TiDB-v7.1",
61+
expected: SemVersion{},
62+
ok: false,
63+
},
64+
}
65+
66+
for _, tc := range testCases {
67+
t.Run(tc.name, func(t *testing.T) {
68+
actual, ok := NewTiDBSemVersion(tc.input)
69+
assert.Equal(t, tc.ok, ok, "ok mismatch")
70+
if tc.ok {
71+
assert.Equal(t, tc.expected, actual, "version mismatch")
72+
}
73+
})
74+
}
75+
}
76+
77+
func TestSemVersionCompare(t *testing.T) {
78+
testCases := []struct {
79+
name string
80+
version1 SemVersion
81+
version2 SemVersion
82+
expected int
83+
}{
84+
{
85+
name: "major version greater",
86+
version1: SemVersion{Major: 8, Minor: 0, Patch: 0},
87+
version2: SemVersion{Major: 7, Minor: 5, Patch: 10},
88+
expected: 1,
89+
},
90+
{
91+
name: "major version less",
92+
version1: SemVersion{Major: 6, Minor: 9, Patch: 9},
93+
version2: SemVersion{Major: 7, Minor: 0, Patch: 0},
94+
expected: -1,
95+
},
96+
{
97+
name: "major same, minor greater",
98+
version1: SemVersion{Major: 7, Minor: 2, Patch: 0},
99+
version2: SemVersion{Major: 7, Minor: 1, Patch: 5},
100+
expected: 1,
101+
},
102+
{
103+
name: "major same, minor less",
104+
version1: SemVersion{Major: 7, Minor: 1, Patch: 10},
105+
version2: SemVersion{Major: 7, Minor: 2, Patch: 0},
106+
expected: -1,
107+
},
108+
{
109+
name: "major and minor same, patch greater",
110+
version1: SemVersion{Major: 7, Minor: 1, Patch: 5},
111+
version2: SemVersion{Major: 7, Minor: 1, Patch: 0},
112+
expected: 1,
113+
},
114+
{
115+
name: "major and minor same, patch less",
116+
version1: SemVersion{Major: 7, Minor: 1, Patch: 0},
117+
version2: SemVersion{Major: 7, Minor: 1, Patch: 1},
118+
expected: -1,
119+
},
120+
{
121+
name: "identical versions",
122+
version1: SemVersion{Major: 7, Minor: 1, Patch: 0},
123+
version2: SemVersion{Major: 7, Minor: 1, Patch: 0},
124+
expected: 0,
125+
},
126+
{
127+
name: "extreme version differences",
128+
version1: SemVersion{Major: 10, Minor: 0, Patch: 0},
129+
version2: SemVersion{Major: 1, Minor: 99, Patch: 99},
130+
expected: 1,
131+
},
132+
}
133+
134+
for _, tc := range testCases {
135+
t.Run(tc.name, func(t *testing.T) {
136+
result := tc.version1.Compare(tc.version2)
137+
if result != tc.expected {
138+
t.Errorf("Expected %v.Compare(%v) = %v, got %v",
139+
tc.version1, tc.version2, tc.expected, result)
140+
}
141+
142+
reverseResult := tc.version2.Compare(tc.version1)
143+
expectedReverse := -tc.expected
144+
if reverseResult != expectedReverse {
145+
t.Errorf("Expected %v.Compare(%v) = %v, got %v",
146+
tc.version2, tc.version1, expectedReverse, reverseResult)
147+
}
148+
})
149+
}
150+
}

0 commit comments

Comments
 (0)