ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Denis Magda <dma...@apache.org>
Subject Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to TcpDiscoverySpi.metricsUpdateFrequency
Date Thu, 02 Mar 2017 20:40:24 GMT

> On Mar 1, 2017, at 11:22 PM, Yakov Zhdanov <yzhdanov@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 <dmagda@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 <yzhdanov@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 <dani@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 <dmagda@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 <yzhdanov@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 <dsetrakyan@apache.org>:
>>>>>> 
>>>>>>> Yakov, what do you think?
>>>>>>> 
>>>>>>> On Mon, Feb 27, 2017 at 4:17 PM, Denis Magda <dmagda@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 <dmagda@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
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>> 
>>>>> 
>>>> 
>> 
>> 


Mime
View raw message