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

GEODE-6269: Extract StatisticsRegistry from InternalDistributedSystem #3068

Merged
merged 2 commits into from Jan 17, 2019

Conversation

demery-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.

Please review @kirklund @galen-pivotal @mhansonp

@kirklund kirklund changed the title Extract StatisticsRegistry from InternalDistributedSystem GEODE-6269: Extract StatisticsRegistry from InternalDistributedSystem Jan 11, 2019
@kirklund
Copy link
Contributor

The UpgradeTests have a bunch of failures that appear to be in just the Lucene tests and caused by the old version not having the method GemFireCacheImpl.getStatisticsRegistry(). I think this is test code blowing up and not product code. I think the test is trying to get some statistics to look at and is using test code that only works against the new version of product code. So we may need to change test code to fix this.

@kirklund
Copy link
Contributor

kirklund commented Jan 15, 2019

This is some pretty bad code in LuceneSearchWithRollingUpgradeDUnit.java. Apparently the line that fails is the one with getMethod("getStatisticsRegistry") and it looks like we incorrectly changed it from "getDistributedSystem":

  private static void assertVersion(Object cache, short ordinal) throws Exception {
    Class idmClass = Thread.currentThread().getContextClassLoader()
        .loadClass("org.apache.geode.distributed.internal.membership.InternalDistributedMember");
    Method getDSMethod = cache.getClass().getMethod("getStatisticsRegistry");
    getDSMethod.setAccessible(true);
    Object ds = getDSMethod.invoke(cache);

    Method getDistributedMemberMethod = ds.getClass().getMethod("getDistributedMember");
    getDistributedMemberMethod.setAccessible(true);
    Object member = getDistributedMemberMethod.invoke(ds);
    Method getVersionObjectMethod = member.getClass().getMethod("getVersionObject");
    getVersionObjectMethod.setAccessible(true);
    Object thisVersion = getVersionObjectMethod.invoke(member);
    Method getOrdinalMethod = thisVersion.getClass().getMethod("ordinal");
    getOrdinalMethod.setAccessible(true);
    short thisOrdinal = (Short) getOrdinalMethod.invoke(thisVersion);
    if (ordinal != thisOrdinal) {
      throw new Error(
          "Version ordinal:" + thisOrdinal + " was not the expected ordinal of:" + ordinal);
    }
  }

Extract StatisticsRegistry from InternalDistributedSystem
@kirklund kirklund merged commit 9d75ff7 into apache:develop Jan 17, 2019
@demery-pivotal demery-pivotal deleted the feature/GEODE-6269 branch January 22, 2019 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants