-
Notifications
You must be signed in to change notification settings - Fork 85
Conversation
Codecov Report
@@ Coverage Diff @@
## master #128 +/- ##
==========================================
+ Coverage 50.75% 50.78% +0.02%
==========================================
Files 17 17
Lines 1909 1910 +1
==========================================
+ Hits 969 970 +1
- Misses 868 869 +1
+ Partials 72 71 -1 |
v4/export/sql.go
Outdated
func FlushTableWithReadLock(db *sql.DB) error { | ||
_, err := db.Exec("FLUSH TABLES WITH READ LOCK") | ||
func FlushTableWithReadLock(db *sql.Conn) error { | ||
_, err := db.ExecContext(context.Background(), "FLUSH TABLES WITH READ LOCK") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please avoiding context.Background()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed, PTAL again
v4/export/consistency.go
Outdated
serverType ServerType | ||
db *sql.DB | ||
conn *sql.Conn | ||
} | ||
|
||
func (c *ConsistencyFlushTableWithReadLock) Setup() error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about
func (c *ConsistencyFlushTableWithReadLock) Setup() error { | |
func (c *ConsistencyFlushTableWithReadLock) Setup(ctx context.Context) error { |
v4/export/consistency.go
Outdated
} | ||
|
||
type ConsistencyLockDumpingTables struct { | ||
db *sql.DB | ||
ctx context.Context |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tests/consistency/run.sh
Outdated
|
||
# dumping with consistency flush | ||
export DUMPLING_TEST_DATABASE=$DB_NAME | ||
export GO_FAILPOINTS="github.com/pingcap/dumpling/v4/export/ConsistencyCheck=return(\"5s\")" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export GO_FAILPOINTS="github.com/pingcap/dumpling/v4/export/ConsistencyCheck=return(\"5s\")" | |
export GO_FAILPOINTS="github.com/pingcap/dumpling/v4/export/ConsistencyCheck=1*sleep(5000)" |
v4/export/dump.go
Outdated
failpoint.Inject("ConsistencyCheck", func(val failpoint.Value) { | ||
interval, err := time.ParseDuration(val.(string)) | ||
if err != nil { | ||
log.Warn("inject failpoint ConsistencyCheck failed", zap.Reflect("value", val), zap.Error(err)) | ||
} else { | ||
log.Info("start to sleep for failpoint ConsistencyCheck", zap.Duration("sleepTime", interval)) | ||
time.Sleep(interval) | ||
} | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
failpoint.Inject("ConsistencyCheck", func(val failpoint.Value) { | |
interval, err := time.ParseDuration(val.(string)) | |
if err != nil { | |
log.Warn("inject failpoint ConsistencyCheck failed", zap.Reflect("value", val), zap.Error(err)) | |
} else { | |
log.Info("start to sleep for failpoint ConsistencyCheck", zap.Duration("sleepTime", interval)) | |
time.Sleep(interval) | |
} | |
}) | |
failpoint.Inject("consistency-check", nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* fix consistency problem * add consistency integration test * address comment * address comments
* fix consistency problem * add consistency integration test * address comment * address comments
* fix consistency problem * add consistency integration test * address comment * address comments
* fix consistency problem * add consistency integration test * address comment * address comments
* fix consistency problem * add consistency integration test * address comment * address comments
What problem does this PR solve?
unlock tables;
to unlock the FTWRL lock it actually doesn't take effect. Because dumpling usesql.DB
to unlock tables butFTWRL
need to be unlocked by the original connection.consistent snapshot
, so we should record it before FTWRL is released.What is changed and how it works?
Check List
Tests
Side effects
Related changes
Release note