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:39:31 GMT
Ofcourse, I do understand that if changing myPrivateMethod changed the
behavior of myPublicMethod in MyPublicStable class, then that change would
be a no-go

On Fri, Oct 28, 2016 at 10:38 AM, Ravi Prakash <ravihadoop@gmail.com> wrote:

> Thanks for the clarification. "regardless of the method
> visibility/stability" confused me for a bit.
>
> On Fri, Oct 28, 2016 at 10:31 AM, Sangjin Lee <sjlee@apache.org> wrote:
>
>> No, private methods are free to change as far as the class contract is
>> concerned.
>>
>> On Fri, Oct 28, 2016 at 10:30 AM, Ravi Prakash <ravihadoop@gmail.com>
>> wrote:
>>
>>> 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