On Mon, May 30, 2016 at 9:44 PM, Karthik Kambatla <[email protected]> wrote:
> Inline. > > On Sat, May 28, 2016 at 11:34 AM, Sangjin Lee <[email protected]> wrote: > >> I think there is more to it. The InterfaceStability javadoc states: >> Incompatible changes must not be made to classes marked as stable. >> >> And in practice, I don't think the class annotation can be considered a >> simple sum of method annotations. There is a notion of class compatibility >> distinct from method stability. One key example is interfaces and abstract >> classes as in this case. The moment a new abstract method is added, the >> class becomes incompatible as it would break all downstream subclasses or >> implementing classes. That's the case even if *all methods are declared >> stable*. Thus, adding any abstract method (no matter what their >> scope/stability is) should be considered in violation of the stable >> contract of the class. >> > > Fair point. I was referring to them in the context of adding @Evolving > methods to @Stable classes. Our policy states that "Classes not annotated > are implicitly “Private”. Class members not annotated inherit the > annotations of the enclosing class." So, the annotation on a method > overrides that of the enclosing class. This seems pretty reasonable to me. > > Do you think there is reason to revisit this? If yes, we should update > this for Hadoop 3. > As this got clarified in this thread, I think being able to add an @Evolving method to a @Stable class is somewhat orthogonal to preserving compatibility of the @Stable class. I think an @Evolving method can be added, but only in a manner that the compatibility of the @Stable class is preserved. > > >> >> Regarding interfaces and abstract classes, one future enhancement to the >> InterfaceStability annotation we could consider is formally separating the >> contract for users of the API and the implementers of the API. They follow >> different rules. It could be feasible to have an interface as Public/Stable >> for users (anyone can use the API in a stable manner) but Private for >> implementers. The idea is that it is still a public interface but no >> third-party code should not subclass or implement it. I suspect a fair >> amount of hadoop's public interface might fall into that category. That >> itself is probably an incompatible change, so we might have to wait until >> after 3.0, however. >> > > Interesting thought. Agree that we do not anticipate users sub-classing > most of our Public-Stable classes. > > There are also classes which we do not anticipate end-users to directly > use, but devs might want to sub-class. This applies to pluggable entities; > e.g. SchedulingPolicy in fairscheduler. We are currently using > Public-Evolving to capture this intent. > > Should we add a third annotation in addition to Audience and Stability to > capture whether a class can be extended? Given the few classes we > anticipate being extended, this is likely lesser work. :) > > >> >> >> On Sat, May 28, 2016 at 10:07 AM, Karthik Kambatla <[email protected]> >> wrote: >> >>> Inline. >>> >>> On Sat, May 28, 2016 at 3:51 AM, Steve Loughran <[email protected]> >>> wrote: >>> >>> > >>> > In SLIDER-1130, I discovered that some changes to some YARN classes had >>> > broken my test code —new abstract methods had been defined, which >>> broke the >>> > subclasses I have to mock a YARN cluster. These are invaluable >>> classes, as, >>> > once eventually the hadoop regressions had been worked around, they >>> showed >>> > that the label code was broken ( YARN-5135 ) and the fixYARN-4991 not >>> > backported to branch-2 >>> > >>> > If the classes that had changed were tagged as @Private or @Evolving, >>> I'd >>> > have just said, "oh well, never mind" >>> > >>> > Except, the two interfaces that broke: ContainerStatus and NodeReport >>> —are >>> > marked @Public, @Stable >>> > >>> > Given that my code broke, I don't believe that the guarantees of a >>> @Stable >>> > interface, "Can evolve while retaining compatibility for minor release >>> > boundaries; in other words, incompatible changes to APIs marked Stable >>> are >>> > allowed only at major releases (i.e. at m.0)." are being kept. >>> > >>> > Now, in YARN-5130, we now have a patch to remark the interfaces as >>> > Unstable. >>> > >>> > However, while I can apply that patch myself, I want to raise a broader >>> > question: what does the Yarn team consider constitutes "stable"? >>> > >>> > And do that changes that took place in YARN-3886 and YARN-2882 meet >>> those >>> > requirements? New methods are going in, some of which are now being >>> tagged >>> > as @Evolving? >>> >>> >>> > I fail to see how something marked as @Stable can add @Evolving >>> methods, >>> > and given that changes are breaking downstream code, I worry that not >>> > enough care is being taken over the maintenance of the guarantee which >>> > @Public, @Stable interfaces make to users >>> > >>> >>> Are you referring to adding @Evolving methods to @Stable classes? If yes, >>> my understanding is - the annotations are per-method and the class >>> annotations are a convenience to avoid having to annotate each method. >>> >>> >>> > >>> > >>> > >>> > -Steve >>> > >>> > >>> > --------------------------------------------------------------------- >>> > To unsubscribe, e-mail: [email protected] >>> > For additional commands, e-mail: [email protected] >>> > >>> > >>> >> >> >
