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.
