ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Pavel Tupitsyn <ptupit...@apache.org>
Subject Re: MemoryMetrics interface inconsistencies
Date Mon, 24 Apr 2017 16:20:04 GMT
Sergey, I disagree.

1) As a user, I would expect MemoryMetrics instance to be
read-only and serializable, so I can send it somewhere, store,
put into a collection and draw a graph in UI, etc.

Other metrics APIs allow this, MemoryMetrics does not.

2) Methods like enableMetrics and rateTimeInterval placed in MemoryMetrics
break single responsibility principle and are confusing.
Why do I need to Get metrics to Enable them?

These methods should be placed somewhere else.

I would suggest some thing like this:
- introduce Memory class with getMetrics, enableMetrics,
setRateTimeInterval, etc
- add Ignite.getMemory method


On Mon, Apr 24, 2017 at 6:53 PM, Sergey Chugunov <sergey.chugunov@gmail.com>
wrote:

> I guess the main reason to have IgniteCache returning snapshot metrics was
> to collect metrics about distributed cache across the cluster.
> At the same time MemoryMetrics were initially designed to be local on each
> node, there were no requirements about collecting cluster-wide
> MemoryMetrics.
>
> Collecting MemoryMetrics is considered as an investigation action when
> something goes wrong, that's why it was decided to add enable/disable
> actions to the interface.
> So for me it sounds reasonable.
>
> The only debatable thing is having MemoryMetrics returning size in
> megabytes, however I think about these metrics as designed only for user,
> so I think it makes sense to return this metric in human-readable form.
>
> On Mon, Apr 24, 2017 at 6:41 PM, Vladimir Ozerov <vozerov@gridgain.com>
> wrote:
>
> > Agree, looks inconsistent to me.
> >
> > Alex G., could you chime in?
> >
> > On Mon, Apr 24, 2017 at 6:30 PM, Pavel Tupitsyn <ptupitsyn@apache.org>
> > wrote:
> >
> > > Igniters,
> > >
> > > I have noticed quite a lot of inconsistencies with the rest of our APIs
> > in
> > > our new MemoryMetrics API:
> > >
> > > 1) MemoryMetrics is not a snapshot. It is a "live" instance which
> returns
> > > different values each time you access some property.
> > (IgniteCache.metrics()
> > > returns a snapshot).
> > >
> > > 2) MemoryMetrics has methods that modify state, like enableMetrics
> > > and rateTimeInterval
> > > (IgniteCache.metrics() returns a read-only serializable snapshot)
> > >
> > > 3) getSize method returns size in megabytes
> > > (IgniteCache.metrics() provides sizes in bytes)
> > >
> > >
> > > I propose to rework this API until it is not too late.
> > >
> > > Thoughts?
> > >
> >
>

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