Skip to content
This repository has been archived by the owner on Aug 21, 2023. It is now read-only.

fix concurrency problem #117

Merged
merged 4 commits into from
Jul 10, 2020
Merged

Conversation

lichunzhu
Copy link
Contributor

What problem does this PR solve?

  1. Now we get tables before we flush all tables, it's not safe when there are tables concurrently written.
  2. Support record show slave status info in metadata for mysql/mariaDB.
  3. Fix the problem that can't set snapshot lock when server type is tidb.

What is changed and how it works?

  1. For consistency flush: lock tables -> record metadata -> get tables list
    For consistency lock: record metadata -> get tables list -> lock tables
  2. Add metadata support in metadata.go.
  3. Check whether consistency is snapshot before add snapshot session variable.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
    Manually check metadata for tidb/mysql. Looks fine to me.

Side effects

  • Increased code complexity

Release note

  • Fix the bug that flush tables may not work for tables to dump

@lichunzhu lichunzhu changed the title Fix concurrency problem fix concurrency problem Jul 8, 2020
@codecov
Copy link

codecov bot commented Jul 8, 2020

Codecov Report

Merging #117 into master will increase coverage by 1.18%.
The diff coverage is 56.43%.

@@            Coverage Diff             @@
##           master     #117      +/-   ##
==========================================
+ Coverage   50.76%   51.94%   +1.18%     
==========================================
  Files          17       17              
  Lines        1775     1829      +54     
==========================================
+ Hits          901      950      +49     
- Misses        811      813       +2     
- Partials       63       66       +3     

Comment on lines 13 to 17
type globalMetadata struct {
logFile string
pos string
gtidSet string
buffer bytes.Buffer

filePath string
startTime time.Time
finishTime time.Time
filePath string
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

why change this to immediately write to a buffer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because I think it's clearer.. WDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Heh ok.

Copy link
Collaborator

@kennytm kennytm left a comment

Choose a reason for hiding this comment

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

rest lgtm

v4/export/metadata.go Outdated Show resolved Hide resolved
@kennytm kennytm merged commit dc5af4e into pingcap:master Jul 10, 2020
@lichunzhu lichunzhu deleted the fixConcurrencyProblem branch July 10, 2020 03:06
tisonkun pushed a commit to tisonkun/dumpling that referenced this pull request Oct 20, 2021
* resolve consistency problem

* support dump follower status for mysql and mariaDB

* revise metadata save point

* address comment
tisonkun pushed a commit to tisonkun/dumpling that referenced this pull request Oct 20, 2021
* resolve consistency problem

* support dump follower status for mysql and mariaDB

* revise metadata save point

* address comment
tisonkun pushed a commit to tisonkun/dumpling that referenced this pull request Oct 20, 2021
* resolve consistency problem

* support dump follower status for mysql and mariaDB

* revise metadata save point

* address comment
tisonkun pushed a commit to tisonkun/dumpling that referenced this pull request Oct 20, 2021
* resolve consistency problem

* support dump follower status for mysql and mariaDB

* revise metadata save point

* address comment
tisonkun pushed a commit to tisonkun/tidb that referenced this pull request Oct 20, 2021
* resolve consistency problem

* support dump follower status for mysql and mariaDB

* revise metadata save point

* address comment
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants