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

BounceMemberRule logging & fixes, smaller cluster in QueryBounceTest #10181

Merged
merged 2 commits into from Apr 20, 2017

Conversation

vbekiaris
Copy link
Contributor

  • Smaller cluster and test drivers size (4 member & 4 test drivers instead of 6 & 5 respectively) in QueryBounceTest to reduce probability of timeout. In local debugging, sometimes initial cluster was formed after 1 minute which does count towards the timeout defined in @Test(timeout=...).
  • Added logging in BounceMemberRule for easier troubleshooting
  • Bounced members and test drivers are now accessed with volatile semantics, as they are used by several threads:
    • Cluster setup is executed in the thread that initializes the AbstractHazelcastClassRunner (eg when running within the IDE this is the main thread)
    • The FailOnTimeoutStatement that wraps every test to enforce the timeout spawns a separate thread that invokes the test method. BounceMemberRule.test* will be invoked from this thread and will typically need to obtain at least a test driver HazelcastInstance
    • Another thread is spawned from BounceMemberRule that executes the member bouncing logic and updates the current cluster members in BounceMemberRule.members

@devOpsHazelcast
Copy link
Collaborator

@devOpsHazelcast
Copy link
Collaborator

Test PASSed.

@@ -363,25 +388,33 @@ public void evaluate() throws Throwable {

// wait until all test tasks complete or one of them throws an exception
private void waitForFutures(Future[] futures) {
// do not wait more than 30 seconds
long deadline = currentTimeMillis() + 30 * 1000;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would make the timout a constant like TIMEOUT_SECONDS and use TimeUnit.Seconds.toMillis(TIMEOUT_SECONDS) here (and of course use the constant in the comment and warning log below as well).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thx, fixed

@vbekiaris
Copy link
Contributor Author

  • Member bouncing thread is now a daemon thread.
  • Fixes timeouts in QueryBounceTest & ClientQueryBounceTest [query] ClientQueryBounceTest.testQuery #10107 & [query] QueryBounceTest.testQueryWithIndexes #9870 : sometimes HazelcastInstance.getMap in the QueryRunnable constructor could take minutes to obtain a reference while members are bouncing; this time was counted against the test method's timeout but not included in the bouncing test duration, so the 3 minutes of bouncing test duration would start minutes after the FailOnTimeoutStatement started counting the default 5-minute @Test method timeout.

@devOpsHazelcast
Copy link
Collaborator

Test PASSed.

- Smaller cluster and test drivers size in QueryBounceTest to reduce probability of timeout
- Added logging in BounceMemberRule for easier troubleshooting
- Bounced members and test drivers with volatile read/write, as they are accessed by several threads
- Bounce member thread is now a daemon thread
@vbekiaris vbekiaris force-pushed the fixes/3.9/query-bounce-test branch from e50de90 to 5c4ca03 Compare April 5, 2017 18:57
@devOpsHazelcast
Copy link
Collaborator

@devOpsHazelcast
Copy link
Collaborator

Test PASSed.

@mdogan
Copy link
Contributor

mdogan commented Apr 20, 2017

run-lab-run

Copy link
Contributor

@mdogan mdogan left a comment

Choose a reason for hiding this comment

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

looks good 👍

@devOpsHazelcast
Copy link
Collaborator

Test PASSed.

@mdogan mdogan merged commit 379f7df into hazelcast:master Apr 20, 2017
@vbekiaris vbekiaris deleted the fixes/3.9/query-bounce-test branch April 20, 2017 12:10
@vbekiaris
Copy link
Contributor Author

thanks @mdogan @Donnerbart

@mmedenjak mmedenjak added the Source: Internal PR or issue was opened by an employee label Apr 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Source: Internal PR or issue was opened by an employee Team: Core Type: Defect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants