Skip to content

Commit

Permalink
executor: fix data race in GetDirtyTable() (pingcap#12767) (pingcap…
Browse files Browse the repository at this point in the history
  • Loading branch information
lzmhhh123 authored and sre-bot committed Nov 4, 2019
1 parent 4a6c705 commit 418023f
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 2 deletions.
5 changes: 3 additions & 2 deletions executor/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -791,8 +791,9 @@ func (b *executorBuilder) buildUnionScanFromReader(reader Executor, v *plannerco
// GetDirtyDB() is safe here. If this table has been modified in the transaction, non-nil DirtyTable
// can be found in DirtyDB now, so GetDirtyTable is safe; if this table has not been modified in the
// transaction, empty DirtyTable would be inserted into DirtyDB, it does not matter when multiple
// goroutines write empty DirtyTable to DirtyDB for this table concurrently. Thus we don't use lock
// to synchronize here.
// goroutines write empty DirtyTable to DirtyDB for this table concurrently. Although the DirtyDB looks
// safe for data race in all the cases, the map of golang will throw panic when it's accessed in parallel.
// So we lock it when getting dirty table.
us.dirty = GetDirtyDB(b.ctx).GetDirtyTable(x.table.Meta().ID)
us.conditions = v.Conditions
us.columns = x.columns
Expand Down
7 changes: 7 additions & 0 deletions executor/union_scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package executor

import (
"sort"
"sync"
"time"

"github.com/pingcap/errors"
Expand All @@ -29,6 +30,8 @@ import (
// DirtyDB stores uncommitted write operations for a transaction.
// It is stored and retrieved by context.Value and context.SetValue method.
type DirtyDB struct {
sync.Mutex

// tables is a map whose key is tableID.
tables map[int64]*DirtyTable
}
Expand Down Expand Up @@ -60,6 +63,9 @@ func (udb *DirtyDB) TruncateTable(tid int64) {

// GetDirtyTable gets the DirtyTable by id from the DirtyDB.
func (udb *DirtyDB) GetDirtyTable(tid int64) *DirtyTable {
// The index join access the tables map parallelly.
// But the map throws panic in this case. So it's locked.
udb.Lock()
dt, ok := udb.tables[tid]
if !ok {
dt = &DirtyTable{
Expand All @@ -68,6 +74,7 @@ func (udb *DirtyDB) GetDirtyTable(tid int64) *DirtyTable {
}
udb.tables[tid] = dt
}
udb.Unlock()
return dt
}

Expand Down

0 comments on commit 418023f

Please sign in to comment.