You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Denis Magda <dm...@apache.org> on 2017/02/27 19:44:16 UTC
Renaming TcpDiscoverySpi.heartbeatsFrequency to
TcpDiscoverySpi.metricsUpdateFrequency
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
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to TcpDiscoverySpi.metricsUpdateFrequency
Posted by Yakov Zhdanov <yz...@apache.org>.
Denis, there were minor test failures on tc. Alex will fix them on Mon and
I expect merge to happen right after.
--
Yakov Zhdanov
On Apr 22, 2017 2:22 AM, "Denis Magda" <dm...@apache.org> wrote:
> Yakov, Sasha,
>
> Are you ready to merge all the changes and close the ticket? As far as I
> understand, we plan to freeze 2.0 branch the next week.
>
> —
> Denis
>
> > On Apr 11, 2017, at 8:59 AM, Denis Magda <dm...@apache.org> wrote:
> >
> > Yasha,
> >
> > I’ve updated the javadoc for IgniteConfiguration.metricsUpdateFrequency
> and merged the changes into your branch (see JIRA). The rest is fine for
> now. An advance documentation on how the heartbeats are implemented can be
> done in readme.io <http://readme.io/>.
> >
> > —
> > Denis
> >
> >> On Apr 11, 2017, at 1:07 AM, Yakov Zhdanov <yz...@apache.org> wrote:
> >>
> >> Messages renaming is never compatible =)
> >>
> >> Denis, did you try to update javadocs?
> >>
> >> --Yakov
> >>
> >> 2017-04-10 21:41 GMT+03:00 Denis Magda <dm...@apache.org>:
> >>
> >>> Yasha,
> >>>
> >>> Thanks, I’ve replied you in the ticket.
> >>>
> >>> In a nutshel, let’s try to merge current breaking changes to 2.0 and do
> >>> the rest of compatible refactoring in further releases. Are you agree
> with
> >>> this?
> >>>
> >>> —
> >>> Denis
> >>>
> >>>> On Apr 10, 2017, at 8:39 AM, Yakov Zhdanov <yz...@apache.org>
> wrote:
> >>>>
> >>>> Alex Belyak, please go ahead. Thank you for your help! There were some
> >>> work
> >>>> I did not expect to happen.
> >>>>
> >>>>
> >>>> --Yakov
> >>>>
> >>>> 2017-04-10 18:13 GMT+03:00 Sasha Belyak <rt...@gmail.com>:
> >>>>
> >>>>> Yakov, I can do replacement "heartbeat" -> "metricsUpdate". Can I get
> >>>>> https://issues.apache.org/jira/browse/IGNITE-4799 ?
> >>>>>
> >>>>> 2017-04-10 14:50 GMT+03:00 Yakov Zhdanov <yz...@apache.org>:
> >>>>>
> >>>>>> Guys, I have finished with initial changes. There were a little bit
> >>> more
> >>>>>> work to do than I originally estimated.
> >>>>>>
> >>>>>> Denis, can you please reply in the ticket? I would like you to
> propose
> >>>>>> documentation changes. Can you help?
> >>>>>>
> >>>>>> There is also another question - we have a lot of "heartbeat"
> mentions
> >>> in
> >>>>>> TCP disco - missed heartbeats, client heartbeats, heartbeat message.
> >>>>>> Should
> >>>>>> we go ahead and refactor this as well e.g. to
> maxMissedMetricUpdates?
> >>>>>>
> >>>>>> Also there are some changes to platforms and webconsole required.
> Alex
> >>>>>> Kuznetsov and Pavel Tupitsyn can you take a look at respective
> parts?
> >>>>>>
> >>>>>> --
> >>>>>> Yakov
> >>>>>>
> >>>>>
> >>>>>
> >>>
> >>>
> >
>
>
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to
TcpDiscoverySpi.metricsUpdateFrequency
Posted by Denis Magda <dm...@apache.org>.
Yakov, Sasha,
Are you ready to merge all the changes and close the ticket? As far as I understand, we plan to freeze 2.0 branch the next week.
—
Denis
> On Apr 11, 2017, at 8:59 AM, Denis Magda <dm...@apache.org> wrote:
>
> Yasha,
>
> I’ve updated the javadoc for IgniteConfiguration.metricsUpdateFrequency and merged the changes into your branch (see JIRA). The rest is fine for now. An advance documentation on how the heartbeats are implemented can be done in readme.io <http://readme.io/>.
>
> —
> Denis
>
>> On Apr 11, 2017, at 1:07 AM, Yakov Zhdanov <yz...@apache.org> wrote:
>>
>> Messages renaming is never compatible =)
>>
>> Denis, did you try to update javadocs?
>>
>> --Yakov
>>
>> 2017-04-10 21:41 GMT+03:00 Denis Magda <dm...@apache.org>:
>>
>>> Yasha,
>>>
>>> Thanks, I’ve replied you in the ticket.
>>>
>>> In a nutshel, let’s try to merge current breaking changes to 2.0 and do
>>> the rest of compatible refactoring in further releases. Are you agree with
>>> this?
>>>
>>> —
>>> Denis
>>>
>>>> On Apr 10, 2017, at 8:39 AM, Yakov Zhdanov <yz...@apache.org> wrote:
>>>>
>>>> Alex Belyak, please go ahead. Thank you for your help! There were some
>>> work
>>>> I did not expect to happen.
>>>>
>>>>
>>>> --Yakov
>>>>
>>>> 2017-04-10 18:13 GMT+03:00 Sasha Belyak <rt...@gmail.com>:
>>>>
>>>>> Yakov, I can do replacement "heartbeat" -> "metricsUpdate". Can I get
>>>>> https://issues.apache.org/jira/browse/IGNITE-4799 ?
>>>>>
>>>>> 2017-04-10 14:50 GMT+03:00 Yakov Zhdanov <yz...@apache.org>:
>>>>>
>>>>>> Guys, I have finished with initial changes. There were a little bit
>>> more
>>>>>> work to do than I originally estimated.
>>>>>>
>>>>>> Denis, can you please reply in the ticket? I would like you to propose
>>>>>> documentation changes. Can you help?
>>>>>>
>>>>>> There is also another question - we have a lot of "heartbeat" mentions
>>> in
>>>>>> TCP disco - missed heartbeats, client heartbeats, heartbeat message.
>>>>>> Should
>>>>>> we go ahead and refactor this as well e.g. to maxMissedMetricUpdates?
>>>>>>
>>>>>> Also there are some changes to platforms and webconsole required. Alex
>>>>>> Kuznetsov and Pavel Tupitsyn can you take a look at respective parts?
>>>>>>
>>>>>> --
>>>>>> Yakov
>>>>>>
>>>>>
>>>>>
>>>
>>>
>
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to
TcpDiscoverySpi.metricsUpdateFrequency
Posted by Denis Magda <dm...@apache.org>.
Yasha,
I’ve updated the javadoc for IgniteConfiguration.metricsUpdateFrequency and merged the changes into your branch (see JIRA). The rest is fine for now. An advance documentation on how the heartbeats are implemented can be done in readme.io <http://readme.io/>.
—
Denis
> On Apr 11, 2017, at 1:07 AM, Yakov Zhdanov <yz...@apache.org> wrote:
>
> Messages renaming is never compatible =)
>
> Denis, did you try to update javadocs?
>
> --Yakov
>
> 2017-04-10 21:41 GMT+03:00 Denis Magda <dm...@apache.org>:
>
>> Yasha,
>>
>> Thanks, I’ve replied you in the ticket.
>>
>> In a nutshel, let’s try to merge current breaking changes to 2.0 and do
>> the rest of compatible refactoring in further releases. Are you agree with
>> this?
>>
>> —
>> Denis
>>
>>> On Apr 10, 2017, at 8:39 AM, Yakov Zhdanov <yz...@apache.org> wrote:
>>>
>>> Alex Belyak, please go ahead. Thank you for your help! There were some
>> work
>>> I did not expect to happen.
>>>
>>>
>>> --Yakov
>>>
>>> 2017-04-10 18:13 GMT+03:00 Sasha Belyak <rt...@gmail.com>:
>>>
>>>> Yakov, I can do replacement "heartbeat" -> "metricsUpdate". Can I get
>>>> https://issues.apache.org/jira/browse/IGNITE-4799 ?
>>>>
>>>> 2017-04-10 14:50 GMT+03:00 Yakov Zhdanov <yz...@apache.org>:
>>>>
>>>>> Guys, I have finished with initial changes. There were a little bit
>> more
>>>>> work to do than I originally estimated.
>>>>>
>>>>> Denis, can you please reply in the ticket? I would like you to propose
>>>>> documentation changes. Can you help?
>>>>>
>>>>> There is also another question - we have a lot of "heartbeat" mentions
>> in
>>>>> TCP disco - missed heartbeats, client heartbeats, heartbeat message.
>>>>> Should
>>>>> we go ahead and refactor this as well e.g. to maxMissedMetricUpdates?
>>>>>
>>>>> Also there are some changes to platforms and webconsole required. Alex
>>>>> Kuznetsov and Pavel Tupitsyn can you take a look at respective parts?
>>>>>
>>>>> --
>>>>> Yakov
>>>>>
>>>>
>>>>
>>
>>
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to TcpDiscoverySpi.metricsUpdateFrequency
Posted by Yakov Zhdanov <yz...@apache.org>.
Messages renaming is never compatible =)
Denis, did you try to update javadocs?
--Yakov
2017-04-10 21:41 GMT+03:00 Denis Magda <dm...@apache.org>:
> Yasha,
>
> Thanks, I’ve replied you in the ticket.
>
> In a nutshel, let’s try to merge current breaking changes to 2.0 and do
> the rest of compatible refactoring in further releases. Are you agree with
> this?
>
> —
> Denis
>
> > On Apr 10, 2017, at 8:39 AM, Yakov Zhdanov <yz...@apache.org> wrote:
> >
> > Alex Belyak, please go ahead. Thank you for your help! There were some
> work
> > I did not expect to happen.
> >
> >
> > --Yakov
> >
> > 2017-04-10 18:13 GMT+03:00 Sasha Belyak <rt...@gmail.com>:
> >
> >> Yakov, I can do replacement "heartbeat" -> "metricsUpdate". Can I get
> >> https://issues.apache.org/jira/browse/IGNITE-4799 ?
> >>
> >> 2017-04-10 14:50 GMT+03:00 Yakov Zhdanov <yz...@apache.org>:
> >>
> >>> Guys, I have finished with initial changes. There were a little bit
> more
> >>> work to do than I originally estimated.
> >>>
> >>> Denis, can you please reply in the ticket? I would like you to propose
> >>> documentation changes. Can you help?
> >>>
> >>> There is also another question - we have a lot of "heartbeat" mentions
> in
> >>> TCP disco - missed heartbeats, client heartbeats, heartbeat message.
> >>> Should
> >>> we go ahead and refactor this as well e.g. to maxMissedMetricUpdates?
> >>>
> >>> Also there are some changes to platforms and webconsole required. Alex
> >>> Kuznetsov and Pavel Tupitsyn can you take a look at respective parts?
> >>>
> >>> --
> >>> Yakov
> >>>
> >>
> >>
>
>
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to
TcpDiscoverySpi.metricsUpdateFrequency
Posted by Denis Magda <dm...@apache.org>.
Yasha,
Thanks, I’ve replied you in the ticket.
In a nutshel, let’s try to merge current breaking changes to 2.0 and do the rest of compatible refactoring in further releases. Are you agree with this?
—
Denis
> On Apr 10, 2017, at 8:39 AM, Yakov Zhdanov <yz...@apache.org> wrote:
>
> Alex Belyak, please go ahead. Thank you for your help! There were some work
> I did not expect to happen.
>
>
> --Yakov
>
> 2017-04-10 18:13 GMT+03:00 Sasha Belyak <rt...@gmail.com>:
>
>> Yakov, I can do replacement "heartbeat" -> "metricsUpdate". Can I get
>> https://issues.apache.org/jira/browse/IGNITE-4799 ?
>>
>> 2017-04-10 14:50 GMT+03:00 Yakov Zhdanov <yz...@apache.org>:
>>
>>> Guys, I have finished with initial changes. There were a little bit more
>>> work to do than I originally estimated.
>>>
>>> Denis, can you please reply in the ticket? I would like you to propose
>>> documentation changes. Can you help?
>>>
>>> There is also another question - we have a lot of "heartbeat" mentions in
>>> TCP disco - missed heartbeats, client heartbeats, heartbeat message.
>>> Should
>>> we go ahead and refactor this as well e.g. to maxMissedMetricUpdates?
>>>
>>> Also there are some changes to platforms and webconsole required. Alex
>>> Kuznetsov and Pavel Tupitsyn can you take a look at respective parts?
>>>
>>> --
>>> Yakov
>>>
>>
>>
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to TcpDiscoverySpi.metricsUpdateFrequency
Posted by Yakov Zhdanov <yz...@apache.org>.
Alex Belyak, please go ahead. Thank you for your help! There were some work
I did not expect to happen.
--Yakov
2017-04-10 18:13 GMT+03:00 Sasha Belyak <rt...@gmail.com>:
> Yakov, I can do replacement "heartbeat" -> "metricsUpdate". Can I get
> https://issues.apache.org/jira/browse/IGNITE-4799 ?
>
> 2017-04-10 14:50 GMT+03:00 Yakov Zhdanov <yz...@apache.org>:
>
>> Guys, I have finished with initial changes. There were a little bit more
>> work to do than I originally estimated.
>>
>> Denis, can you please reply in the ticket? I would like you to propose
>> documentation changes. Can you help?
>>
>> There is also another question - we have a lot of "heartbeat" mentions in
>> TCP disco - missed heartbeats, client heartbeats, heartbeat message.
>> Should
>> we go ahead and refactor this as well e.g. to maxMissedMetricUpdates?
>>
>> Also there are some changes to platforms and webconsole required. Alex
>> Kuznetsov and Pavel Tupitsyn can you take a look at respective parts?
>>
>> --
>> Yakov
>>
>
>
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to TcpDiscoverySpi.metricsUpdateFrequency
Posted by Sasha Belyak <rt...@gmail.com>.
Yakov, I can do replacement "heartbeat" -> "metricsUpdate". Can I get
https://issues.apache.org/jira/browse/IGNITE-4799 ?
2017-04-10 14:50 GMT+03:00 Yakov Zhdanov <yz...@apache.org>:
> Guys, I have finished with initial changes. There were a little bit more
> work to do than I originally estimated.
>
> Denis, can you please reply in the ticket? I would like you to propose
> documentation changes. Can you help?
>
> There is also another question - we have a lot of "heartbeat" mentions in
> TCP disco - missed heartbeats, client heartbeats, heartbeat message. Should
> we go ahead and refactor this as well e.g. to maxMissedMetricUpdates?
>
> Also there are some changes to platforms and webconsole required. Alex
> Kuznetsov and Pavel Tupitsyn can you take a look at respective parts?
>
> --
> Yakov
>
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to TcpDiscoverySpi.metricsUpdateFrequency
Posted by Yakov Zhdanov <yz...@apache.org>.
Guys, I have finished with initial changes. There were a little bit more
work to do than I originally estimated.
Denis, can you please reply in the ticket? I would like you to propose
documentation changes. Can you help?
There is also another question - we have a lot of "heartbeat" mentions in
TCP disco - missed heartbeats, client heartbeats, heartbeat message. Should
we go ahead and refactor this as well e.g. to maxMissedMetricUpdates?
Also there are some changes to platforms and webconsole required. Alex
Kuznetsov and Pavel Tupitsyn can you take a look at respective parts?
--
Yakov
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to TcpDiscoverySpi.metricsUpdateFrequency
Posted by Yakov Zhdanov <yz...@apache.org>.
Alex, I am finishing this up.
--Yakov
2017-03-30 14:03 GMT+03:00 Александр Меньшиков <sh...@gmail.com>:
> Yakov, can I take this ticket or you have already started to implement it?
> Ask because you are an assignee in JIRA.
>
> 2017-03-07 22:21 GMT+03:00 Denis Magda <dm...@apache.org>:
>
>> Good, the ticket is ready:
>> https://issues.apache.org/jira/browse/IGNITE-4799 <
>> https://issues.apache.org/jira/browse/IGNITE-4799>
>>
>> —
>> Denis
>>
>> > On Mar 7, 2017, at 1:47 AM, Yakov Zhdanov <yz...@apache.org> wrote:
>> >
>> >>> What’s about compute engine load balancers then? They rely on the
>> > metrics received from remote nodes.
>> >
>> > First of all, that was poor design decision - to make different
>> components
>> > dependant, possibly, through user-defined implementation of Discovery
>> Spi.
>> >
>> > What you say makes sense to me. Let's remove HB frequency from Discovery
>> > SPI API and adjust it accordingly to
>> > IgniteConfiguraion.getMetricsUpdateFrequency.
>> > You should also consider completely removing heartbeats from failure
>> > detection process - it should rely only on internal ping packets.
>> >
>> > --Yakov
>>
>>
>
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to TcpDiscoverySpi.metricsUpdateFrequency
Posted by Александр Меньшиков <sh...@gmail.com>.
Yakov, can I take this ticket or you have already started to implement it?
Ask because you are an assignee in JIRA.
2017-03-07 22:21 GMT+03:00 Denis Magda <dm...@apache.org>:
> Good, the ticket is ready:
> https://issues.apache.org/jira/browse/IGNITE-4799 <
> https://issues.apache.org/jira/browse/IGNITE-4799>
>
> —
> Denis
>
> > On Mar 7, 2017, at 1:47 AM, Yakov Zhdanov <yz...@apache.org> wrote:
> >
> >>> What’s about compute engine load balancers then? They rely on the
> > metrics received from remote nodes.
> >
> > First of all, that was poor design decision - to make different
> components
> > dependant, possibly, through user-defined implementation of Discovery
> Spi.
> >
> > What you say makes sense to me. Let's remove HB frequency from Discovery
> > SPI API and adjust it accordingly to
> > IgniteConfiguraion.getMetricsUpdateFrequency.
> > You should also consider completely removing heartbeats from failure
> > detection process - it should rely only on internal ping packets.
> >
> > --Yakov
>
>
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to
TcpDiscoverySpi.metricsUpdateFrequency
Posted by Denis Magda <dm...@apache.org>.
Good, the ticket is ready:
https://issues.apache.org/jira/browse/IGNITE-4799 <https://issues.apache.org/jira/browse/IGNITE-4799>
—
Denis
> On Mar 7, 2017, at 1:47 AM, Yakov Zhdanov <yz...@apache.org> wrote:
>
>>> What’s about compute engine load balancers then? They rely on the
> metrics received from remote nodes.
>
> First of all, that was poor design decision - to make different components
> dependant, possibly, through user-defined implementation of Discovery Spi.
>
> What you say makes sense to me. Let's remove HB frequency from Discovery
> SPI API and adjust it accordingly to
> IgniteConfiguraion.getMetricsUpdateFrequency.
> You should also consider completely removing heartbeats from failure
> detection process - it should rely only on internal ping packets.
>
> --Yakov
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to TcpDiscoverySpi.metricsUpdateFrequency
Posted by Yakov Zhdanov <yz...@apache.org>.
>> What’s about compute engine load balancers then? They rely on the
metrics received from remote nodes.
First of all, that was poor design decision - to make different components
dependant, possibly, through user-defined implementation of Discovery Spi.
What you say makes sense to me. Let's remove HB frequency from Discovery
SPI API and adjust it accordingly to
IgniteConfiguraion.getMetricsUpdateFrequency.
You should also consider completely removing heartbeats from failure
detection process - it should rely only on internal ping packets.
--Yakov
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to
TcpDiscoverySpi.metricsUpdateFrequency
Posted by Denis Magda <dm...@apache.org>.
> On Mar 1, 2017, at 11:22 PM, Yakov Zhdanov <yz...@apache.org> wrote:
>
> 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?
>
What’s about compute engine load balancers then? They rely on the metrics received from remote nodes.
BTW, I’ve found this method suddenly - IgniteConfiguraion.getMetricsUpdateFrequency which defines frequency for job metrics.
What if we use this method to adjust frequency for metrics sent over heartbeats? It’s obvious that we need to align our API somehow.
—
Denis
> --Yakov
>
> 2017-03-02 2:24 GMT+03:00 Denis Magda <dm...@apache.org>:
>
>> 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 <yz...@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 <da...@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 <dm...@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 <yz...@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 <ds...@apache.org>:
>>>>>>
>>>>>>> Yakov, what do you think?
>>>>>>>
>>>>>>> On Mon, Feb 27, 2017 at 4:17 PM, Denis Magda <dm...@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 <
>>>> dsetrakyan@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 <dm...@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 <
>>>>>>> dsetrakyan@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 <dmagda@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
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>>
>>
>>
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to TcpDiscoverySpi.metricsUpdateFrequency
Posted by Yakov Zhdanov <yz...@apache.org>.
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 <dm...@apache.org>:
> 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 <yz...@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 <da...@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 <dm...@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 <yz...@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 <ds...@apache.org>:
> >>>>
> >>>>> Yakov, what do you think?
> >>>>>
> >>>>> On Mon, Feb 27, 2017 at 4:17 PM, Denis Magda <dm...@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 <
> >> dsetrakyan@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 <dm...@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 <
> >>>>> dsetrakyan@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 <dmagda@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
> >>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>
> >>>
> >>
>
>
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to
TcpDiscoverySpi.metricsUpdateFrequency
Posted by Denis Magda <dm...@apache.org>.
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 <yz...@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 <da...@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 <dm...@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 <yz...@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 <ds...@apache.org>:
>>>>
>>>>> Yakov, what do you think?
>>>>>
>>>>> On Mon, Feb 27, 2017 at 4:17 PM, Denis Magda <dm...@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 <
>> dsetrakyan@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 <dm...@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 <
>>>>> dsetrakyan@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 <dm...@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
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>
>>>
>>
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to TcpDiscoverySpi.metricsUpdateFrequency
Posted by Yakov Zhdanov <yz...@apache.org>.
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 <da...@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 <dm...@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 <yz...@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 <ds...@apache.org>:
> > >
> > >> Yakov, what do you think?
> > >>
> > >> On Mon, Feb 27, 2017 at 4:17 PM, Denis Magda <dm...@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 <
> dsetrakyan@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 <dm...@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 <
> > >> dsetrakyan@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 <dm...@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
> > >>>>>
> > >>>>>
> > >>>
> > >>>
> > >>
> >
> >
>
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to TcpDiscoverySpi.metricsUpdateFrequency
Posted by Dani Traphagen <da...@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 <dm...@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 <yz...@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 <ds...@apache.org>:
> >
> >> Yakov, what do you think?
> >>
> >> On Mon, Feb 27, 2017 at 4:17 PM, Denis Magda <dm...@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 <dsetrakyan@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 <dm...@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 <
> >> dsetrakyan@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 <dm...@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
> >>>>>
> >>>>>
> >>>
> >>>
> >>
>
>
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to
TcpDiscoverySpi.metricsUpdateFrequency
Posted by Denis Magda <dm...@apache.org>.
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 <yz...@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 <ds...@apache.org>:
>
>> Yakov, what do you think?
>>
>> On Mon, Feb 27, 2017 at 4:17 PM, Denis Magda <dm...@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 <ds...@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 <dm...@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 <
>> dsetrakyan@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 <dm...@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
>>>>>
>>>>>
>>>
>>>
>>
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to TcpDiscoverySpi.metricsUpdateFrequency
Posted by Yakov Zhdanov <yz...@apache.org>.
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 <ds...@apache.org>:
> Yakov, what do you think?
>
> On Mon, Feb 27, 2017 at 4:17 PM, Denis Magda <dm...@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 <ds...@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 <dm...@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 <
> dsetrakyan@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 <dm...@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
> > >>
> > >>
> >
> >
>
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to TcpDiscoverySpi.metricsUpdateFrequency
Posted by Dmitriy Setrakyan <ds...@apache.org>.
Yakov, what do you think?
On Mon, Feb 27, 2017 at 4:17 PM, Denis Magda <dm...@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 <ds...@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 <dm...@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 <ds...@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 <dm...@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
> >>
> >>
>
>
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to
TcpDiscoverySpi.metricsUpdateFrequency
Posted by Denis Magda <dm...@apache.org>.
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 <ds...@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 <dm...@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 <ds...@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 <dm...@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
>>
>>
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to TcpDiscoverySpi.metricsUpdateFrequency
Posted by Dmitriy Setrakyan <ds...@apache.org>.
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 <dm...@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 <ds...@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 <dm...@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
>
>
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to
TcpDiscoverySpi.metricsUpdateFrequency
Posted by Denis Magda <dm...@apache.org>.
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 <ds...@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 <dm...@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
Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to TcpDiscoverySpi.metricsUpdateFrequency
Posted by Dmitriy Setrakyan <ds...@apache.org>.
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 <dm...@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