Skip to content

GEODE-4312: Wait for rebalancing to complete as part of teardown#1302

Merged
jdeppe-pivotal merged 3 commits intoapache:developfrom
jdeppe-pivotal:feature/GEODE-4312
Jan 19, 2018
Merged

GEODE-4312: Wait for rebalancing to complete as part of teardown#1302
jdeppe-pivotal merged 3 commits intoapache:developfrom
jdeppe-pivotal:feature/GEODE-4312

Conversation

@jdeppe-pivotal
Copy link
Contributor

Thank you for submitting a contribution to Apache Geode.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

  • Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?

  • Has your PR been rebased against the latest commit within the target branch (typically develop)?

  • Is your initial contribution a single, squashed commit?

  • Does gradlew build run cleanly?

  • Have you written or updated unit tests to verify your changes?

  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?

Note:

Please ensure that once the PR is submitted, you check travis-ci for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.

@jinmeiliao
Copy link
Member

thanks for fixing this. I had this failure on my precheckin

private static synchronized void remoteTearDown() {
try {
DistributionMessageObserver.setInstance(null);
waitForNoRebalancing();

Choose a reason for hiding this comment

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

Looks like this is test specific...how about having this in "CacheXmlTestCase" instead here...

* Field is static so it doesn't get serialized with SerializableRunnable inner classes.
*/
private static InternalCache cache;
protected static InternalCache cache;

Choose a reason for hiding this comment

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

Why has the access been changed? There aren't any direct references to cache in this change set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The change I made is in a subclass (CacheXmlTestCase) and references cache there.

Choose a reason for hiding this comment

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

OK, I missed that reference to the cache during my review.

@jdeppe-pivotal jdeppe-pivotal merged commit 2d04130 into apache:develop Jan 19, 2018
@jdeppe-pivotal jdeppe-pivotal deleted the feature/GEODE-4312 branch January 19, 2018 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants