stability: retry truncating sst files upon failure #484
Conversation
sst files may be deleted after compaction
tests/pkg/ops/tikv.go
Outdated
| stdout, stderr, err := exec("find", "/var/lib/tikv/db", "-name", "*.sst", "-o", "-name", "*.save") | ||
| if err != nil { | ||
| glog.Errorf("list sst files: stderr=%s err=%s", stderr, err.Error()) | ||
| return errors.Annotate(err, "list sst files") |
There was a problem hiding this comment.
| return errors.Annotate(err, "list sst files") | |
| continue |
tests/pkg/ops/tikv.go
Outdated
| } | ||
| } | ||
| if len(sst) == 0 { | ||
| return errors.New("cannot find a sst file") |
There was a problem hiding this comment.
| return errors.New("cannot find a sst file") | |
| glog.Error("cannot find a sst file") | |
| continue |
| return errors.Annotate(err, "list sst files") | ||
| } | ||
| retryCount := 0 | ||
| for ; retryCount < retryLimit; retryCount++ { |
There was a problem hiding this comment.
| for ; retryCount < retryLimit; retryCount++ { | |
| for ; retryCount < retryLimit; retryCount++ { | |
| time.Sleep(10*time.Second) |
|
|
||
| sstCandidates := make(map[string]bool) | ||
| sstCandidates := make(map[string]bool) |
There was a problem hiding this comment.
the one more indent because it's in retry loop now.
| glog.Errorf("truncate sst file: stderr=%s err=%s", stderr, err.Error()) | ||
| return errors.Annotate(err, "truncate sst file") | ||
| if retryCount == retryLimit { | ||
| return errors.New("failed to truncate sst file after " + strconv.Itoa(retryLimit) + " trials") |
There was a problem hiding this comment.
the error log needs opts.Namespace and opts.Cluster field
There was a problem hiding this comment.
I added annotations to log methods. however, it's caller who pass the opts arg, that is, the caller must know these info, so I don't think there is a need for adding these fields to returned error.
There was a problem hiding this comment.
but the caller has not added these fields to error log too
ref: https://github.com/pingcap/tidb-operator/blob/master/tests/failover.go#L74-L81
There was a problem hiding this comment.
I added some error logs to the case func. If you think every error should be annotated with additional fields, it would be better to create an another PR to do it, because most of methods of operatorActions failed to do so.
|
/run-e2e-tests |
zyguan
force-pushed
the
retry-truncate-sst
branch
from
56c7f9a
to
4bd7fba
Compare
|
/run-e2e-tests |
|
/run-e2e-tests |
|
@zyguan PTAL |
|
/run-e2e-tests |
|
/run-e2e-tests |
What problem does this PR solve?
sst files may be deleted after compaction, thus the
TruncateSSTFilemay fail unexpectly.This pr closes: #501
What is changed and how it works?
add retry logical.
Check List
Tests
Code changes
Side effects
Related changes
Does this PR introduce a user-facing change?: