ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yakov Zhdanov <yzhda...@apache.org>
Subject Re: Renaming TcpDiscoverySpi.heartbeatsFrequency to TcpDiscoverySpi.metricsUpdateFrequency
Date Thu, 02 Mar 2017 07:22:09 GMT
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 <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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message