+1
2013/10/21 Stefan Egli <[email protected]> > Hi Chetan, > > Thanks. I think we should go ahead then and undo the deprecation of > ClusterView.getId() and instead making the id 'officially' more stable.. > > Cheers, > Stefan > > On 10/21/13 1:33 PM, "Chetan Mehrotra" <[email protected]> wrote: > > >Hi Stefan > > > >I have created SLING-3195 to track this. It would be better if we can > >make existing id attribute stable > >Chetan Mehrotra > > > > > >On Tue, Oct 15, 2013 at 7:45 PM, Stefan Egli <[email protected]> wrote: > >> After some offline discussion with Carsten, here's a summary of a > >>possible > >> replacement-implementation for calculating a more stable clusterid: > >> > >> The clusterId is stored in the repository (eg > >>/var/discovery/impl/clusterId) > >> The cluster leader is in charge of making sure, this id is set > >> When a node joins a cluster, it 'inherits' the clusterId and thus its > >> clusterId changes (an absolutely stable clusterId is impossible) > >> In the opposite case, ie in a network partitioning case (or a node > >>otherwise > >> leaves a cluster), they initially both have the same clusterId stored > >>in the > >> repository. If they are in different Topologies, no one notices. If > >>they are > >> in the same Topology though, you'd end up with ClusterView's with the > >>same > >> ID.. > >> > >> Re 4: This could either be seen as: > >> > >> a bug: the id would no longer be unique (in a topology) > >> a feature: it would allow to detect a network partitioning! (by checking > >> clusterview.id's uniqueness in a topology). > >> > >> The question now is: should we still deprecate ClusterView.getId() or > >>apply > >> above logic (and change the API javadoc accordingly)? > >> > >> What do people think? > >> > >> Cheers, > >> Stefan > >> > >> On 10/12/13 6:05 PM, "Carsten Ziegeler" <[email protected]> wrote: > >> > >> Exactly, thats what I wrote :) > >> > >> Carsten > >> > >> > >> 2013/10/11 Felix Meschberger <[email protected]> > >> > >> Hi > >> > >> Am 11.10.2013 um 05:45 schrieb Carsten Ziegeler: > >> > >>> Rethink this, I guess my use cases call more for a cluster name or > >> cluster > >>> description - which is a user created information. > >>> The more I think about it, the more I tend to agree that we should > >>> deprecate it and clearly state that the id is not guaranteed to be > >>>stable > >>> (for people using is already). And once my use cases are more concrete > >> than > >>> ideas, we can see how to fulfil them. > >> > >> Whatever -- deprecate or not -- I wold like to have the doc be clear: > >> Either stable or not. The longer I use APIs the more I realize that > >>notions > >> such as "not guaranteed to ..." essentially mean, that it is not usable > >>and > >> dependable. > >> > >> Regards > >> Felix > >> > >>> > >>> Carsten > >>> > >>> > >>> 2013/10/11 Stefan Egli <[email protected]> > >>> > >>>> On 10/11/13 1:35 PM, "Carsten Ziegeler" <[email protected]> wrote: > >>>> > >>>>> Hi, > >>>>> > >>>>> yes, I totally agree either we have the id stable or its of no use - > >>>>>I > >>>>> thought that it is stable, actually :) But fortunately the javadocs > >>>>>are > >>>>> unclear about this. > >>>>> I guess it heavily depends on the used implementation whether a > >>>>>stable > >> id > >>>>> is easy to implement or not > >>>> > >>>> Given that we have a cluster leader I would assume that the leader > >>>>could > >>>> define the cluster Id. However I see problems when two clusters join > >> each > >>>> other (as is possible eg in crx): there the two clusters both would > >>>>have > >>>> had 'stable' cluster ids. After the join, for one of the two parties > >>>>the > >>>> cluster id would change. Hence it might be inherently impossible to > >>>>have > >>>> stable cluster ids.. in that case, what's the gain of having it in the > >>>> API.. it would only be 'stable-but'.. > >>>> > >>>>> - I could imagine use cases where the id > >>>>> corresponds to the data center where the cluster is running and > >> therefore > >>>>> provide useful information. > >>>>> Therefore I'm a little bit hesitant to remove the method - however we > >>>>> could > >>>>> add javadocs like "it is not guaranteed that the id is stable across > >> view > >>>>> changes, however implementation should provide stable ids" > >>>> > >>>> I'd vote for either removing it (ie first deprecating it) or improving > >> it > >>>> (ie making it 'stable-but'). > >>>> > >>>> Cheers, > >>>> Stefan > >>>> > >>>>> > >>>>> WDYT? > >>>>> > >>>>> Carsten > >>>>> > >>>>> > >>>>> 2013/10/11 Stefan Egli <[email protected]> > >>>>> > >>>>>> Hi, > >>>>>> > >>>>>> The discovery's ClusterView contains a 'getId()' which returns the > >>>>>> current > >>>>>> ClusterView's ID. I'm not sure this ID is any good though. First, it > >> is > >>>>>> only valid for the lifetime of the ClusterView - ie if an instance > >>>>>> joins a > >>>>>> cluster, a new ClusterView is created, with a new ID. Second, what > >> would > >>>>>> this ID be usable for, especially given the relatively short > >>>>>> validity/lifetime of it? It does not reflect a stable, persistent > >>>>>> identifier of the cluster after all. If you want to learn which > >>>>>>other > >>>>>> instances are part of the local instance, then the API already > >> provides > >>>>>> that access. > >>>>>> > >>>>>> Comparing this to InstanceDescription.getSlingId(): that one iss by > >>>>>> definition the 'slingId' which is retrieved from SettingsService and > >> is > >>>>>> thus guaranteed to be stable/persistent. So the API is somewhat > >>>>>> inconsistent regarding the ClusterView.getId. > >>>>>> > >>>>>> I thus suggest getting rid of ClusterView.getId ie marking it as > >>>>>> deprecated. (Or, an alternative would be to come up with a stable, > >>>>>> persistent clusterId but that is potentially more complex) > >>>>>> > >>>>>> Wdyt? > >>>>>> > >>>>>> Cheers, > >>>>>> Stefan > >>>>>> > >>>>> > >>>>> > >>>>> > >>>>> -- > >>>>> Carsten Ziegeler > >>>>> [email protected] > >>>> > >>>> > >>> > >>> > >>> -- > >>> Carsten Ziegeler > >>> [email protected] > >> > >> > >> > >> > >> -- > >> Carsten Ziegeler > >> [email protected] > >> > > -- Carsten Ziegeler [email protected]
