ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexander Fedotov <alexander.fedot...@gmail.com>
Subject Re: IgniteConfiguration.gridName is very confusing
Date Mon, 30 Jan 2017 13:37:20 GMT
Done. But it looks like something went wrong since Upsource reports:
"Review has too many files (1244), aborting".

Also guys, I believe we need to merge this change in short time because
it's targeted for 2.0 and chances for a conflict are high.



On Mon, Jan 30, 2017 at 4:16 PM, Pavel Tupitsyn <ptupitsyn@apache.org>
wrote:

> Alexander,
>
> Please name the review appropriately and link it in the ticket as
> described:
> https://cwiki.apache.org/confluence/display/IGNITE/How+
> to+Contribute#HowtoContribute-ReviewWithUpsource
>
> Thanks,
> Pavel
>
> On Mon, Jan 30, 2017 at 4:00 PM, Alexander Fedotov <
> alexander.fedotoff@gmail.com> wrote:
>
> > Hi,
> >
> > Created Upsource review for the subject:
> > http://reviews.ignite.apache.org/ignite/review/IGNT-CR-81
> >
> > On Thu, Jan 19, 2017 at 7:59 PM, Alexander Fedotov <
> > alexander.fedotoff@gmail.com> wrote:
> >
> > > Hi,
> > >
> > > I've completed working on IGNITE-3207
> > > https://issues.apache.org/jira/browse/IGNITE-3207
> > >
> > > Looks like TC test results don't have problems related to my changes
> > > http://ci.ignite.apache.org/viewLog.html?buildId=423955&
> > > tab=buildResultsDiv&buildTypeId=IgniteTests_RunAll
> > >
> > > Kindly take a look at PR https://github.com/apache/ignite/pull/1435/
> > >
> > > On Thu, Jan 12, 2017 at 1:16 AM, Denis Magda <dmagda@apache.org>
> wrote:
> > >
> > >> Support Pavel’s point of view.
> > >>
> > >> Also Alexander please make sure that your changes are merged into
> > >> ignite-2.0 branch rather than to the master. I think this
> functionality
> > >> has to be available in 2.0 first. Finally, please update 2.0 Migration
> > >> Guide once you’ve finished with this task:
> > >> https://cwiki.apache.org/confluence/display/IGNITE/Apache+
> > >> Ignite+2.0+Migration+Guide <https://cwiki.apache.org/conf
> > >> luence/display/IGNITE/Apache+Ignite+2.0+Migration+Guide>
> > >>
> > >> —
> > >> Denis
> > >>
> > >> > On Jan 10, 2017, at 1:58 AM, Pavel Tupitsyn <ptupitsyn@apache.org>
> > >> wrote:
> > >> >
> > >> > I think we should fix log output as well and replace all "grid"
> > >> occurences
> > >> > with "instance".
> > >> >
> > >> > On Tue, Jan 10, 2017 at 12:55 PM, Alexander Fedotov <
> > >> > alexander.fedotoff@gmail.com> wrote:
> > >> >
> > >> >> Hi,
> > >> >>
> > >> >> I think we should leave null as a default value for unnamed Ignite
> > >> >> instances. At least that change should be considered out of the
> > current
> > >> >> scope.
> > >> >>
> > >> >> What about naming, I'm also renaming log occurrences of "grid"
and
> > >> "grid
> > >> >> name" where it stands reasonable.
> > >> >> Are there places in the logging logic where we should prefer name
> > >> "grid" or
> > >> >> "grid name" instead of "Ignite instance name" or "Ignite instance
> > >> name" can
> > >> >> be used without any semantic impact?
> > >> >>
> > >> >> On Sat, Dec 31, 2016 at 11:23 AM, Alexander Fedotov <
> > >> >> alexander.fedotoff@gmail.com> wrote:
> > >> >>
> > >> >>> Okay. From the all said above I suppose "instanceName" should
work
> > for
> > >> >>> IgniteConfiguration and "igniteInstanceName" in all other
places.
> > >> >>>
> > >> >>> Regards,
> > >> >>> Alexander
> > >> >>>
> > >> >>> 31 дек. 2016 г. 3:43 AM пользователь "Dmitriy
Setrakyan" <
> > >> >>> dsetrakyan@apache.org> написал:
> > >> >>>
> > >> >>> It sounds like it must be unique then. I would propose the
> > following:
> > >> >>>
> > >> >>>   1. If user defines the instanceName, then we assign it to
the
> > node.
> > >> >>>   2. If user does not define the instance name, then we have
to
> give
> > >> it
> > >> >>>   some unique value, like node ID or PID.
> > >> >>>
> > >> >>> Will this change be backward compatible, or should we leave
it as
> > >> null if
> > >> >>> user does not define it?
> > >> >>>
> > >> >>> D.
> > >> >>>
> > >> >>> On Fri, Dec 30, 2016 at 4:19 PM, Denis Magda <dmagda@gridgain.com
> >
> > >> >> wrote:
> > >> >>>
> > >> >>>> Sounds reasonable. Agree that 'instanceName' suits better
> > considering
> > >> >>> your
> > >> >>>> explanation.
> > >> >>>>
> > >> >>>> --
> > >> >>>> Denis
> > >> >>>>
> > >> >>>> On Friday, December 30, 2016, Valentin Kulichenko <
> > >> >>>> valentin.kulichenko@gmail.com> wrote:
> > >> >>>>> This name identifies instance of Ignite, in case there
are more
> > than
> > >> >>> one
> > >> >>>>> within an application. Here are our API methods around
this:
> > >> >>>>>
> > >> >>>>> // We provide a name and get newly started *Ignite*
instance.
> > >> >>>>> Ignite ignite = Ignition.start(new
> > >> >>>> IgniteConfiguration().setGridName(name));
> > >> >>>>>
> > >> >>>>> // We provide a name and get existing *Ignite* instance.
> > >> >>>>> Ignite ignite = Ignition.ignite(name);
> > >> >>>>>
> > >> >>>>> This has nothing to do with nodes. For node representation
we
> have
> > >> >>>>> ClusterNode API, which already has nodeId() method
for
> > >> >> identification.
> > >> >>>>>
> > >> >>>>> In other words, if we choose nodeName, we will have
both
> nodeName
> > >> and
> > >> >>>>> nodeId in the product, but with absolutely different
meaning and
> > >> used
> > >> >>> in
> > >> >>>>> different parts of API. How user is going to understand
the
> > >> >> difference
> > >> >>>>> between them? In my view, this is even more confusing
than
> current
> > >> >>>> gridName.
> > >> >>>>>
> > >> >>>>> -Val
> > >> >>>>>
> > >> >>>>> On Fri, Dec 30, 2016 at 2:42 PM, Denis Magda <
> dmagda@gridgain.com
> > >
> > >> >>>> wrote:
> > >> >>>>>
> > >> >>>>>> Alexander, frankly speaking I'm still for your
original
> proposal
> > -
> > >> >>>>>> nodeName. The uniqueness specificities can be
set in the doc.
> > >> >>>>>>
> > >> >>>>>> --
> > >> >>>>>> Denis
> > >> >>>>>>
> > >> >>>>>> On Friday, December 30, 2016, Alexander Fedotov
<
> > >> >>>>>> alexander.fedotoff@gmail.com> wrote:
> > >> >>>>>>> Well, then may be we should go with one of
the below names:
> > >> >>>>>>>
> > >> >>>>>>> processNodeName
> > >> >>>>>>> jvmNodeName
> > >> >>>>>>> runtimeNodeName
> > >> >>>>>>> processScopedNodeName
> > >> >>>>>>> jvmScopedNodeName
> > >> >>>>>>> runtimeScopedNodeName
> > >> >>>>>>> processWideNodeName
> > >> >>>>>>> jvmWideNodeName
> > >> >>>>>>> runtimeWideNodeName
> > >> >>>>>>>
> > >> >>>>>>> Regards,
> > >> >>>>>>> Alexander
> > >> >>>>>>>
> > >> >>>>>>> 31 дек. 2016 г. 12:37 AM пользователь
"Denis Magda" <
> > >> >>>> dmagda@apache.org>
> > >> >>>>>>> написал:
> > >> >>>>>>>
> > >> >>>>>>> The parameter specifies a node name which
has to be unique per
> > JVM
> > >> >>>>>> process
> > >> >>>>>>> (if you start multiple nodes in a single process).
In my
> > >> >>> understanding
> > >> >>>> it
> > >> >>>>>>> was mainly introduced to handle these multiple-nodes-per-JVM
> > >> >>>> scenarios.
> > >> >>>>>>>
> > >> >>>>>>> However, several nodes can have the same name
cluster wide.
> > >> >>>>>>>
> > >> >>>>>>> —
> > >> >>>>>>> Denis
> > >> >>>>>>>
> > >> >>>>>>>
> > >> >>>>>>>> On Dec 30, 2016, at 1:30 PM, Dmitriy Setrakyan
<
> > >> >>>> dsetrakyan@apache.org>
> > >> >>>>>>> wrote:
> > >> >>>>>>>>
> > >> >>>>>>>> Now I am confused. What is the purpose
of this configuration
> > >> >>>> parameter?
> > >> >>>>>>>>
> > >> >>>>>>>> On Fri, Dec 30, 2016 at 1:15 PM, Denis
Magda <
> > dmagda@apache.org>
> > >> >>>> wrote:
> > >> >>>>>>>>
> > >> >>>>>>>>> See Val’s concern in the discussion.
I’m absolutely fine
> with
> > >> >>>>>> ‘nodeName’.
> > >> >>>>>>>>>
> > >> >>>>>>>>> —
> > >> >>>>>>>>> Denis
> > >> >>>>>>>>>
> > >> >>>>>>>>>> On Dec 30, 2016, at 1:13 PM, Dmitriy
Setrakyan <
> > >> >>>> dsetrakyan@apache.org
> > >> >>>>>>>
> > >> >>>>>>>>> wrote:
> > >> >>>>>>>>>>
> > >> >>>>>>>>>> On Fri, Dec 30, 2016 at 1:12 PM,
Denis Magda <
> > >> >> dmagda@apache.org>
> > >> >>>>>> wrote:
> > >> >>>>>>>>>>
> > >> >>>>>>>>>>> What’s about ‘localNodeName’?
> > >> >>>>>>>>>>>
> > >> >>>>>>>>>>
> > >> >>>>>>>>>> Why is it better than "nodeName"?
Isn't it obvious that the
> > >> >> name
> > >> >>> is
> > >> >>>>>> for
> > >> >>>>>>>>> the
> > >> >>>>>>>>>> local node?
> > >> >>>>>>>>>
> > >> >>>>>>>>>
> > >> >>>>>>>
> > >> >>>>>>
> > >> >>>>>
> > >> >>>>
> > >> >>>
> > >> >>>
> > >> >>>
> > >> >>
> > >> >>
> > >> >> --
> > >> >> Kind regards,
> > >> >> Alexander.
> > >> >>
> > >>
> > >>
> > >
> > >
> > > --
> > > Kind regards,
> > > Alexander.
> > >
> >
> >
> >
> > --
> > Kind regards,
> > Alexander.
> >
>



-- 
Kind regards,
Alexander.

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message