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.

Sign up for GitHub

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

Jump to bottom

ddl: fix mix DDLs blocks 2*lease on unistore #18468

Merged
merged 3 commits into from
Jul 14, 2020
Merged

ddl: fix mix DDLs blocks 2*lease on unistore #18468

merged 3 commits into from
Jul 14, 2020

Conversation

zimulala
Copy link
Contributor

@zimulala zimulala commented Jul 10, 2020 •
edited
Loading

What problem does this PR solve?

Issue Number: close #18456

Do the test in #18456, this will block 90s(2*lease), and it means OwnerCheckAllVersions returns "context deadline exceeded". And this problem will happen on unistore, not on TiKV.
Screen Shot 2020-07-10 at 12 26 50

Problem Summary:
On unistore we use MockSchemaSyncer, we need to do MockSchemaSyncer.OwnerCheckAllVersions like schemaVersionSyncer.OwnerCheckAllVersions.

Check List

Tests

Release note

@zimulala zimulala added the sig/sql-infra SIG: SQL Infra label Jul 10, 2020
@codecov
Copy link

codecov bot commented Jul 10, 2020 •
edited
Loading

Codecov Report

Merging #18468 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #18468   +/-   ##
===========================================
  Coverage   79.5501%   79.5501%           
===========================================
  Files           541        541           
  Lines        146676     146676           
===========================================
  Hits         116681     116681           
  Misses        20695      20695           
  Partials       9300       9300

wjhuang2016
wjhuang2016 reviewed Jul 10, 2020
View reviewed changes
Copy link
Member

@wjhuang2016 wjhuang2016 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot
Copy link
Contributor

@wjhuang2016,Thanks for your review.

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 10, 2020
crazycs520
crazycs520 approved these changes Jul 13, 2020
View reviewed changes
Copy link
Contributor

@crazycs520 crazycs520 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Jul 13, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Jul 13, 2020
@ti-srebot
Copy link
Contributor

@crazycs520,Thanks for your review.

@crazycs520
Copy link
Contributor

/run-all-tests

@zimulala zimulala added the status/can-merge Indicates a PR has been approved by a committer. label Jul 14, 2020
@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@zimulala merge failed.

@zimulala
Copy link
Contributor Author

zimulala commented Jul 14, 2020 •
edited
Loading

UT failed, related to #18526

/run-unit-test

@zimulala zimulala merged commit 61c33d1 into pingcap:master Jul 14, 2020
@zimulala zimulala deleted the mix-ddl branch July 14, 2020 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wjhuang2016 wjhuang2016 wjhuang2016 left review comments

@crazycs520 crazycs520 crazycs520 approved these changes

@ti-srebot ti-srebot ti-srebot approved these changes

Assignees
No one assigned
Labels
sig/sql-infra SIG: SQL Infra status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

create index concurrently lead to block
4 participants
@zimulala @ti-srebot @crazycs520 @wjhuang2016