I don't think new name makes things better. Btw, what if we remove metrics from heartbeats at all and make metrics local and allow aggregations only via management console?
--Yakov 2017-03-02 2:24 GMT+03:00 Denis Magda <[email protected]>: > 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 <[email protected]> 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 <[email protected]>: > > > >> 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 <[email protected]> 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 <[email protected]> > >> 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 <[email protected]>: > >>>> > >>>>> Yakov, what do you think? > >>>>> > >>>>> On Mon, Feb 27, 2017 at 4:17 PM, Denis Magda <[email protected]> > >> 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 < > >> [email protected] > >>>> > >>>>>> 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 <[email protected]> > >>>>> 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 < > >>>>> [email protected]> > >>>>>>>> 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 <[email protected] > > > >>>>>> 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 > >>>>>>>> > >>>>>>>> > >>>>>> > >>>>>> > >>>>> > >>> > >>> > >> > >
