gc_worker: fix serveral bugs of green gc #16413
Conversation
Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>
youjiali1995
added
type/bugfix
| // The store is checked and has been resolved before. | ||
| if _, ok := dirtyStores[store]; !ok { | ||
| delete(stores, store) | ||
| } |
There was a problem hiding this comment.
resolved stores may become dirty again.
There was a problem hiding this comment.
Yes. It will be scanned and resolved again.
store/tikv/gcworker/gc_worker.go
Outdated
| break | ||
| } | ||
| // Clear all collected locks of dirty stores to prevent it from being dirty again. | ||
| w.removeLockObservers(ctx, safePoint, dirtyStores) |
There was a problem hiding this comment.
I found we can't do it. If the store recevies duplicated removeLockObserver and registerLockObserver between L1061 and L1071. The store is cleaned but the lock observer drops some locks.
There was a problem hiding this comment.
Maybe we can't retry here.
There was a problem hiding this comment.
Maybe we shouldn't do "remove and re-register" on stores which fail to be checked. Instead, can we just register for those newly added stores?
There was a problem hiding this comment.
Yes. If dirty stores are newly added, we can retry. If a store is dirty due to too many collected locks, we don't retry and falls back to legacy mode.
Codecov Report
@@ Coverage Diff @@
## master #16413 +/- ##
===========================================
Coverage 80.4461% 80.4461%
===========================================
Files 507 507
Lines 137676 137676
===========================================
Hits 110755 110755
Misses 18241 18241
Partials 8680 8680 |
Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>
youjiali1995
force-pushed
the
fix-physical-resolve-lock
branch
2 times, most recently
from
e4a7fc2 to
3d21cae
Compare
youjiali1995
force-pushed
the
fix-physical-resolve-lock
branch
from
3d21cae to
b6a606d
Compare
…li1995/tidb into fix-physical-resolve-lock
Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>
|
/run-all-tests |
…sical-resolve-lock Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>
|
/merge |
sre-bot
added
the
status/can-merge
|
/run-all-tests |
|
@youjiali1995 merge failed. |
|
/merge |
|
Your auto merge job has been accepted, waiting for:
|
|
/run-all-tests |
|
@youjiali1995 merge failed. |
Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>
|
/merge |
|
/run-all-tests |
|
@youjiali1995 merge failed. |
|
/run-unit-test |
|
/merge |
|
/run-all-tests |
|
cherry pick to release-4.0 in PR #16949 |
Signed-off-by: sre-bot <sre-bot@pingcap.com> Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>
Signed-off-by: youjiali1995 zlwgx1023@gmail.com
What problem does this PR solve?
Issue Number: close #16306
Problem Summary:
What is changed and how it works?
What's Changed:
It's simple. Please refer to the codes.
Related changes
Check List
Tests
Side effects
Release note
Fix the issue that green GC may leave unresolved locks if there are dirty stores.