Re: Should Geode stats conform to backwards compatibility constraints?

2019-02-14 Thread Michael Oleske
Here's a PR for a revert of the offending commits. https://github.com/apache/geode/pull/3195 Seems valuable to revert for now and decide a path forward rather than rush to get in 1.9 -michael On Wed, Feb 13, 2019 at 9:51 PM Jacob Barrett wrote: > How about mod MAX_INT? It would wrap back to 0

Re: Should Geode stats conform to backwards compatibility constraints?

2019-02-13 Thread Jacob Barrett
How about mod MAX_INT? It would wrap back to 0 and make it more consistent with at least SNMP counters that roll over to 0 when maxed. A monitoring and graphing system can account for this by recognizing the current value is less than the previous and typically uses the previous and current valu

Re: Should Geode stats conform to backwards compatibility constraints?

2019-02-13 Thread Ryan McMahon
Sorry that should read “and if the value exceeds MAX_INT” On Wed, Feb 13, 2019 at 8:21 PM Ryan McMahon wrote: > +1 to introducing a new method which returns the stat as long per Jake’s > suggestion. I vote for getInt() to downcast as an int, and the value > exceeds MAX_INT, return MAX_INT and p

Re: Should Geode stats conform to backwards compatibility constraints?

2019-02-13 Thread Ryan McMahon
+1 to introducing a new method which returns the stat as long per Jake’s suggestion. I vote for getInt() to downcast as an int, and the value exceeds MAX_INT, return MAX_INT and possibly add a warning message which points users to the new long version of the method. I think throwing an exception

Re: Should Geode stats conform to backwards compatibility constraints?

2019-02-13 Thread Owen Nichols
Is it possible for the underlying counter to be maintained as a long, and change the getInt method to return as normal when the value is within MAX_INT, otherwise throw an exception and/or return MAX_INT when the long value would overflow an int? For the MX Bean, should we keep (but deprecate)

Re: Should Geode stats conform to backwards compatibility constraints?

2019-02-13 Thread Dan Smith
+1 to what Jake said about MemberMXBean - that is definitely a public API class, so we need to deprecate the old method and introduce a new one. I'm also not clear about the stats. Technically, they are discoverable through the public API, so maybe? It seems like they are mix of things users might

Re: Should Geode stats conform to backwards compatibility constraints?

2019-02-13 Thread Jacob Barrett
I don’t consider the stats API a public API. I think it is a leaked internal API. Do we document the interactions with this API? Do we document the stat names? I consider documentation the API. I can discover all sorts of exposed methods in a library but shouldn’t consider them contracted API.

Should Geode stats conform to backwards compatibility constraints?

2019-02-13 Thread Kirk Lund
Quite a few Geode stats are currently defined as IntCounters which very easily wrap past Integer.MAX_VALUE. Some users wanted these stats to not wrap to negative MAX_VALUE, so my team defined the following two tickets and changed them to LongCounters on the develop branch: GEODE-6345: StatSamplerS