I think we should not rename this, but properly document the behavior and all the relationships.
--Yakov 2017-02-28 20:40 GMT+03:00 Dani Traphagen <d...@gridgain.com>: > I had this thought when exploring this parameter at first and only was able > to change my understanding when evaluating it more deeply in the source and > reaching out to other users/devs. > > There could be an issue with new users who think increasing the hb > frequency will impact inter-node failure detection - so people might tweak > it thinking it does something it doesn't. I think renaming it could help > avoid this. This is because other systems use heartbeat frequencies for > failure detection protocols under the hood so people may be coming with > this understanding and think that's what this parameter is for. > On Tue, Feb 28, 2017 at 9:16 AM Denis Magda <dma...@apache.org> wrote: > > > They expect exactly what the name of the > > TcpDiscoverySpi.heartbeatsFrequency method says - rate of heartbeats > will > > be adjusted if you play with this parameter. > > > > However, this is not true because the frequency of heartbeats is > > calculated from the failure detection timeout. > > > > — > > Denis > > > > > On Feb 28, 2017, at 7:28 AM, Yakov Zhdanov <yzhda...@apache.org> > wrote: > > > > > > Denis, I did not get your last message. What did users expect from > > changing > > > hbFreq? > > > > > > --Yakov > > > > > > 2017-02-28 4:08 GMT+03:00 Dmitriy Setrakyan <dsetrak...@apache.org>: > > > > > >> Yakov, what do you think? > > >> > > >> On Mon, Feb 27, 2017 at 4:17 PM, Denis Magda <dma...@apache.org> > wrote: > > >> > > >>> Frankly, I decided to initiate this discussion after talking to many > > >>> Apache Ignite users who had initially thought that TcpDiscoverySpi. > > >> heartbeatsFrequency > > >>> manages the heartbeats and they had tried to tweak it not getting a > > >> desired > > >>> outcome. Even more, TcpDiscoverySpi.heartbeatsFrequenc’s javadoc > > already > > >>> states that this is for metrics frequency only but looks like the > guys > > >>> hadn’t note this. > > >>> > > >>> So, personally, yes I would break the compatibility here which is > fine > > to > > >>> do in 2.0. > > >>> > > >>> — > > >>> Denis > > >>> > > >>>> On Feb 27, 2017, at 3:59 PM, Dmitriy Setrakyan < > dsetrak...@apache.org > > > > > >>> wrote: > > >>>> > > >>>> To me it sounds rather as an aesthetic change. Is it really worth > > >>> breaking > > >>>> the API for this? > > >>>> > > >>>> On Mon, Feb 27, 2017 at 3:30 PM, Denis Magda <dma...@apache.org> > > >> wrote: > > >>>> > > >>>>> The heartbeats frequency has to be lower than the failure detection > > >>>>> timeout. This is why we decided to calculate the former basing on a > > >>> value > > >>>>> set for the latter rather than making a user to adjust both > > properties > > >>>>> properly. This is how both parameters became related some time ago > :) > > >>>>> > > >>>>> Honestly, I don’t think that the javadoc improvement will make > things > > >>>>> clearer for the users. Hope you will agree that people pay > attention > > >> to > > >>> the > > >>>>> naming first and, only if the naming makes sense to them, learn > more > > >>> about > > >>>>> the details referring to the javadoc. > > >>>>> > > >>>>> — > > >>>>> Denis > > >>>>> > > >>>>>> On Feb 27, 2017, at 2:59 PM, Dmitriy Setrakyan < > > >> dsetrak...@apache.org> > > >>>>> wrote: > > >>>>>> > > >>>>>> Hm... I don't think that heartbeat frequency has to be associated > > >> with > > >>>>>> failure detection. What if we just update the javadoc for this > > >>> parameter, > > >>>>>> stating that it has to do with metrics update only? > > >>>>>> > > >>>>>> On Mon, Feb 27, 2017 at 11:44 AM, Denis Magda <dma...@apache.org> > > >>> wrote: > > >>>>>> > > >>>>>>> Igniters, > > >>>>>>> > > >>>>>>> Long time ago we updated the logic in discovery SPI that issues > > >>>>> heartbeats > > >>>>>>> messages from one node to another. Presently, heartbeats > frequency > > >> is > > >>>>>>> calculated basing on IgniteConfiguration.failureDetectionTimeout > > >>>>> meaning > > >>>>>>> that TcpDiscoverySpi.heartbeatsFrequency has nothing to do with > > >>>>>>> heartbeats frequency at all. > > >>>>>>> > > >>>>>>> TcpDiscoverySpi.heartbeatsFrequency defines a frequency for > metrics > > >>>>>>> message. So, I propose to rename this method in Apache Igntie 2.0 > > to > > >>>>>>> something more meaningful like TcpDiscoverySpi. > > >>> metricsUpdateFrequency? > > >>>>>>> > > >>>>>>> Do you agree? Alternatives thoughts? > > >>>>>>> > > >>>>>>> — > > >>>>>>> Denis > > >>>>> > > >>>>> > > >>> > > >>> > > >> > > > > >
