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] >>
