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

*: fix context usage for SQL operation (#377) #400

Merged
merged 12 commits into from
Dec 9, 2019

Conversation

csuzhangxc
Copy link
Member

@csuzhangxc csuzhangxc commented Dec 4, 2019

cherry pick #377 manually before merged for testing.


What problem does this PR solve?

when retrying SQL operation to downstream, stop-task / pause-task may not take effect because of the misuse of context.

What is changed and how it works?

fix context usage for SQL operation to downstream, including:

  • data migration
  • checkpoint update
  • online DDL meta update

NOTE:

  • shard DDL meta operation not updated in this PR because we are refactoring it
  • better improvement for these operations can be done when moving them to etcd.
  • retry_cancel integration testing has much wait time, and I'll split it run separately in CI after received 2 LGTM.

Check List

Tests

  • Unit test
  • Integration test

Side effects

  • Increased code complexity

Related changes

  • Need to cherry-pick to the release branch
  • Need to be included in the release note

@csuzhangxc csuzhangxc added the status/DNM Do not merge, test is failing or blocked by another PR label Dec 4, 2019
@csuzhangxc
Copy link
Member Author

/run-all-tests tidb=release-3.0

@codecov
Copy link

codecov bot commented Dec 4, 2019

Codecov Report

Merging #400 into release-1.0 will not change coverage.
The diff coverage is n/a.

@@              Coverage Diff              @@
##           release-1.0      #400   +/-   ##
=============================================
  Coverage       57.179%   57.179%           
=============================================
  Files              159       159           
  Lines            16172     16172           
=============================================
  Hits              9247      9247           
  Misses            6017      6017           
  Partials           908       908

@csuzhangxc
Copy link
Member Author

/run-all-tests tidb=release-3.0

@csuzhangxc
Copy link
Member Author

/run-all-tests tidb=release-3.0

@csuzhangxc
Copy link
Member Author

@WangXiangUSTC @GregoryIan PTAL

@WangXiangUSTC
Copy link
Contributor

LGTM

@WangXiangUSTC WangXiangUSTC added the status/LGT1 One reviewer already commented LGTM label Dec 9, 2019
@csuzhangxc
Copy link
Member Author

/run-all-tests tidb=release-3.0

Copy link
Contributor

@WangXiangUSTC WangXiangUSTC left a comment

Choose a reason for hiding this comment

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

LGTM

@csuzhangxc csuzhangxc merged commit 55ab839 into pingcap:release-1.0 Dec 9, 2019
@csuzhangxc csuzhangxc deleted the pick-377 branch December 9, 2019 12:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status/DNM Do not merge, test is failing or blocked by another PR status/LGT1 One reviewer already commented LGTM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants