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

Quality reinforcement (8): fix unstable test cases#1351

Merged
lance6716 merged 7 commits into
masterfrom
fixTest
Jan 26, 2021
Merged

Quality reinforcement (8): fix unstable test cases#1351
lance6716 merged 7 commits into
masterfrom
fixTest

Conversation

@lance6716
Copy link
Copy Markdown
Collaborator

@lance6716 lance6716 commented Dec 21, 2020

What problem does this PR solve?

@lance6716 lance6716 added needs-cherry-pick-release-2.0 This PR should be cherry-picked to release-2.0. Remove this label after cherry-picked to release-2.0 type/test Changes related to test cases labels Dec 23, 2020

# resume manually
# this operation may not return 3 `"result": true`, because worker may
# - response too slowly to resume, so resume still see old error and waitOperationOk will return early
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

that's not a "declarative command", and I think the right way of waitOperationOk of resuming is resuming at least once and see that result. Our persistent data in etcd may not support this change

Comment thread relay/relay.go

if needSavePos {
err = r.SaveMeta(lastPos, lastGTID)
err = r.SaveMeta(lastPos, lastGTID.Clone())
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Possible performance regression. But I have no idea.

Copy link
Copy Markdown
Collaborator Author

@lance6716 lance6716 Jan 20, 2021

Choose a reason for hiding this comment

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

How about we merging this PR now? I met this DATA RACE again

@lance6716
Copy link
Copy Markdown
Collaborator Author

seems "some unit test using etcd jenkins" is failed because etcd too slow

[2021-01-11T08:54:57.388Z] [2021/01/11 16:53:08.828 +08:00] [WARN] [util.go:163] ["apply request took too long"] [took=1.630559395s] [expected-duration=100ms] [prefix=] [request="header:<ID:14726057132519571993 > txn:<compare:<target:CREATE key:\"unit-test/election-evict-leader/4c5d76f0a5efc205\" create_revision:0 > success:<request_put:<key:\"unit-test/election-evict-leader/4c5d76f0a5efc205\" value_size:40 lease:5502685095664796165 >> failure:<request_range:<key:\"unit-test/election-evict-leader/4c5d76f0a5efc205\" > >>"] [response=size:14] []
...
[2021-01-11T08:54:57.388Z] [2021/01/11 16:53:08.830 +08:00] [INFO] [election.go:292] ["get response from election observe"] [component=election] [key=unit-test/election-evict-leader/4c5d76f0a5efc205] [value="{\"id\":\"member1\",\"addr\":\"127.0.0.1:1234\"}"]

putting election key spends more than 1 second, so in the test sleeping 1 second and checking the leader is not enough

Copy link
Copy Markdown
Contributor

@zier-one zier-one left a comment

Choose a reason for hiding this comment

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

LGTM

@zier-one
Copy link
Copy Markdown
Contributor

/lgtm

@ti-srebot ti-srebot added the status/LGT1 One reviewer already commented LGTM label Jan 25, 2021
Copy link
Copy Markdown
Contributor

@3pointer 3pointer left a comment

Choose a reason for hiding this comment

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

LGTM

@lance6716 lance6716 merged commit 59b0b62 into master Jan 26, 2021
ti-srebot pushed a commit to ti-srebot/dm that referenced this pull request Jan 26, 2021
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Copy Markdown

cherry pick to release-2.0 in PR #1402

@ti-srebot ti-srebot added already-cherry-pick-2.0 The related PR is already cherry-picked to release-2.0. Add this label once the PR is cherry-picked and removed needs-cherry-pick-release-2.0 This PR should be cherry-picked to release-2.0. Remove this label after cherry-picked to release-2.0 labels Jan 26, 2021
lance6716 pushed a commit that referenced this pull request Jan 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

already-cherry-pick-2.0 The related PR is already cherry-picked to release-2.0. Add this label once the PR is cherry-picked status/LGT1 One reviewer already commented LGTM type/test Changes related to test cases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants