Thank you Alexander, I merged these changes. I recommend everybody get latest chages from ignite-2.0.
Thanks! On Mon, Mar 13, 2017 at 1:23 PM, Alexander Fedotov < alexander.fedot...@gmail.com> wrote: > PR updated > > On Mon, Mar 13, 2017 at 1:05 PM, Alexander Fedotov < > alexander.fedot...@gmail.com> wrote: > > > Okay. Will do it shortly. > > > > On Mon, Mar 13, 2017 at 1:03 PM, Semyon Boikov <sboi...@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 <yzhda...@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.fedot...@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.fedot...@gmail.com> wrote: > >> > > > >> > > > Sure. Will take a look. > >> > > > > >> > > > On Thu, Mar 9, 2017 at 6:05 PM, Yakov Zhdanov < > yzhda...@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.fedot...@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 < > >> yzhda...@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.fedot...@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 < > >> > > >> ptupit...@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.fedot...@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.fedot...@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 < > >> > > >> dma...@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 < > >> > > >> > > ptupit...@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.fedot...@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.fedot...@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" > >> > > >> < > >> > > >> > > > > > >> >>> dsetrak...@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 < > >> > > >> > > > dma...@gridgain.com > >> > > >> > > > > > > >> > > >> > > > > > >> >> wrote: > >> > > >> > > > > > >> >>> > >> > > >> > > > > > >> >>>> Sounds reasonable. Agree that 'instanceName' > >> suits > >> > > >> better > >> > > >> > > > > > considering > >> > > >> > > > > > >> >>> your > >> > > >> > > > > > >> >>>> explanation. > >> > > >> > > > > > >> >>>> > >> > > >> > > > > > >> >>>> -- > >> > > >> > > > > > >> >>>> Denis > >> > > >> > > > > > >> >>>> > >> > > >> > > > > > >> >>>> On Friday, December 30, 2016, Valentin > >> Kulichenko < > >> > > >> > > > > > >> >>>> valentin.kuliche...@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 < > >> > > >> > > > > dma...@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.fedot...@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" < > >> > > >> > > > > > >> >>>> dma...@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 < > >> > > >> > > > > > >> >>>> dsetrak...@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 > >> < > >> > > >> > > > > > dma...@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 < > >> > > >> > > > > > >> >>>> dsetrak...@apache.org > >> > > >> > > > > > >> >>>>>>> > >> > > >> > > > > > >> >>>>>>>>> wrote: > >> > > >> > > > > > >> >>>>>>>>>> > >> > > >> > > > > > >> >>>>>>>>>> On Fri, Dec 30, 2016 at 1:12 PM, Denis > >> Magda < > >> > > >> > > > > > >> >> dma...@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. >
