eolivelli commented on issue #854: ZOOKEEPER-3143 Pluggable metrics system for ZooKeeper - Data Collection on Server URL: https://github.com/apache/zookeeper/pull/854#issuecomment-474259942 > Thanks @eolivelli . Great stuff. > > Unfortunately I kinda lost tracking the conversation in the previous patch, but why are we doing > > > Make ServerMetrics a class, not an enum. > > I liked the enum approach, why do you need to change it? @lvfangmin @anmolnar @jhuan31 @phunt The main reason from my point of view is that **enums** should be treated as close as "constants", but after booting the MetricsProvider we will have to re-assign all of the references to Counters/Summaries, so we will alter the internal state of the enum values. I like that the new class has all *final* fields, when you boot the provider you throw away current ServerMetrics and start using the new one. The only cost is to have that single *volatile* static reference to the *current* ServerMetrics. The overall access cost to the single Metric is mostly the same ("access the static final? enum value + access a non final field" vs "access the volatile static reference + access the final field"), but I find the class approach most clean and testable.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
