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

test: Fix unstable integration test test_auto_gc #5212

Merged
merged 3 commits into from Aug 8, 2019

Conversation

@MyonKeminta
Copy link
Contributor

commented Aug 6, 2019

Signed-off-by: MyonKeminta MyonKeminta@users.noreply.github.com

What have you changed? (mandatory)

Before #4641 , GCManager will wait for the first safe point greater than 0 during initializing. #4641 removed the waiting. However in the integration test test_auto_gc, the correct timing depends on the waiting, which means, #4641 makes test_auto_gc probably fails randomly, which is exactly #5206 . The reason of failing is that the GCManagers may initialize with safepoint 0, then after pd_client.set_gc_safe_point(1), it will trigger a round of GC, which will invoke the callback and push messages to the channel finish_signal_rx. Then after pd_client.set_gc_safe_point(150) and receiving for the channel, it may receive extra signals, while the actual GC with safepoint 150 haven't been done yet, causing assertion failure.

This PR fixes it by:

  • Remove the pd_client.set_gc_safe_point(1), which is useless after #4641 ;
  • In gc_worker, moves calling to initialize before starting GCManager thread, so in the test the initialization will be synchronized with the main thread.

Therefore the timing between set_gc_safe_point, GCManager's initialization and GC will be fixed, which is expected able to make test_auto_gc stable.

GCWorker needs further refactorying. This PR is intended to fix the unstable test quickly.

What are the type of the changes? (mandatory)

  • Bug fix (change which fixes an issue)

How has this PR been tested? (mandatory)

By CI

Does this PR affect documentation (docs) or release note? (mandatory)

No

Does this PR affect tidb-ansible update? (mandatory)

No

Refer to a related PR or issue link (optional)

Fixes #5206
Introduced by #4641

test: Fix unstable integration test test_auto_gc
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@MyonKeminta

This comment has been minimized.

Copy link
Contributor Author

commented Aug 6, 2019

/run-all-tests

@siddontang siddontang requested review from youjiali1995 and sticnarf and removed request for siddontang and youjiali1995 Aug 6, 2019

@MyonKeminta

This comment has been minimized.

Copy link
Contributor Author

commented Aug 7, 2019

/test

@AndreMouche
Copy link
Member

left a comment

LGTM

@MyonKeminta

This comment has been minimized.

Copy link
Contributor Author

commented Aug 7, 2019

/run-all-tests

@youjiali1995

This comment has been minimized.

Copy link
Contributor

commented Aug 8, 2019

/run-auto-merge

@sre-bot

This comment has been minimized.

Copy link
Collaborator

commented Aug 8, 2019

/run-all-tests

@sre-bot sre-bot merged commit 5f4a59a into tikv:master Aug 8, 2019

5 checks passed

DCO All commits are signed off!
Details
idc-jenkins-ci-tikv/integration-common-test Jenkins job succeeded.
Details
idc-jenkins-ci-tikv/integration-compatibility-test Jenkins job succeeded.
Details
idc-jenkins-ci-tikv/integration-ddl-test Jenkins job succeeded.
Details
idc-jenkins-ci/test Jenkins job succeeded.
Details
@sre-bot

This comment has been minimized.

Copy link
Collaborator

commented Aug 8, 2019

cherry pick to release-3.0 failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.