Guyd, I've extended the set of metrics a little bit and pushed my changes to ignite-5375 branch. Can you please review the metrics and see if we are now ok with the interfaces?
2017-06-01 21:04 GMT+03:00 Denis Magda <dma...@apache.org>: > Sergey, thanks, > > You get a green light from my side. Please go ahead and bring this to life > unless anyone else has any comments. > > — > Denis > > > On Jun 1, 2017, at 4:18 AM, Sergey Chugunov <sergey.chugu...@gmail.com> > wrote: > > > > Guys, > > > > I created a ticket [1] summing up results of our discussion. Please > review > > and leave comments if something is still unclear there. > > > > Thanks, > > Sergey > > > > [1] https://issues.apache.org/jira/browse/IGNITE-5375 > > > > On Wed, May 31, 2017 at 11:56 PM, Dmitriy Setrakyan < > dsetrak...@apache.org> > > wrote: > > > >> Denis, I do agree, the concept of virtual memory may work. I will start > >> another discussion thread on that. > >> > >> We should extensively javadoc all these interfaces. The current > one-liner > >> javadoc documentation is not enough. > >> > >> Also, I would like to make a small correction to the MemoryMetrics > >> interface, see below... > >> > >> > >> On Wed, May 31, 2017 at 1:35 PM, Denis Magda <dma...@apache.org> wrote: > >> > >>> Guys, > >>> If to assume that the *page memory* is a sort of *virtual* memory that > >>> maps a page to RAM or disk then MemoryMetrics interface makes sense if > we > >>> think of it as {Virtual}MemoryMetrics. It’s late to rename the > interface > >>> but this design flaw can be handled with a proper documentation. > >>> At all, let’s gather all the page memory related metrics (both RAM and > >>> disk) in MemoryMetrics interface. Here is it’s final version: > >>> public interface MemoryMetrics { > >>> public String getName(); > >>> > >>> public long getTotalAllocatedPages(); > >> > >> > >> I think this method currently returns only the pages allocated in > memory. > >> To behave correctly, it now should return the total pages allocated > within > >> the virtual memory, which should be equal to the total pages on disk. > >> > >> > >>> public long getPagesOnDisk(); > >>> > >> > >> This method should be removed as it represents the same value as > >> "getTotalAllocatedPages()" method. Instead, we should add the following > >> method: > >> > >> // Returns the number of pages allocated in physical off-heap memory. > >> getPhysicalMemoryPages(); > >> > >> > >>> > >>> public long getDirtyPages(); > >>> > >>> public float getAllocationRate(); > >>> > >>> public float getEvictionRate(); > >>> > >>> public float getPagesFillFactor(); > >>> > >>> public double getPagesMissRate(); > >>> > >>> public float getLargeEntriesPagesPercentage(); > >>> } > >>> I would rename getAllocationRate and getEvitionRate to > >>> getPagesAllocationRate and getEvictionRates respectively but then we > need > >>> to deprecate the existing methods which is not good. > >>> > >>> As for the persistence metrics related to WAL and checkpointing let’s > >>> store them here: > >>> > >>> public interface PersistentStoreMetrics { > >>> public double getWalLoggingRate(); > >>> > >>> public int getWalArchiveSegments(); > >>> > >>> public double getWalFsyncTime(); > >>> > >>> public double getCheckpointingTime(); > >>> > >>> public double getCheckpointingFsyncTime(); > >>> > >>> public long getCheckpointingTotalPagesNumber(); > >>> > >>> public long[] getCheckpointingPagesByTypeNumber(); > >>> > >>> public long getCheckpointingCopiedOnWritePagesNumber(); > >>> } > >>> > >>> Comments on the name of non released methods are welcomed. > >>> > >>> — > >>> Denis > >>> > >>>> On May 30, 2017, at 2:59 PM, Dmitriy Setrakyan <dsetrak...@apache.org > > > >>> wrote: > >>>> > >>>> On Tue, May 30, 2017 at 2:46 PM, Denis Magda <dma...@apache.org> > >> wrote: > >>>> > >>>>> I don’t have any extra metrics in mind for now. All I want to say > that > >>>>> presently the Store metrics interface aggregates WAL and > checkpointing > >>>>> metrics and in the future we might add additional Store related > >> metrics > >>>>> there (that has nothing to do with WAL). So, this is why > >>> IgniteWalMetrics > >>>>> doesn’t sound like a generic interface. > >>>>> > >>>> > >>>> I think it is getting more and more confusing. I, for instance, do not > >>>> understand where memory metrics end and storage begins, neither I > think > >>> it > >>>> is important to separate memory policy related metrics from the WAL or > >>>> Checkpoint related metrics. > >>>> > >>>> How about we put them all together and have clear method names? > >>>> > >>>> > >>>>> > >>>>> — > >>>>> Denis > >>>>> > >>>>>> On May 30, 2017, at 2:39 PM, Dmitriy Setrakyan < > >> dsetrak...@apache.org> > >>>>> wrote: > >>>>>> > >>>>>> On Tue, May 30, 2017 at 1:22 PM, Denis Magda <dma...@apache.org> > >>> wrote: > >>>>>> > >>>>>>> I’m afraid that in the future we might need to add non WAL related > >>>>> metrics > >>>>>>> under this interface. This is why it might make sense to keep the > >> name > >>>>>>> generic. > >>>>>>> > >>>>>> > >>>>>> Hm... I am not sure we are going to have any other components in > >>> addition > >>>>>> to WAL and Store here. What do you have in mind? > >>>>> > >>>>> > >>> > >>> > >> > >
