You are viewing a plain text version of this content. The canonical link for it is here.
Posted to yarn-dev@hadoop.apache.org by Sangjin Lee <sj...@apache.org> on 2016/10/27 21:55:13 UTC

Re: [DISCUSS] what exactly are the stability guarantees of the YARN APIs

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 <sj...@gmail.com> wrote:

>
>
> On Tue, May 31, 2016 at 10:10 AM, Chris Nauroth <cn...@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" <ka...@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 <st...@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
>>
>>
>

Re: [DISCUSS] what exactly are the stability guarantees of the YARN APIs

Posted by Ravi Prakash <ra...@gmail.com>.
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 <ra...@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 <sj...@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 <ra...@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 <sj...@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 <sj...@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" <ka...@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
>>> > >>
>>> > >>
>>> > >
>>> >
>>>
>>
>>
>

Re: [DISCUSS] what exactly are the stability guarantees of the YARN APIs

Posted by Sangjin Lee <sj...@apache.org>.
Yeah, I should have been precise. I was referring to the Hadoop annotations
regarding visibility/stability, not java's visibility.

On Fri, Oct 28, 2016 at 10:38 AM, Ravi Prakash <ra...@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 <sj...@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 <ra...@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 <sj...@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 <sj...@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" <ka...@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
> >> > >>
> >> > >>
> >> > >
> >> >
> >>
> >
> >
>

Re: [DISCUSS] what exactly are the stability guarantees of the YARN APIs

Posted by Ravi Prakash <ra...@gmail.com>.
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 <sj...@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 <ra...@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 <sj...@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 <sj...@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" <ka...@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
>> > >>
>> > >>
>> > >
>> >
>>
>
>

Re: [DISCUSS] what exactly are the stability guarantees of the YARN APIs

Posted by Sangjin Lee <sj...@apache.org>.
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 <ra...@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 <sj...@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 <sj...@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" <ka...@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
> > >>
> > >>
> > >
> >
>

Re: [DISCUSS] what exactly are the stability guarantees of the YARN APIs

Posted by Ravi Prakash <ra...@gmail.com>.
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 <sj...@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 <sj...@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" <ka...@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
> >>
> >>
> >
>