You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by Bobby Evans <ev...@yahoo-inc.com.INVALID> on 2016/11/07 15:43:51 UTC
Fw: Re: [DISCUSS] breaking changes in 2.x
Made a mistake and put something on private that never should have been there. Here is the discussion in full so far.
In response to Jungtake removing the nocamel option will change set_bar/get_bar in the generated thrift code to setBar/getBar. So any thrift object that clients interact with will need to make similar changes.
- Bobby
----- Forwarded Message -----From: Bobby Evans <ev...@yahoo-inc.com>To: "private@storm.apache.org" <pr...@storm.apache.org>Sent: Monday, November 7, 2016, 3:37:50 AM CST Subject: Re: [DISCUSS] breaking changes in 2.x
Sorry you are right, I put in the wrong mailing list. I feel dumb. Will move it to dev.
- Bobby
On Sunday, November 6, 2016, 2:18:33 AM CST, P. Taylor Goetz <pt...@gmail.com> wrote:Can we move this discussion to dev@? There doesn't seem to be anything sensitive about the subject.
-Taylor
On Nov 6, 2016, at 1:28 AM, Jungtaek Lim <ka...@gmail.com> wrote:
Regarding breaking change, I'm OK to break backward UX compatibility if we're sure that it gives better UX. And since we're talking about next major, if we don't do it now, we might have no chance to do the near future.
For 1) this should be corrected, and for 2) I'm not sure how it affects to end-users, but it would be better to address if it doesn't hurt user codes much. 3) I think expected user impact is similar to what we did it for adding dependencies to shade list. Then it doesn't look matter to me.
Thanks,Jungtaek Lim (HeartSaVioR)
2016년 11월 5일 (토) 오후 11:01, Bobby Evans <bo...@apache.org>님이 작성:
I have been working on getting some of clojure code translated to java, and I have run into a few issues that I really would like to address, but will impact users. Because 2.x has not been released yet technically these should be OK, but I want to get feedback from others before I spend a lot of time on them or even file a JIRA.
1)WHAT: The thrift NodeInfo.port should be a single long, not a set of longs.USER IMPACT: it is exposed to end users through the profiling request API. BACK COMPAT: it is possible, but I would prefer not to.
2)WHAT: Using 'nocamel' when generating thrift makes generated exceptions less useful and the API less java like.USER IMPACT: users when upgrading would need to modify some API calls when setting/reading thrift objects.BACK COMPAT: most older clients should still work for a lot of things
3) WHAT: split up storm-core so we have class path separation between daemons and a true client.USER IMPACT: Users may need to include new dependencies to get what they want. Old topologies may not get access to everything that they want.BACK COMPAT: we could leave some old dependencies in place that are not needed, but why?
If we do all of these it is likely that profiling requests from an old client will not work, but for the most part a 1.x client would still work with 2.x.
- Bobby
Re: Fw: Re: [DISCUSS] breaking changes in 2.x
Posted by Bobby Evans <ev...@yahoo-inc.com.INVALID>.
For all of these we should be able to support a rolling upgrade from 1.x with some caveats.
1) There are a few possibilities on how to make this happen, but none of them are ideal. If you feel strongly about a rolling upgrade I might skip this.
2) would work for all workers that do not access objects from storm.thrift directly. (which I think should be most topologies)
3) Would require us to do changes similar to what we did when we first shaded everything. Have users compile their topologies against the newer version of storm, but override some dependencies to match the older version of storm. Then when it is deployed it could run on either version of storm.
- Bobby
On Monday, November 7, 2016, 3:56:08 AM CST, Harsha Chintalapani <st...@harsha.io> wrote:My only concern here is the rolling upgrade of storm cluster. We supported
the rolling upgrade going to 0.10 and broke it because of storm 1.x
release. Users are not inclined to upgrade to a new release if it's not
rolling upgradable. In this case, it looks like we are going to break this.
Correct me if I am wrong here.
Thanks,
Harsha
On Mon, Nov 7, 2016 at 7:43 AM Bobby Evans <ev...@yahoo-inc.com.invalid>
wrote:
> Made a mistake and put something on private that never should have been
> there. Here is the discussion in full so far.
> In response to Jungtake removing the nocamel option will change
> set_bar/get_bar in the generated thrift code to setBar/getBar. So any
> thrift object that clients interact with will need to make similar changes.
> - Bobby
>
> ----- Forwarded Message -----From: Bobby Evans <ev...@yahoo-inc.com>To: "
> private@storm.apache.org" <pr...@storm.apache.org>Sent: Monday,
> November 7, 2016, 3:37:50 AM CST Subject: Re: [DISCUSS] breaking changes in
> 2.x
> Sorry you are right, I put in the wrong mailing list. I feel dumb. Will
> move it to dev.
> - Bobby
>
> On Sunday, November 6, 2016, 2:18:33 AM CST, P. Taylor Goetz <
> ptgoetz@gmail.com> wrote:Can we move this discussion to dev@? There
> doesn't seem to be anything sensitive about the subject.
> -Taylor
> On Nov 6, 2016, at 1:28 AM, Jungtaek Lim <ka...@gmail.com> wrote:
>
>
> Regarding breaking change, I'm OK to break backward UX compatibility if
> we're sure that it gives better UX. And since we're talking about next
> major, if we don't do it now, we might have no chance to do the near future.
> For 1) this should be corrected, and for 2) I'm not sure how it affects to
> end-users, but it would be better to address if it doesn't hurt user codes
> much. 3) I think expected user impact is similar to what we did it for
> adding dependencies to shade list. Then it doesn't look matter to me.
> Thanks,Jungtaek Lim (HeartSaVioR)
> 2016년 11월 5일 (토) 오후 11:01, Bobby Evans <bo...@apache.org>님이 작성:
>
> I have been working on getting some of clojure code translated to java,
> and I have run into a few issues that I really would like to address, but
> will impact users. Because 2.x has not been released yet technically these
> should be OK, but I want to get feedback from others before I spend a lot
> of time on them or even file a JIRA.
> 1)WHAT: The thrift NodeInfo.port should be a single long, not a set of
> longs.USER IMPACT: it is exposed to end users through the profiling request
> API. BACK COMPAT: it is possible, but I would prefer not to.
>
> 2)WHAT: Using 'nocamel' when generating thrift makes generated exceptions
> less useful and the API less java like.USER IMPACT: users when upgrading
> would need to modify some API calls when setting/reading thrift
> objects.BACK COMPAT: most older clients should still work for a lot of
> things
> 3) WHAT: split up storm-core so we have class path separation between
> daemons and a true client.USER IMPACT: Users may need to include new
> dependencies to get what they want. Old topologies may not get access to
> everything that they want.BACK COMPAT: we could leave some old
> dependencies in place that are not needed, but why?
> If we do all of these it is likely that profiling requests from an old
> client will not work, but for the most part a 1.x client would still work
> with 2.x.
> - Bobby
>
>
Re: Fw: Re: [DISCUSS] breaking changes in 2.x
Posted by Harsha Chintalapani <st...@harsha.io>.
My only concern here is the rolling upgrade of storm cluster. We supported
the rolling upgrade going to 0.10 and broke it because of storm 1.x
release. Users are not inclined to upgrade to a new release if it's not
rolling upgradable. In this case, it looks like we are going to break this.
Correct me if I am wrong here.
Thanks,
Harsha
On Mon, Nov 7, 2016 at 7:43 AM Bobby Evans <ev...@yahoo-inc.com.invalid>
wrote:
> Made a mistake and put something on private that never should have been
> there. Here is the discussion in full so far.
> In response to Jungtake removing the nocamel option will change
> set_bar/get_bar in the generated thrift code to setBar/getBar. So any
> thrift object that clients interact with will need to make similar changes.
> - Bobby
>
> ----- Forwarded Message -----From: Bobby Evans <ev...@yahoo-inc.com>To: "
> private@storm.apache.org" <pr...@storm.apache.org>Sent: Monday,
> November 7, 2016, 3:37:50 AM CST Subject: Re: [DISCUSS] breaking changes in
> 2.x
> Sorry you are right, I put in the wrong mailing list. I feel dumb. Will
> move it to dev.
> - Bobby
>
> On Sunday, November 6, 2016, 2:18:33 AM CST, P. Taylor Goetz <
> ptgoetz@gmail.com> wrote:Can we move this discussion to dev@? There
> doesn't seem to be anything sensitive about the subject.
> -Taylor
> On Nov 6, 2016, at 1:28 AM, Jungtaek Lim <ka...@gmail.com> wrote:
>
>
> Regarding breaking change, I'm OK to break backward UX compatibility if
> we're sure that it gives better UX. And since we're talking about next
> major, if we don't do it now, we might have no chance to do the near future.
> For 1) this should be corrected, and for 2) I'm not sure how it affects to
> end-users, but it would be better to address if it doesn't hurt user codes
> much. 3) I think expected user impact is similar to what we did it for
> adding dependencies to shade list. Then it doesn't look matter to me.
> Thanks,Jungtaek Lim (HeartSaVioR)
> 2016년 11월 5일 (토) 오후 11:01, Bobby Evans <bo...@apache.org>님이 작성:
>
> I have been working on getting some of clojure code translated to java,
> and I have run into a few issues that I really would like to address, but
> will impact users. Because 2.x has not been released yet technically these
> should be OK, but I want to get feedback from others before I spend a lot
> of time on them or even file a JIRA.
> 1)WHAT: The thrift NodeInfo.port should be a single long, not a set of
> longs.USER IMPACT: it is exposed to end users through the profiling request
> API. BACK COMPAT: it is possible, but I would prefer not to.
>
> 2)WHAT: Using 'nocamel' when generating thrift makes generated exceptions
> less useful and the API less java like.USER IMPACT: users when upgrading
> would need to modify some API calls when setting/reading thrift
> objects.BACK COMPAT: most older clients should still work for a lot of
> things
> 3) WHAT: split up storm-core so we have class path separation between
> daemons and a true client.USER IMPACT: Users may need to include new
> dependencies to get what they want. Old topologies may not get access to
> everything that they want.BACK COMPAT: we could leave some old
> dependencies in place that are not needed, but why?
> If we do all of these it is likely that profiling requests from an old
> client will not work, but for the most part a 1.x client would still work
> with 2.x.
> - Bobby
>
>
Re: [DISCUSS] breaking changes in 2.x
Posted by Harsha Chintalapani <st...@harsha.io>.
If we want users to upgrade to new version, the rolling upgrade is a major
decision factor. As a community, we need to look API updates or breaking
changes much more diligently.
I agree to an extent we shouldn't limiting ourselves with rolling upgrade.
But having announced rolling-upgrade in 0.10 and then not supporting it in
1.x and now in 2.x. In User's point of view, Storm is not rolling
upgradable although we shipped a release stating that rolling upgrade is
supported and in follow-up release we taken that off.
Does these API changes are critical and worth breaking rolling upgrade?
-Harsha
On Mon, Nov 7, 2016 at 9:16 AM Kyle Nusbaum <kn...@yahoo-inc.com.invalid>
wrote:
I worry that making it a priority to have rolling upgrades between major
versions significantly restricts the kinds of changes that we can make,
including some kinds of changes that a major version increment is supposed
to mark. I'm not really in support of trying to do that.
If we can't make changes that break compatibility now, when can we make
them? Can changes like that ever be made? I don't know that it's good to
limit ourselves like that.
Trying to accommodate rolling upgrades is a good idea, but I'm not sure
about rejecting code changes across major versions to support them. 2.x
represents a large shift in the project, and I expect once the translation,
etc. is done, things will calm down and APIs will become more stable,
allowing more of our future releases to be rolling even across major
versions. I'd rather get these kinds of changes out of the way now in the
2.x release than cart along the vestiges of 1.x from now on.
What do others think about this?
-- Kyle
On Monday, November 7, 2016, 3:10:08 AM CST, Bobby Evans
<ev...@yahoo-inc.com.INVALID> wrote:Let's distinguish between wire
compatibility changes and API compatibility changes, along with impact to
workers vs impact to clients.
For 3) splitting the classpath up for each daemon wire compatibility is not
impacted, but we are potentially removing a bunch of APIs from the worker
and client classpath. Most of these where shaded and users should not be
impacted by them being removed, but a few like servlet-api-2.5.jar are
likely to be removed. So yes the impact here would likely be very small.
However on the client side if a topology wants to include LocalCluster
(like we do in a lot of examples) the topology jar will get a lot bigger.
LocalCluster needs access to nimbus, supervisor, and drpc server. These
would not be on the worker classpath any more and would then need to be
packaged into the topology jar to make LocalCluster work. In production I
would expect LocalCluster to be used by tests and not be included like we
do in a lot of examples. This is more of a shift for how we expect users
to interact with the LocalCluster.
For 1) NodeInfo.port depending on how we do it, it could break wire
compatibility and API compatibility (which is what I would prefer). We
could play some games to maintain compatibility, but for me it is enough of
a pain that I am not sure it is worth it. However this is not likely to
impact workers because they don't use these APIs directly. It might impact
clients but only if they have custom code to profile their topologies. IF
they use the build in CLI/UI it would not be impacted.
For 2) nocamel would break API compatibility, but not wire compatibility.
This is not likely to impact workers because like with 1 workers don't
really interact with nimbus directly so it would not be a problem. Old
clients running with older versions of storm would continue to work, but
any custom client code (think what gets run by storm jar) would need to be
recompiled/modified to be able to run on against a storm-2.x client.
- Bobby
Re: [DISCUSS] breaking changes in 2.x
Posted by "P. Taylor Goetz" <pt...@gmail.com>.
I agree with Kyle. I think trying to cling to being able to do rolling upgrades across major versions would be too limiting in terms of adding new features. If we can keep RU capability within major version lines, that seems like a fair tradeoff.
-Taylor
> On Nov 7, 2016, at 12:16 PM, Kyle Nusbaum <kn...@yahoo-inc.com.INVALID> wrote:
>
> I worry that making it a priority to have rolling upgrades between major versions significantly restricts the kinds of changes that we can make, including some kinds of changes that a major version increment is supposed to mark. I'm not really in support of trying to do that.
> If we can't make changes that break compatibility now, when can we make them? Can changes like that ever be made? I don't know that it's good to limit ourselves like that.
> Trying to accommodate rolling upgrades is a good idea, but I'm not sure about rejecting code changes across major versions to support them. 2.x represents a large shift in the project, and I expect once the translation, etc. is done, things will calm down and APIs will become more stable, allowing more of our future releases to be rolling even across major versions. I'd rather get these kinds of changes out of the way now in the 2.x release than cart along the vestiges of 1.x from now on.
> What do others think about this?
> -- Kyle
>
> On Monday, November 7, 2016, 3:10:08 AM CST, Bobby Evans <ev...@yahoo-inc.com.INVALID> wrote:Let's distinguish between wire compatibility changes and API compatibility changes, along with impact to workers vs impact to clients.
> For 3) splitting the classpath up for each daemon wire compatibility is not impacted, but we are potentially removing a bunch of APIs from the worker and client classpath. Most of these where shaded and users should not be impacted by them being removed, but a few like servlet-api-2.5.jar are likely to be removed. So yes the impact here would likely be very small.
> However on the client side if a topology wants to include LocalCluster (like we do in a lot of examples) the topology jar will get a lot bigger. LocalCluster needs access to nimbus, supervisor, and drpc server. These would not be on the worker classpath any more and would then need to be packaged into the topology jar to make LocalCluster work. In production I would expect LocalCluster to be used by tests and not be included like we do in a lot of examples. This is more of a shift for how we expect users to interact with the LocalCluster.
> For 1) NodeInfo.port depending on how we do it, it could break wire compatibility and API compatibility (which is what I would prefer). We could play some games to maintain compatibility, but for me it is enough of a pain that I am not sure it is worth it. However this is not likely to impact workers because they don't use these APIs directly. It might impact clients but only if they have custom code to profile their topologies. IF they use the build in CLI/UI it would not be impacted.
> For 2) nocamel would break API compatibility, but not wire compatibility. This is not likely to impact workers because like with 1 workers don't really interact with nimbus directly so it would not be a problem. Old clients running with older versions of storm would continue to work, but any custom client code (think what gets run by storm jar) would need to be recompiled/modified to be able to run on against a storm-2.x client.
>
> - Bobby
Re: [DISCUSS] breaking changes in 2.x
Posted by Kyle Nusbaum <kn...@yahoo-inc.com.INVALID>.
I worry that making it a priority to have rolling upgrades between major versions significantly restricts the kinds of changes that we can make, including some kinds of changes that a major version increment is supposed to mark. I'm not really in support of trying to do that.
If we can't make changes that break compatibility now, when can we make them? Can changes like that ever be made? I don't know that it's good to limit ourselves like that.
Trying to accommodate rolling upgrades is a good idea, but I'm not sure about rejecting code changes across major versions to support them. 2.x represents a large shift in the project, and I expect once the translation, etc. is done, things will calm down and APIs will become more stable, allowing more of our future releases to be rolling even across major versions. I'd rather get these kinds of changes out of the way now in the 2.x release than cart along the vestiges of 1.x from now on.
What do others think about this?
-- Kyle
On Monday, November 7, 2016, 3:10:08 AM CST, Bobby Evans <ev...@yahoo-inc.com.INVALID> wrote:Let's distinguish between wire compatibility changes and API compatibility changes, along with impact to workers vs impact to clients.
For 3) splitting the classpath up for each daemon wire compatibility is not impacted, but we are potentially removing a bunch of APIs from the worker and client classpath. Most of these where shaded and users should not be impacted by them being removed, but a few like servlet-api-2.5.jar are likely to be removed. So yes the impact here would likely be very small.
However on the client side if a topology wants to include LocalCluster (like we do in a lot of examples) the topology jar will get a lot bigger. LocalCluster needs access to nimbus, supervisor, and drpc server. These would not be on the worker classpath any more and would then need to be packaged into the topology jar to make LocalCluster work. In production I would expect LocalCluster to be used by tests and not be included like we do in a lot of examples. This is more of a shift for how we expect users to interact with the LocalCluster.
For 1) NodeInfo.port depending on how we do it, it could break wire compatibility and API compatibility (which is what I would prefer). We could play some games to maintain compatibility, but for me it is enough of a pain that I am not sure it is worth it. However this is not likely to impact workers because they don't use these APIs directly. It might impact clients but only if they have custom code to profile their topologies. IF they use the build in CLI/UI it would not be impacted.
For 2) nocamel would break API compatibility, but not wire compatibility. This is not likely to impact workers because like with 1 workers don't really interact with nimbus directly so it would not be a problem. Old clients running with older versions of storm would continue to work, but any custom client code (think what gets run by storm jar) would need to be recompiled/modified to be able to run on against a storm-2.x client.
- Bobby
Re: [DISCUSS] breaking changes in 2.x
Posted by Bobby Evans <ev...@yahoo-inc.com.INVALID>.
Let's distinguish between wire compatibility changes and API compatibility changes, along with impact to workers vs impact to clients.
For 3) splitting the classpath up for each daemon wire compatibility is not impacted, but we are potentially removing a bunch of APIs from the worker and client classpath. Most of these where shaded and users should not be impacted by them being removed, but a few like servlet-api-2.5.jar are likely to be removed. So yes the impact here would likely be very small.
However on the client side if a topology wants to include LocalCluster (like we do in a lot of examples) the topology jar will get a lot bigger. LocalCluster needs access to nimbus, supervisor, and drpc server. These would not be on the worker classpath any more and would then need to be packaged into the topology jar to make LocalCluster work. In production I would expect LocalCluster to be used by tests and not be included like we do in a lot of examples. This is more of a shift for how we expect users to interact with the LocalCluster.
For 1) NodeInfo.port depending on how we do it, it could break wire compatibility and API compatibility (which is what I would prefer). We could play some games to maintain compatibility, but for me it is enough of a pain that I am not sure it is worth it. However this is not likely to impact workers because they don't use these APIs directly. It might impact clients but only if they have custom code to profile their topologies. IF they use the build in CLI/UI it would not be impacted.
For 2) nocamel would break API compatibility, but not wire compatibility. This is not likely to impact workers because like with 1 workers don't really interact with nimbus directly so it would not be a problem. Old clients running with older versions of storm would continue to work, but any custom client code (think what gets run by storm jar) would need to be recompiled/modified to be able to run on against a storm-2.x client.
- Bobby
Re: [DISCUSS] breaking changes in 2.x
Posted by "P. Taylor Goetz" <pt...@gmail.com>.
Can you provide a hypothetical example for how #3 might break something for users?
It seems like most of the cases of backward incompatibility are not typical/average use cases, and that rolling upgrades would only be affected in certain edge cases. If that’s the case then I’m fine with the proposed changes.
-Taylor
> On Nov 7, 2016, at 10:43 AM, Bobby Evans <ev...@yahoo-inc.com.INVALID> wrote:
>
> Made a mistake and put something on private that never should have been there. Here is the discussion in full so far.
> In response to Jungtake removing the nocamel option will change set_bar/get_bar in the generated thrift code to setBar/getBar. So any thrift object that clients interact with will need to make similar changes.
> - Bobby
>
> ----- Forwarded Message -----From: Bobby Evans <ev...@yahoo-inc.com>To: "private@storm.apache.org" <pr...@storm.apache.org>Sent: Monday, November 7, 2016, 3:37:50 AM CST Subject: Re: [DISCUSS] breaking changes in 2.x
> Sorry you are right, I put in the wrong mailing list. I feel dumb. Will move it to dev.
> - Bobby
>
> On Sunday, November 6, 2016, 2:18:33 AM CST, P. Taylor Goetz <pt...@gmail.com> wrote:Can we move this discussion to dev@? There doesn't seem to be anything sensitive about the subject.
> -Taylor
> On Nov 6, 2016, at 1:28 AM, Jungtaek Lim <ka...@gmail.com> wrote:
>
>
> Regarding breaking change, I'm OK to break backward UX compatibility if we're sure that it gives better UX. And since we're talking about next major, if we don't do it now, we might have no chance to do the near future.
> For 1) this should be corrected, and for 2) I'm not sure how it affects to end-users, but it would be better to address if it doesn't hurt user codes much. 3) I think expected user impact is similar to what we did it for adding dependencies to shade list. Then it doesn't look matter to me.
> Thanks,Jungtaek Lim (HeartSaVioR)
> 2016년 11월 5일 (토) 오후 11:01, Bobby Evans <bo...@apache.org>님이 작성:
>
> I have been working on getting some of clojure code translated to java, and I have run into a few issues that I really would like to address, but will impact users. Because 2.x has not been released yet technically these should be OK, but I want to get feedback from others before I spend a lot of time on them or even file a JIRA.
> 1)WHAT: The thrift NodeInfo.port should be a single long, not a set of longs.USER IMPACT: it is exposed to end users through the profiling request API. BACK COMPAT: it is possible, but I would prefer not to.
>
> 2)WHAT: Using 'nocamel' when generating thrift makes generated exceptions less useful and the API less java like.USER IMPACT: users when upgrading would need to modify some API calls when setting/reading thrift objects.BACK COMPAT: most older clients should still work for a lot of things
> 3) WHAT: split up storm-core so we have class path separation between daemons and a true client.USER IMPACT: Users may need to include new dependencies to get what they want. Old topologies may not get access to everything that they want.BACK COMPAT: we could leave some old dependencies in place that are not needed, but why?
> If we do all of these it is likely that profiling requests from an old client will not work, but for the most part a 1.x client would still work with 2.x.
> - Bobby
>