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.org/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/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 <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+Ignite+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. >
