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 Wed, 01 Mar 2017 23:24:31 GMT
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