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, 13 Mar 2017 10:23:56 GMT
PR updated

On Mon, Mar 13, 2017 at 1:05 PM, Alexander Fedotov <
alexander.fedotoff@gmail.com> wrote:

> Okay. Will do it shortly.
>
> On Mon, Mar 13, 2017 at 1:03 PM, Semyon Boikov <sboikov@gridgain.com>
> wrote:
>
>> Alexander,
>>
>> I see there are conflicts again, could you plase resolve them, I'm going
>> to
>> review and merge these changes today.
>>
>> Thanks!
>>
>> On Fri, Mar 10, 2017 at 5:50 PM, Yakov Zhdanov <yzhdanov@apache.org>
>> wrote:
>>
>> > Thanks, Alex!
>> >
>> > Sam, can you please also take a look to make sure we catch all possible
>> > issues on review? Let's merge this on Monday since this is very
>> > conflict-prone change.
>> >
>> > --Yakov
>> >
>> > 2017-03-10 12:57 GMT+03:00 Alexander Fedotov <
>> alexander.fedotoff@gmail.com
>> > >:
>> >
>> > > Hi,
>> > > PR updated. Currently no conflicts at
>> > > https://github.com/apache/ignite/pull/1435.
>> > >
>> > > On Thu, Mar 9, 2017 at 6:50 PM, Alexander Fedotov <
>> > > alexander.fedotoff@gmail.com> wrote:
>> > >
>> > > > Sure. Will take a look.
>> > > >
>> > > > On Thu, Mar 9, 2017 at 6:05 PM, Yakov Zhdanov <yzhdanov@apache.org>
>> > > wrote:
>> > > >
>> > > >> Alexander,
>> > > >>
>> > > >> Page https://github.com/apache/ignite/pull/1435 reports several
>> > > >> conflicts.
>> > > >> Can you please check and resolve if necessary. Then resubmit for
>> > review
>> > > >> again.
>> > > >>
>> > > >> --Yakov
>> > > >>
>> > > >> 2017-03-03 13:24 GMT+03:00 Alexander Fedotov <
>> > > >> alexander.fedotoff@gmail.com>:
>> > > >>
>> > > >> > Hi, it's ready for review
>> > > >> > http://reviews.ignite.apache.org/ignite/review/IGNT-CR-81
>> > > >> >
>> > > >> > On Fri, Mar 3, 2017 at 11:39 AM, Yakov Zhdanov <
>> yzhdanov@apache.org
>> > >
>> > > >> > wrote:
>> > > >> >
>> > > >> > > Guys, I want to bring this up. What is the status of
this
>> ticket
>> > and
>> > > >> > > further steps?
>> > > >> > >
>> > > >> > > --Yakov
>> > > >> > >
>> > > >> > > 2017-01-30 16:37 GMT+03:00 Alexander Fedotov <
>> > > >> > alexander.fedotoff@gmail.com
>> > > >> > > >:
>> > > >> > >
>> > > >> > > > 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.o
>> rg/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/vi
>> ewLog.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+I
>> gnite+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.
>> > > >> > > >
>> > > >> > >
>> > > >> >
>> > > >> >
>> > > >> >
>> > > >> > --
>> > > >> > Kind regards,
>> > > >> > Alexander.
>> > > >> >
>> > > >>
>> > > >
>> > > >
>> > > >
>> > > > --
>> > > > Kind regards,
>> > > > Alexander.
>> > > >
>> > >
>> > >
>> > >
>> > > --
>> > > Kind regards,
>> > > Alexander.
>> > >
>> >
>>
>
>
>
> --
> Kind regards,
> Alex.
>



-- 
Kind regards,
Alex.

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