Alex, Makes sense to me. The only thing that should be fixed is the javadoc of this method: /** * Gets a total number of allocated pages in a memory region. * * @return Total number of allocated pages. */ public long getTotalAllocatedPages();
As far as I understand the metric should represent “all the pages allocated in the persistent store if the one enabled or all the pages we have on disk”. What’d be a right description? — Denis > On Jun 5, 2017, at 9:33 AM, Alexey Goncharuk <alexey.goncha...@gmail.com> > wrote: > > 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? >>>>>>> >>>>>>> >>>>> >>>>> >>>> >> >>
