Yasha, Is there any other reason rather than compatibility that prevents us from both improving documentation and renaming the parameter?
— Denis > On Mar 1, 2017, at 1:13 AM, Yakov Zhdanov <yzhda...@apache.org> wrote: > > 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 >>>>>>>> >>>>>>>> >>>>>> >>>>>> >>>>> >>> >>> >>
