hadoop-yarn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ravi Prakash <ravihad...@gmail.com>
Subject Re: [DISCUSS] what exactly are the stability guarantees of the YARN APIs
Date Fri, 28 Oct 2016 17:30:01 GMT
Would this mean that if there is a private method in MyPublicStableClass,
changing which wouldn't break anything, could we still not change it?

On Thu, Oct 27, 2016 at 2:55 PM, Sangjin Lee <sjlee@apache.org> wrote:

> Resurrecting an old thread as part of the YARN bug bash (YARN-5130).
>
> At minimum, I believe we agree to the following (do let me know if that is
> not the case):
> (1) If the class is declared Public/Stable, no changes to the class that
> breaks the Stable contract should be made between non-major releases
> **regardless
> of the method visibility/stability**. For example, the following would
> break the stability:
> - adding a new abstract method, whether that method is stable, evolving, or
> even private
> - renaming a public method
> Although it may be possible to have methods with weaker
> stability/visibility, they still MUST not break the class contract.
>
> (2) We need to address the existing violations to ContainerStatus and
> NodeReport by adding a default implementation for **minor releases**.
> - ContainerStatus: YARN-3866 (2.8)
> - NodeReport: YARN-4293 (2.8)
>
> There are subsequent changes to ContainerStatus by YARN-2882 and YARN-5430,
> but they are marked 2.9.0. Is there going to be 2.9.0? If not, then these
> might not matter as 3.0.0 permits backward incompatible changes.
>
> Thoughts?
>
> On Tue, May 31, 2016 at 10:48 AM, Sangjin Lee <sjlee0@gmail.com> wrote:
>
> >
> >
> > On Tue, May 31, 2016 at 10:10 AM, Chris Nauroth <
> cnauroth@hortonworks.com>
> > wrote:
> >
> >> I recommend that we update the compatibility guide with some text that
> >> explicitly addresses subclassing/interface inheritance stability for
> >> classes/interfaces annotated Stable.  This is for our own benefit too.
> (I
> >> often refer back to that doc when I'm coding a patch that might have a
> >> chance of being backwards-incompatible.)
> >>
> >
> > I agree that making this distinction helps not only users but also the
> > hadoop contributors. In addition to updating the compatibility guide, how
> > about adding a new audience annotation for interfaces & abstract classes
> > that spells out whether a 3rd-party is expected to extend/implement it?
> >
> > For example, some interface can be Public/Stable for use but could be
> > off-limits in terms of extending/implementing it, while another can be
> > Public/Stable for use and allowed to be extended but with an Evolving
> > stability. It requires a little design, but should helps us a great deal
> on
> > both ends. My 2 cents.
> >
> > Sangjin
> >
> >
> >>
> >> --Chris Nauroth
> >>
> >>
> >>
> >>
> >> On 5/31/16, 9:46 AM, "Karthik Kambatla" <kasha@cloudera.com> wrote:
> >>
> >> >Argh! Totally my bad on YARN-2882. Kept missing the changes to
> >> >ContainerStatus even after you pointed out.
> >> >
> >> >Filed YARN-5184 to fix this before we release it. Thanks for pointing
> it
> >> >out, Steve.
> >> >
> >> >On Tue, May 31, 2016 at 6:00 AM, Steve Loughran <
> stevel@hortonworks.com>
> >> >wrote:
> >> >
> >> >>
> >> >> On 31 May 2016, at 05:44, Karthik Kambatla <kasha@cloudera.com
> <mailto:
> >> >> kasha@cloudera.com>> wrote:
> >> >>
> >> >> Inline.
> >> >>
> >> >> On Sat, May 28, 2016 at 11:34 AM, Sangjin Lee <sjlee0@gmail.com
> >> <mailto:
> >> >> sjlee0@gmail.com>> 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.
> >> >>
> >> >>
> >> >>
> >> >> My code wouldn't even compile because new abstract methods were added
> >> >>to a
> >> >> class tagged as stable.
> >> >>
> >> >> As far as I'm concerned, it doesn't meat the strict semantics of
> >> >>"stable",
> >> >> unless there is some nuance I'm missing.
> >> >>
> >> >> Therefore, I'm with Sangin: adding new abstract methods to an
> existing
> >> >> @Stable class breaks compatibility. Adding new non-abstract methods
> >> >>‹fine.
> >> >> It would have been straightforward to add some new methods to, say
> >> >> ContainerReport, which were no-ops/exception raising, but which at
> >> least
> >> >> didn't break compilation. (though they may have broken codepaths
> which
> >> >> required the methods to act as getters/settes)
> >> >>
> >> >> Do you think there is reason to revisit this? If yes, we should
> update
> >> >> this for Hadoop 3.
> >> >>
> >> >> I'm not sure about revisiting. I'm raising the fact that changes to
> >> >> classes marked as stable have broken code, and querying the validity
> of
> >> >> such an operation within the constraints of the 2.x codebase.
> >> >>
> >> >> And I'm raising it on yarn-dev, as that's where things broke. If we
> do
> >> >> want to revisit things, that'll mean a move to common-dev.
> >> >>
> >> >>
> >> >>
> >> >> 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. :)
> >> >>
> >> >>
> >> >> Some options.
> >> >>
> >> >> -add a specific @PluginPoint extension with different stability
> >> >> requirements.(stable, unstable, evolving). That tells implementors
> how
> >> >> likely things are to break.
> >> >>
> >> >> -Add some interface to indicate really, really, unstable. That comes
> up
> >> >> more with things like the Async FS APIs, where the discussion there
> is
> >> >> about how it may change radically.
> >> >>
> >> >> Something like @Experimental could be that. That means not just "can
> >> >> change" but "can go away"
> >> >>
> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: yarn-dev-unsubscribe@hadoop.apache.org
> >> For additional commands, e-mail: yarn-dev-help@hadoop.apache.org
> >>
> >>
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message