Originally we just had a single instance of CachePerfStats per jvm. So all the region related stats were always rolled up onto the single CachePerfStats. Later we added RegionPerfStats so that users could see what was happening on a per region basis. When RegionPerfStats was added it was made to extend CachePerfStats but no new stats were added to it.
I think we now have some stats that only make sense on a Cache (like "regions" and "partitionedRegions") but we also have them on RegionPerfStats. I thought these used to always return zero on the region but I just looked at the implementation and it looks like they just return 1 or 0 on the region (depending on if it is partitioned or not). The help text says it will be the number of regions on the cache (so the help is correct for CachePerfStats but wrong for RegionPerfStats). I would be okay with us dropping the stats that only make sense at the cache level from the per region stats. Something I could not tell from you diagram is if you are cleaning this up. Does CacheStats also have everything that is on RegionStats? Or will it now just have the stats that are unique to a cache? If you change the internal names (like CachePerfStats -> CacheStats) keep in mind that you should use the same type name when calling "createType" (in this case "CachePerfStats") for backwards compatibility. On Thu, Jul 11, 2019 at 9:45 AM Mark Hanson <mhan...@pivotal.io> wrote: > See my comments inline.. > > > On Jul 11, 2019, at 9:36 AM, Darrel Schneider <dschnei...@pivotal.io> > wrote: > > > > Currently we do not make visible per bucket stats. Is the above proposal > > just to change the internal implementation but the stats visible in tools > > like vsd are unchanged? > > > As with my previous e-mail exchange with Jake, I would like to back burner > per bucket stats. If it turns out to be a problem, I will bring it back > before the group. > > My goal is these are internal changes. I would see it as a problem > requiring re-evaluation, if this were a meaningful breaking change. E.g. > breaks vsd, changes public API > An important note would be that fixing a bug in a stat, is not a > meaningful breaking change. > > > Currently we have an internal CachePerfStats which the internal > > RegionPerfStats extends. Does your CacheStats replace CachePerfStats and > > your RegionStats replace RegionPerfStats? > > > > You are 100% correct. > > > Currently we have an internal PartitionedRegionStats class. Does > > your PartitionedRegionStats replace it? > > > > Yes. I considered that under the “RegionPerfStats” umbrella. > > > Are your "*Stats" interfaces and your "*StatsImpl" classes? > > Yes. > > > > > On Thu, Jul 11, 2019 at 9:29 AM Mark Hanson <mhan...@pivotal.io> wrote: > > > >> It depends to be honest. This is just a plan. I would hope to roll them > >> up, but I can see a case where you have buckets on different servers, > that > >> would seem to necessitate that. > >> > >>> On Jul 11, 2019, at 9:26 AM, Jacob Barrett <jbarr...@pivotal.io> > wrote: > >>> > >>> There isn’t currently a partition stat instance per bucket. Are you > >> saying you’re making that a thing now? > >>> > >>>> On Jul 11, 2019, at 9:24 AM, Mark Hanson <mhan...@pivotal.io> wrote: > >>>> > >>>> Correct. > >>>> > >>>>> On Jul 11, 2019, at 9:23 AM, Darrel Schneider <dschnei...@pivotal.io > > > >> wrote: > >>>>> > >>>>> Why would a PartitionedRegionStatsImpl contain more than one > >> RegionStats? > >>>>> Are these representing the local buckets? > >>>>> > >>>>>> On Wed, Jul 10, 2019 at 4:57 PM Mark Hanson <mhan...@pivotal.io> > >> wrote: > >>>>>> > >>>>>> PartitionRegionStatsImpl can contain one to many RegionStats > >>>>>> > >>>>>>> On Jul 10, 2019, at 4:53 PM, Dan Smith <dsm...@pivotal.io> wrote: > >>>>>>> > >>>>>>> Seems reasonable. I'm guessing that CachePerfImpl contains many > >>>>>> RegionStats. Does PartitionRegionStatsImpl just contain a single > >>>>>> RegionStats? > >>>>>>> > >>>>>>> On Wed, Jul 10, 2019 at 4:49 PM Mark Hanson <mhan...@pivotal.io > >> <mailto: > >>>>>> mhan...@pivotal.io>> wrote: > >>>>>>> Hi All, > >>>>>>> > >>>>>>> As many of you may know our structure for our perf stats is not > >> great. I > >>>>>> would like to propose we refactor the code to have the following > >>>>>> inheritance model, which Kirk and I came up with. > >>>>>>> > >>>>>>> It is my belief that fixing this will allow future features to be > >>>>>> implemented in a much less painful way. > >>>>>>> > >>>>>>> Thoughts? > >>>>>>> > >>>>>>> Thanks, > >>>>>>> Mark > >>>>>>> > >>>>>> > >>>>>> > >>>> > >> > >> > >
