geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Owen Nichols <onich...@pivotal.io>
Subject Re: Changes to statistics types
Date Sat, 23 Feb 2019 15:26:11 GMT
This seems like an entirely reasonable bugfix to make in a minor release.  A “release note”
mentioning the change should be more than sufficient to alert anyone who may be directly accessing
stats.

For me, the key information here is "This change does not affect VSD”.  For most people,
VSD is the "public interface" to statistics.

+1 for moving from ints to longs wherever possible

-Owen

> On Feb 22, 2019, at 11:00 AM, Helena Bales <heybales@apache.org> wrote:
> 
> Hello all,
> 
> The performance team wants to change some of the statistics that are
> currently ints to be longs so that they do not overflow as quickly. We were
> encountering issues with the performance tests due to overflows in stats
> following an increase in performance for gets. Recent pull requests to make
> this change have been reverted due to concern that this changes "public
> APIs". In our current proposed change we did not change the types of the
> MBean stats, however the stats are technically accessible from the
> distributed system. They can be accessed as long as you know the name of
> that statistic, but the names are not all correctly documented. Accessing
> internals through incorrectly documented identifiers does not constitute a
> public API but since it is technically something a user could do, this
> change demands some discussion. We have done our best to mitigate any
> potential issues for the user by downcasting the long stats to ints when
> the stats are retrieved by name only. If the user knew the unique id of
> that stat, and then tried to retrieve it as an int, they could potentially
> retrieve the wrong counter, or an exception if that id was not valid for
> that type. This is because the unique id is unique within that type of
> stat, not unique for all stats (see the second link below). Updating an
> internal stat by name or id will also fail for those stats we have changed
> but it is strongly arguable that updating internal stats should not be done
> externally by users. This change does not affect VSD or MBeans.
> 
> Does anyone believe that there is a valid use case for accessing internal
> statistics not addressed by this change?
> 
> Thanks,
> Helena and Jake
> 
> The PR: https://github.com/apache/geode/pull/3214
> The test that shows our workaround:
> https://github.com/apache/geode/pull/3214/files#diff-42320800db41504616849361b4b2a3af


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