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