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.

Sign up for GitHub

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

Jump to bottom

GEODE-7221: Ensure management regions close CachePerfStats #4072

Merged
merged 3 commits into from Sep 20, 2019
Merged

GEODE-7221: Ensure management regions close CachePerfStats #4072

merged 3 commits into from Sep 20, 2019

Conversation

kirklund
Copy link
Contributor

@kirklund kirklund commented Sep 19, 2019 •
edited

  1. Cleanup and unit test FederatingManager
commit a47be7a495b8a62b5372a4ec0d2b8bcb259ea92d
Author: Kirk Lund <klund@apache.org>
Date:   Thu Sep 19 11:30:07 2019 -0700

    GEODE-7221: Cleanup and unit test FederatingManager
    
    * Move inner classes to bottom of outer class
    * Extract addMemberArtifacts from GIITask
    * Remove unnecessary code
    * Create FederatingManagerTest
  1. Cleanup and unit test LocalManager
commit 74e4467410e58f32897715a763ede0bff3020f77
Author: Kirk Lund <klund@apache.org>
Date:   Thu Sep 19 11:47:59 2019 -0700

    GEODE-7221: Cleanup and unit test LocalManager
    
    * Move inner class to bottom of outer class
    * Remove unnecessary code
    * Extract doManagementTask from ManagementTask
    * Create LocalManagerTest
  1. Fix GEODE-7221 by setting hasOwnStats true for management regions
commit a13a4a236de93955f0eac118bec0a93fe3ef47f9
Author: Kirk Lund <klund@apache.org>
Date:   Thu Sep 19 13:02:00 2019 -0700

    GEODE-7221: Set hasOwnStats true for management regions
    
    Ensure that managements regions close CachePerfStats by setting
    hasOwnStats to true from FederatingManager and LocalManager.

@kirklund kirklund force-pushed the GEM-2695-internalregions-hasownstats branch from f35f4d1 to 9159f06 Compare September 19, 2019 20:33
@kirklund
GEODE-7221: Cleanup and unit test FederatingManager
a47be7a 
* Move inner classes to bottom of outer class
* Extract addMemberArtifacts from GIITask
* Remove unnecessary code
* Create FederatingManagerTest
@kirklund
GEODE-7221: Cleanup and unit test LocalManager
74e4467 
* Move inner class to bottom of outer class
* Remove unnecessary code
* Extract doManagementTask from ManagementTask
* Create LocalManagerTest
@kirklund kirklund force-pushed the GEM-2695-internalregions-hasownstats branch from 9159f06 to a13a4a2 Compare September 19, 2019 20:44
@kirklund kirklund changed the title DRAFT: Do not review GEODE-7221: Ensure management regions close CachePerfStats Sep 19, 2019
@kirklund kirklund marked this pull request as ready for review September 19, 2019 20:49
boglesby
boglesby approved these changes Sep 20, 2019
View reviewed changes
Copy link
Contributor

@boglesby boglesby left a comment

Choose a reason for hiding this comment

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

I reviewed mainly the fix for GEODE-7221.

The leak of StripedStatisticsImpl instances is gone after the fix.

jmap -histo:live | grep StripedStatisticsImpl
num #instances #bytes class name

Without the fix:

Start server:
459: 16 1280 org.apache.geode.internal.statistics.StripedStatisticsImpl

Stop server:
468: 15 1200 org.apache.geode.internal.statistics.StripedStatisticsImpl

Start server:
439: 18 1440 org.apache.geode.internal.statistics.StripedStatisticsImpl

Stop server:
441: 17 1360 org.apache.geode.internal.statistics.StripedStatisticsImpl

Start server:
409: 20 1600 org.apache.geode.internal.statistics.StripedStatisticsImpl

Stop server:
422: 19 1520 org.apache.geode.internal.statistics.StripedStatisticsImpl

With the fix:

Start server:
462: 16 1280 org.apache.geode.internal.statistics.StripedStatisticsImpl

Stop server:
506: 13 1040 org.apache.geode.internal.statistics.StripedStatisticsImpl

Start server:
455: 16 1280 org.apache.geode.internal.statistics.StripedStatisticsImpl

Stop server:
508: 13 1040 org.apache.geode.internal.statistics.StripedStatisticsImpl

Start server:
459: 16 1280 org.apache.geode.internal.statistics.StripedStatisticsImpl

Stop server:
508: 13 1040 org.apache.geode.internal.statistics.StripedStatisticsImpl

@kirklund
GEODE-7221: Set hasOwnStats true for management regions
f681a04 
Ensure that managements regions close CachePerfStats by setting
hasOwnStats to true from FederatingManager and LocalManager.
@kirklund kirklund force-pushed the GEM-2695-internalregions-hasownstats branch from 7082b5e to f681a04 Compare September 20, 2019 17:39
@kirklund kirklund merged commit 6d3dd7b into apache:develop Sep 20, 2019
@kirklund kirklund deleted the GEM-2695-internalregions-hasownstats branch September 20, 2019 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@boglesby boglesby boglesby approved these changes

@dschneider-pivotal dschneider-pivotal dschneider-pivotal approved these changes

@mhansonp mhansonp Awaiting requested review from mhansonp

@demery-pivotal demery-pivotal Awaiting requested review from demery-pivotal

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@kirklund @boglesby @dschneider-pivotal @demery-pivotal