geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dan Smith <dsm...@pivotal.io>
Subject Re: [DISCUSS] changing geode 32-bit counter stats to 64-bit
Date Fri, 07 Jun 2019 21:01:07 GMT
+1 - this sounds like a good change to me.

-Dan

On Fri, Jun 7, 2019 at 1:47 PM Jacob Barrett <jbarrett@pivotal.io> wrote:

> I like this!
>
> I’d go ahead and change all the usage of the int methods to the long
> methods. I’d deprecate the int methods to make it very clear.
>
> If some consumer is using the int methods they will still work with the
> same rollover issues but perhaps with the deprecated warning they will
> update their code. Without the warning they may never know.
>
> -jake
>
>
> > On Jun 7, 2019, at 1:32 PM, Darrel Schneider <dschneider@pivotal.io>
> wrote:
> >
> > We have had a couple of tickets that have problems with 32-bit counters
> > changing too fast and causing them to be hard to understand when they
> wrap
> > around (see GEODE-6425 and GEODE-6834). We have also had problems with
> some
> > stats being broken because they were changing the 32-bit one when they
> > should have been changing the 64-bit one (see GEODE-6776).
> > The current implementation has one array of values for the 32-bit stats
> and
> > another array of values for the 64-bit stats. We use indexes into these
> > arrays when changing a stat. Given an int "id" used to index these
> arrays,
> > we can not tell if we should be indexing the 32-bit array or 64-bit
> array.
> > The first 32-bit stat for a type will have an id of 0 and the first
> 64-bit
> > stat on that type will also have an id of 0. But our current
> implementation
> > has the same type of value in both arrays (LongAdder
> > see: StripedStatisticsImpl fields intAdders and longAdders). So if we
> > simply change our implementation to have a single array, then the ids
> will
> > no longer be overloaded.
> >
> > Changing this internal implementation also improves backward
> compatibility.
> > Currently when we change one of our counters from 32-bit to 64-bit it is
> > possible we would break existing applications that are using the
> Statistics
> > interface. It has methods on it that allow stats to be read given an it:
> > getInt(int id) and getLong(int id). If they are currently reading a
> 32-bit
> > stat using getInt(id) and we change that stat to be 64-bit (like we did
> in
> > GEODE-6425) then they will now be reading the wrong stat when they call
> > getInt(id). If they used getInt(String) or getInt(StatisticDescriptor)
> they
> > will be okay. But if we make this simple change to have 32-bit and 64-bit
> > stats stored in the same array then getInt(id) will do the right thing
> when
> > we change a stat from 32-bit to 64-bit.
> >
> > Does anyone see a problem with making this change?
> >
> > After we do it I think we should change all of our counters to 64-bit
> since
> > they are always stored in a LongAdder anyway and we should deprecate the
> > methods that support 32-bit stats.
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message