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.

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

tso, server, tests: add a test case for tso allocation bug (#2675) #2678

Merged
merged 1 commit into from
Jul 27, 2020

Conversation

ti-srebot
Copy link
Contributor

cherry-pick #2675 to release-4.0


Signed-off-by: JmPotato ghzpotato@gmail.com

What problem does this PR solve?

Previous PR #2665 tries to fix a bug that tso allocation may fail though it's already a PD leader, which lacks of a test case can cover this situation. This pr adds a test case and makes some slight changes than before.

According to these two TiDB jenkins tests, tso may still have some problems when leader changes. I need more logs to figure out why, so this pr also adds some log info when a tso request arrive.

What is changed and how it works?

  • Try to set lease as soon as possible in SyncTimestamp.
  • Add some new methods to Server for test purpose.
  • Add more logs.

Check List

Tests

  • Unit test
  • Integration test

Related changes

Release note

Add more tests and logs for TSO request.

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot added component/testing CI testing. contribution This PR is from a community contributor. status/LGT2 Indicates that a PR has LGTM 2. type/cherry-pick labels Jul 23, 2020
@ti-srebot ti-srebot added this to the v4.0.4 milestone Jul 23, 2020
@ti-srebot ti-srebot self-assigned this Jul 23, 2020
@codecov-commenter
Copy link

Codecov Report

Merging #2678 into release-4.0 will increase coverage by 0.00%.
The diff coverage is 76.92%.

Impacted file tree graph

@@              Coverage Diff              @@
##           release-4.0    #2678    +/-   ##
=============================================
  Coverage        77.16%   77.17%            
=============================================
  Files              204      205     +1     
  Lines            21940    22232   +292     
=============================================
+ Hits             16931    17157   +226     
- Misses            3724     3773    +49     
- Partials          1285     1302    +17     
Impacted Files Coverage Δ
server/api/operator.go 45.40% <ø> (ø)
server/schedule/scheduler.go 69.76% <0.00%> (ø)
server/statistics/store_collection.go 59.70% <ø> (+0.44%) ⬆️
server/util.go 46.91% <0.00%> (-3.09%) ⬇️
server/schedule/operator/step.go 65.40% <41.17%> (-9.78%) ⬇️
server/cluster/store_limiter.go 53.70% <46.15%> (ø)
server/api/store.go 66.32% <58.62%> (+0.98%) ⬆️
server/config/config.go 77.30% <62.50%> (-1.62%) ⬇️
server/handler.go 50.32% <71.42%> (-1.29%) ⬇️
server/schedule/operator_controller.go 81.41% <71.42%> (+0.57%) ⬆️
... and 56 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 096ab27...4de4130. Read the comment docs.

@ti-srebot ti-srebot added status/LGT1 Indicates that a PR has LGTM 1. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Jul 23, 2020
@ti-srebot ti-srebot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 23, 2020
@rleungx
Copy link
Member

rleungx commented Jul 27, 2020

/merge

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

Your auto merge job has been accepted, waiting for:

  • 2669

@ti-srebot
Copy link
Contributor Author

/run-all-tests

@nolouch nolouch merged commit 2f2447a into tikv:release-4.0 Jul 27, 2020
@disksing disksing modified the milestones: v4.0.4, v4.0.5 Jul 28, 2020
@disksing disksing deleted the release-4.0-6dab049720f4 branch July 29, 2020 02:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/testing CI testing. contribution This PR is from a community contributor. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants