018e8c46-474f-4f6b-80ab-751d576edd29

Committed 29 Mar 2024 10:13PM UTC coverage: 65.249% (-0.01%) from 65.259%

Build # 018e8c46-474f-4f6b-80ab-751d576edd29

Build Type

push

buildkite

Committed by

web-flow

Commit Message

Deadlock fix in acquireShards (#5825)

Fixing the deadlock demonstrated in #5824.

I decided to move the channel-writing entirely before consuming so it's a bit more accidental-change-resistant: some kinds of simple incorrect changes will lead to an _immediate_ deadlock every time, rather than a random chance of one.

And if someone _does_ want to move it after and go back to a smaller buffer, more code will have to be changed, so hopefully people will pay more attention to the concurrency risks involved.

More generally, the atomic shutdown stuff is _highly_ prone to causing this kind of error because there's no way to wait on it safely, and I would really love for us to get rid of it.

Run Details

8 of 8 new or added lines in 1 file covered. (100.0%)

53 existing lines in 12 files now uncovered.

95414 of 146231 relevant lines covered (65.25%)

2380.23 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

79.49

/common/persistence/sql/sqlplugin/mysql/db.go

// Copyright (c) 2017 Uber Technologies, Inc.
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

package mysql

import (
        "context"
        "database/sql"
        "time"

        "github.com/VividCortex/mysqlerr"
        "github.com/go-sql-driver/mysql"
        "github.com/jmoiron/sqlx"

        "github.com/uber/cadence/common/persistence/sql/sqldriver"
        "github.com/uber/cadence/common/persistence/sql/sqlplugin"
)

type (
        db struct {
                converter   DataConverter
                driver      sqldriver.Driver
                originalDBs []*sqlx.DB
                numDBShards int
        }
)

func (mdb *db) GetTotalNumDBShards() int {
        return mdb.numDBShards
}

var _ sqlplugin.AdminDB = (*db)(nil)
var _ sqlplugin.DB = (*db)(nil)
var _ sqlplugin.Tx = (*db)(nil)

func (mdb *db) IsDupEntryError(err error) bool {
        sqlErr, ok := err.(*mysql.MySQLError)
        // ErrDupEntry MySQL Error 1062 indicates a duplicate primary key i.e. the row already exists,
        // so we don't do the insert and return a ConditionalUpdate error.
        return ok && sqlErr.Number == mysqlerr.ER_DUP_ENTRY
}

func (mdb *db) IsNotFoundError(err error) bool {
        return err == sql.ErrNoRows
}

func (mdb *db) IsTimeoutError(err error) bool {
        if err == context.DeadlineExceeded {
                return true
        }
        sqlErr, ok := err.(*mysql.MySQLError)
        if ok {
                if sqlErr.Number == mysqlerr.ER_NET_READ_INTERRUPTED ||
                        sqlErr.Number == mysqlerr.ER_NET_WRITE_INTERRUPTED ||
                        sqlErr.Number == mysqlerr.ER_LOCK_WAIT_TIMEOUT ||
                        sqlErr.Number == mysqlerr.ER_XA_RBTIMEOUT ||
                        sqlErr.Number == mysqlerr.ER_QUERY_TIMEOUT ||
                        sqlErr.Number == mysqlerr.ER_LOCKING_SERVICE_TIMEOUT ||
                        sqlErr.Number == mysqlerr.ER_REGEXP_TIME_OUT {
                        return true
                }
        }
        return false
}

func (mdb *db) IsThrottlingError(err error) bool {
        sqlErr, ok := err.(*mysql.MySQLError)
        if ok {
                if sqlErr.Number == mysqlerr.ER_CON_COUNT_ERROR ||
                        sqlErr.Number == mysqlerr.ER_TOO_MANY_USER_CONNECTIONS ||
                        sqlErr.Number == mysqlerr.ER_TOO_MANY_CONCURRENT_TRXS ||
                        sqlErr.Number == mysqlerr.ER_CLONE_TOO_MANY_CONCURRENT_CLONES {
                        return true
                }
        }
        return false
}

// newDB returns an instance of DB, which is a logical
// connection to the underlying mysql database
// dbShardID is needed when tx is not nil
func newDB(xdbs []*sqlx.DB, tx *sqlx.Tx, dbShardID int, numDBShards int) (*db, error) {
        driver, err := sqldriver.NewDriver(xdbs, tx, dbShardID)
        if err != nil {
                return nil, err
        }

        db := &db{
                converter:   &converter{},
                originalDBs: xdbs, // this is kept because newDB will be called again when starting a transaction
                driver:      driver,
                numDBShards: numDBShards,
        }

        return db, nil
}

// BeginTx starts a new transaction and returns a reference to the Tx object
func (mdb *db) BeginTx(ctx context.Context, dbShardID int) (sqlplugin.Tx, error) {
        xtx, err := mdb.driver.BeginTxx(ctx, dbShardID, nil)
        if err != nil {
                return nil, err
        }
        return newDB(mdb.originalDBs, xtx, dbShardID, mdb.numDBShards)
}

// Commit commits a previously started transaction
func (mdb *db) Commit() error {
        return mdb.driver.Commit()
}

// Rollback triggers rollback of a previously started transaction
func (mdb *db) Rollback() error {
        return mdb.driver.Rollback()
}

// Close closes the connection to the mysql db
func (mdb *db) Close() error {
        return mdb.driver.Close()
}

// PluginName returns the name of the mysql plugin
func (mdb *db) PluginName() string {
        return PluginName
}

// SupportsTTL returns weather MySQL supports TTL
func (mdb *db) SupportsTTL() bool {
        return false
}

// MaxAllowedTTL returns the max allowed ttl MySQL supports
func (mdb *db) MaxAllowedTTL() (*time.Duration, error) {
        return nil, sqlplugin.ErrTTLNotSupported
}

// SupportsTTL returns weather MySQL supports Asynchronous transaction
func (mdb *db) SupportsAsyncTransaction() bool {
        return false
}

1	// Copyright (c) 2017 Uber Technologies, Inc.
2	//
3	// Permission is hereby granted, free of charge, to any person obtaining a copy
4	// of this software and associated documentation files (the "Software"), to deal
5	// in the Software without restriction, including without limitation the rights
6	// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
7	// copies of the Software, and to permit persons to whom the Software is
8	// furnished to do so, subject to the following conditions:
9	//
10	// The above copyright notice and this permission notice shall be included in
11	// all copies or substantial portions of the Software.
12	//
13	// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
14	// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
15	// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
16	// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
17	// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
18	// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
19	// THE SOFTWARE.
20
21	package mysql
22
23	import (
24	"context"
25	"database/sql"
26	"time"
27
28	"github.com/VividCortex/mysqlerr"
29	"github.com/go-sql-driver/mysql"
30	"github.com/jmoiron/sqlx"
31
32	"github.com/uber/cadence/common/persistence/sql/sqldriver"
33	"github.com/uber/cadence/common/persistence/sql/sqlplugin"
34	)
35
36	type (
37	db struct {
38	converter DataConverter
39	driver sqldriver.Driver
40	originalDBs []*sqlx.DB
41	numDBShards int
42	}
43	)
44
45	func (mdb *db) GetTotalNumDBShards() int {	32,254✔
46	return mdb.numDBShards	32,254✔
47	}	32,254✔
48
49	var _ sqlplugin.AdminDB = (*db)(nil)
50	var _ sqlplugin.DB = (*db)(nil)
51	var _ sqlplugin.Tx = (*db)(nil)
52
53	func (mdb *db) IsDupEntryError(err error) bool {	12✔
54	sqlErr, ok := err.(*mysql.MySQLError)	12✔
55	// ErrDupEntry MySQL Error 1062 indicates a duplicate primary key i.e. the row already exists,	12✔
56	// so we don't do the insert and return a ConditionalUpdate error.	12✔
57	return ok && sqlErr.Number == mysqlerr.ER_DUP_ENTRY	12✔
58	}	12✔
59
60	func (mdb *db) IsNotFoundError(err error) bool {	775✔
61	return err == sql.ErrNoRows	775✔
62	}	775✔
63
64	func (mdb *db) IsTimeoutError(err error) bool {	595✔
65	if err == context.DeadlineExceeded {	595✔
66	return true	×
67	}	×
68	sqlErr, ok := err.(*mysql.MySQLError)	595✔
69	if ok {	655✔
70	if sqlErr.Number == mysqlerr.ER_NET_READ_INTERRUPTED \|\|	60✔
71	sqlErr.Number == mysqlerr.ER_NET_WRITE_INTERRUPTED \|\|	60✔
72	sqlErr.Number == mysqlerr.ER_LOCK_WAIT_TIMEOUT \|\|	60✔
73	sqlErr.Number == mysqlerr.ER_XA_RBTIMEOUT \|\|	60✔
74	sqlErr.Number == mysqlerr.ER_QUERY_TIMEOUT \|\|	60✔
75	sqlErr.Number == mysqlerr.ER_LOCKING_SERVICE_TIMEOUT \|\|	60✔
76	sqlErr.Number == mysqlerr.ER_REGEXP_TIME_OUT {	60✔
77	return true	×
78	}	×
79	}
80	return false	595✔
81	}
82
83	func (mdb *db) IsThrottlingError(err error) bool {	595✔
84	sqlErr, ok := err.(*mysql.MySQLError)	595✔
85	if ok {	655✔
86	if sqlErr.Number == mysqlerr.ER_CON_COUNT_ERROR \|\|	60✔
87	sqlErr.Number == mysqlerr.ER_TOO_MANY_USER_CONNECTIONS \|\|	60✔
88	sqlErr.Number == mysqlerr.ER_TOO_MANY_CONCURRENT_TRXS \|\|	60✔
89	sqlErr.Number == mysqlerr.ER_CLONE_TOO_MANY_CONCURRENT_CLONES {	60✔
90	return true	×
91	}	×
92	}
93	return false	595✔
94	}
95
96	// newDB returns an instance of DB, which is a logical
97	// connection to the underlying mysql database
98	// dbShardID is needed when tx is not nil
99	func newDB(xdbs []sqlx.DB, tx sqlx.Tx, dbShardID int, numDBShards int) (*db, error) {	6,110✔
100	driver, err := sqldriver.NewDriver(xdbs, tx, dbShardID)	6,110✔
101	if err != nil {	6,110✔
102	return nil, err	×
103	}	×
104
105	db := &db{	6,110✔
106	converter: &converter{},	6,110✔
107	originalDBs: xdbs, // this is kept because newDB will be called again when starting a transaction	6,110✔
108	driver: driver,	6,110✔
109	numDBShards: numDBShards,	6,110✔
110	}	6,110✔
111		6,110✔
112	return db, nil	6,110✔
113	}
114
115	// BeginTx starts a new transaction and returns a reference to the Tx object
116	func (mdb *db) BeginTx(ctx context.Context, dbShardID int) (sqlplugin.Tx, error) {	6,012✔
117	xtx, err := mdb.driver.BeginTxx(ctx, dbShardID, nil)	6,012✔
118	if err != nil {	6,012✔
UNCOV 119	return nil, err	×
UNCOV 120	}	×
121	return newDB(mdb.originalDBs, xtx, dbShardID, mdb.numDBShards)	6,012✔
122	}
123
124	// Commit commits a previously started transaction
125	func (mdb *db) Commit() error {	5,851✔
126	return mdb.driver.Commit()	5,851✔
127	}	5,851✔
128
129	// Rollback triggers rollback of a previously started transaction
130	func (mdb *db) Rollback() error {	162✔
131	return mdb.driver.Rollback()	162✔
132	}	162✔
133
134	// Close closes the connection to the mysql db
135	func (mdb *db) Close() error {	99✔
136	return mdb.driver.Close()	99✔
137	}	99✔
138
139	// PluginName returns the name of the mysql plugin
140	func (mdb *db) PluginName() string {	1,943✔
141	return PluginName	1,943✔
142	}	1,943✔
143
144	// SupportsTTL returns weather MySQL supports TTL
145	func (mdb *db) SupportsTTL() bool {	3,386✔
146	return false	3,386✔
147	}	3,386✔
148
149	// MaxAllowedTTL returns the max allowed ttl MySQL supports
150	func (mdb db) MaxAllowedTTL() (time.Duration, error) {	×
151	return nil, sqlplugin.ErrTTLNotSupported	×
152	}	×
153
154	// SupportsTTL returns weather MySQL supports Asynchronous transaction
155	func (mdb *db) SupportsAsyncTransaction() bool {	×
156	return false	×
157	}	×

uber / cadence / 018e8c46-474f-4f6b-80ab-751d576edd29

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous