geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Robert Houghton <rhough...@pivotal.io>
Subject Re: [DISCUSS] changing geode 32-bit counter stats to 64-bit
Date Mon, 10 Jun 2019 18:20:59 GMT
+1 to @Udo Kohlmeyer <ukohlmeyer@pivotal.io> comment. If the memory has
been used, might as well allow us to query the values

On Fri, Jun 7, 2019 at 2:18 PM Udo Kohlmeyer <ukohlmeyer@pivotal.io> wrote:

> +1, if the LongAdders have already added and the additional memory usage
> has already been dealt with, then adding the accessors does not really
> make a difference anymore.
>
> On 6/7/19 13:47, Jacob Barrett 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