Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

store/tikv,mysql: improve error message of GC life time #7658

Merged
merged 4 commits into from
Sep 11, 2018

Conversation

tiancaiamao
Copy link
Contributor

What problem does this PR solve?

Fix #7650

What is changed and how it works?

Print the transaction start time and GC safe time.

When GC was happened is not easy to know, because the gcworker may not run in current TiDB.

Check List

Tests

  • No code

@coocood

store/tikv/kv.go Outdated
@@ -165,7 +165,7 @@ func (s *tikvStore) CheckVisibility(startTime uint64) error {
}

if startTime < cachedSafePoint {
return ErrGCTooEarly
return ErrGCTooEarly.GenByArgs(startTime, cachedSafePoint)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are startTime and cachedSafePoint human readable?

Copy link
Contributor

@winkyao winkyao Sep 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

startTime is tso, and the safetime is calculated in GCWorker.prepare, also tso, so both is not readable.

@coocood
Copy link
Member

coocood commented Sep 10, 2018

@tiancaiamao
We better use the physical time for the log.

@tiancaiamao
Copy link
Contributor Author

PTAL @shenli @coocood

@shenli
Copy link
Member

shenli commented Sep 11, 2018

LGTM

@shenli
Copy link
Member

shenli commented Sep 11, 2018

/run-all-tests

@shenli shenli added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 11, 2018
Copy link
Contributor

@winkyao winkyao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

winkyao
winkyao previously approved these changes Sep 11, 2018
@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@shenli shenli added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Sep 11, 2018
@coocood coocood merged commit 2c26243 into pingcap:master Sep 11, 2018
@tiancaiamao tiancaiamao deleted the gc-life-time branch September 11, 2018 04:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/GC status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants