Skip to content

Commit

Permalink
store: fix potential panic in GC worker (pingcap#14403)
Browse files Browse the repository at this point in the history
  • Loading branch information
jackysp authored and imtbkcat committed Jan 17, 2020
1 parent 7b49e43 commit 66c23c9
Showing 1 changed file with 7 additions and 17 deletions.
24 changes: 7 additions & 17 deletions store/tikv/gcworker/gc_worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ type GCWorker struct {
lastFinish time.Time
cancel context.CancelFunc
done chan error

session session.Session
}

// NewGCWorker creates a GCWorker instance.
Expand Down Expand Up @@ -136,8 +134,6 @@ func (w *GCWorker) start(ctx context.Context, wg *sync.WaitGroup) {
logutil.Logger(ctx).Info("[gc worker] start",
zap.String("uuid", w.uuid))

w.session = createSession(w.store)

w.tick(ctx) // Immediately tick once to initialize configs.
wg.Done()

Expand Down Expand Up @@ -864,7 +860,6 @@ func (w *GCWorker) checkLeader() (bool, error) {
if err != nil {
return false, errors.Trace(err)
}
w.session = se
leader, err := w.loadValueFromSysTable(gcLeaderUUIDKey)
if err != nil {
_, err1 := se.Execute(ctx, "ROLLBACK")
Expand Down Expand Up @@ -895,6 +890,7 @@ func (w *GCWorker) checkLeader() (bool, error) {
}
lease, err := w.loadTime(gcLeaderLeaseKey)
if err != nil {
se.RollbackTxn(ctx)
return false, errors.Trace(err)
}
if lease == nil || lease.Before(time.Now()) {
Expand Down Expand Up @@ -998,8 +994,10 @@ func (w *GCWorker) loadDurationWithDefault(key string, def time.Duration) (*time

func (w *GCWorker) loadValueFromSysTable(key string) (string, error) {
ctx := context.Background()
se := createSession(w.store)
defer se.Close()
stmt := fmt.Sprintf(`SELECT HIGH_PRIORITY (variable_value) FROM mysql.tidb WHERE variable_name='%s' FOR UPDATE`, key)
rs, err := w.session.Execute(ctx, stmt)
rs, err := se.Execute(ctx, stmt)
if len(rs) > 0 {
defer terror.Call(rs[0].Close)
}
Expand Down Expand Up @@ -1028,10 +1026,9 @@ func (w *GCWorker) saveValueToSysTable(key, value string) error {
ON DUPLICATE KEY
UPDATE variable_value = '%[2]s', comment = '%[3]s'`,
key, value, gcVariableComments[key])
if w.session == nil {
return errors.New("[saveValueToSysTable session is nil]")
}
_, err := w.session.Execute(context.Background(), stmt)
se := createSession(w.store)
defer se.Close()
_, err := se.Execute(context.Background(), stmt)
logutil.Logger(context.Background()).Debug("[gc worker] save kv",
zap.String("key", key),
zap.String("value", value),
Expand Down Expand Up @@ -1084,13 +1081,6 @@ func NewMockGCWorker(store tikv.Storage) (*MockGCWorker, error) {
lastFinish: time.Now(),
done: make(chan error),
}
worker.session, err = session.CreateSession(worker.store)
if err != nil {
logutil.Logger(context.Background()).Error("initialize MockGCWorker session fail", zap.Error(err))
return nil, errors.Trace(err)
}
privilege.BindPrivilegeManager(worker.session, nil)
worker.session.GetSessionVars().InRestrictedSQL = true
return &MockGCWorker{worker: worker}, nil
}

Expand Down

0 comments on commit 66c23c9

Please sign in to comment.