> On Mar 1, 2017, at 11:22 PM, Yakov Zhdanov <[email protected]> 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 <[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
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>> 
>>>>> 
>>>> 
>> 
>> 

Reply via email to