hadoop-yarn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chris Nauroth <cnaur...@hortonworks.com>
Subject Re: [DISCUSS] what exactly are the stability guarantees of the YARN APIs
Date Tue, 31 May 2016 17:10:16 GMT
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.)

--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
View raw message