Skip to content

Commit

Permalink
refactor: Replace ctx.Done() with ctx.Err() (#19546)
Browse files Browse the repository at this point in the history
* refactor: Replace ctx.Done() with ctx.Err()

Prior to this commit we checked for context cancellation with a select
block and context.Context.Done() without multiplexing over any other
channel like:

  select {
    case <-ctx.Done():
      // handle cancellation
    default:
      // fallthrough
  }

This commit replaces those type of blocks with a simple check of
ctx.Err().  This has the following benefits:

* Calling ctx.Err() is much faster than entering a select block.

* ctx.Done() allocates a channel when called for the first time.

* Testing the result of ctx.Err() is a reliable way of determininging if
  a context.Context value has been canceled.

* fix: Fix data race in execDeleteTagValueEntry()
  • Loading branch information
ayang64 authored Sep 16, 2020
1 parent f1c5c75 commit ca2055c
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 25 deletions.
10 changes: 1 addition & 9 deletions influxql/query/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,15 +311,7 @@ LOOP:
e.Metrics.Requests.WithLabelValues(statusLabel).Inc()

// Check if the query was interrupted during an uninterruptible statement.
interrupted := false
select {
case <-ctx.Done():
interrupted = true
default:
// Query has not been interrupted.
}

if interrupted {
if err := ctx.Err(); err != nil {
statusLabel = control.LabelInterruptedErr
e.Metrics.Requests.WithLabelValues(statusLabel).Inc()
break
Expand Down
6 changes: 2 additions & 4 deletions kv/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,10 +249,8 @@ func WalkCursor(ctx context.Context, cursor ForwardCursor, visit VisitFunc) (err
return err
}

select {
case <-ctx.Done():
return ctx.Err()
default:
if err := ctx.Err(); err != nil {
return err
}
}

Expand Down
8 changes: 4 additions & 4 deletions telemetry/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,10 @@ func (p *Pusher) push(ctx context.Context) error {
req.Header.Set("Content-Type", string(p.PushFormat))

res, err := p.Client.Do(req)
select {
case <-ctx.Done():
return ctx.Err()
default:

// FIXME: consider why we're checking for cancellation here.
if err := ctx.Err(); err != nil {
return err
}

if err != nil {
Expand Down
3 changes: 3 additions & 0 deletions tsdb/index/tsi1/log_file.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ func (f *LogFile) bytes() int {

// Open reads the log from a file and validates all the checksums.
func (f *LogFile) Open() error {
f.mu.Lock()
defer f.mu.Unlock()
if err := f.open(); err != nil {
f.Close()
return err
Expand Down Expand Up @@ -717,6 +719,7 @@ func (f *LogFile) execSeriesEntry(e *LogEntry) {
}

ts.tagValues[string(v)] = tv

mm.tagSet[string(k)] = ts
}

Expand Down
11 changes: 3 additions & 8 deletions v1/coordinator/statement_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,11 +203,8 @@ func (e *StatementExecutor) executeExplainAnalyzeStatement(ctx context.Context,
goto CLEANUP
} else if row == nil {
// Check if the query was interrupted while emitting.
select {
case <-ctx.Done():
err = ctx.Err()
if err = ctx.Err(); err != nil {
goto CLEANUP
default:
}
break
}
Expand Down Expand Up @@ -266,10 +263,8 @@ func (e *StatementExecutor) executeSelectStatement(ctx context.Context, stmt *in
return err
} else if row == nil {
// Check if the query was interrupted while emitting.
select {
case <-ctx.Done():
return ctx.Err()
default:
if err := ctx.Err(); err != nil {
return err
}
break
}
Expand Down

0 comments on commit ca2055c

Please sign in to comment.