Spark 2.4.2

classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Spark 2.4.2

Michael Armbrust
Hello All,

I know we just released Spark 2.4.1, but in light of fixing SPARK-27453 I was wondering if it might make sense to follow up quickly with 2.4.2.  Without this fix its very hard to build a datasource that correctly handles partitioning without using unstable APIs.  There are also a few other fixes that have trickled in since 2.4.1.

If there are no objections, I'd like to start the process shortly.

Michael
Reply | Threaded
Open this post in threaded view
|

Re: Spark 2.4.2

Ryan Blue
Is this a bug fix? It looks like a new feature to me.

On Tue, Apr 16, 2019 at 4:13 PM Michael Armbrust <[hidden email]> wrote:
Hello All,

I know we just released Spark 2.4.1, but in light of fixing SPARK-27453 I was wondering if it might make sense to follow up quickly with 2.4.2.  Without this fix its very hard to build a datasource that correctly handles partitioning without using unstable APIs.  There are also a few other fixes that have trickled in since 2.4.1.

If there are no objections, I'd like to start the process shortly.

Michael


--
Ryan Blue
Software Engineer
Netflix
Reply | Threaded
Open this post in threaded view
|

Re: Spark 2.4.2

Michael Armbrust
I would argue that its confusing enough to a user for options from DataFrameWriter to be silently dropped when instantiating the data source to consider this a bug.  They asked for partitioning to occur, and we are doing nothing (not even telling them we can't).  I was certainly surprised by this behavior.  Do you have a different proposal about how this should be handled?

On Tue, Apr 16, 2019 at 4:23 PM Ryan Blue <[hidden email]> wrote:
Is this a bug fix? It looks like a new feature to me.

On Tue, Apr 16, 2019 at 4:13 PM Michael Armbrust <[hidden email]> wrote:
Hello All,

I know we just released Spark 2.4.1, but in light of fixing SPARK-27453 I was wondering if it might make sense to follow up quickly with 2.4.2.  Without this fix its very hard to build a datasource that correctly handles partitioning without using unstable APIs.  There are also a few other fixes that have trickled in since 2.4.1.

If there are no objections, I'd like to start the process shortly.

Michael


--
Ryan Blue
Software Engineer
Netflix
Reply | Threaded
Open this post in threaded view
|

Re: Spark 2.4.2

Ryan Blue
Spark has a lot of strange behaviors already that we don't fix in patch releases. And bugs aren't usually fixed with a configuration flag to turn on the fix.

That said, I don't have a problem with this commit making it into a patch release. This is a small change and looks safe enough to me. I was just a little surprised since I was expecting a correctness issue if this is prompting a release. I'm definitely on the side of case-by-case judgments on what to allow in patch releases and this looks fine.

On Tue, Apr 16, 2019 at 4:27 PM Michael Armbrust <[hidden email]> wrote:
I would argue that its confusing enough to a user for options from DataFrameWriter to be silently dropped when instantiating the data source to consider this a bug.  They asked for partitioning to occur, and we are doing nothing (not even telling them we can't).  I was certainly surprised by this behavior.  Do you have a different proposal about how this should be handled?

On Tue, Apr 16, 2019 at 4:23 PM Ryan Blue <[hidden email]> wrote:
Is this a bug fix? It looks like a new feature to me.

On Tue, Apr 16, 2019 at 4:13 PM Michael Armbrust <[hidden email]> wrote:
Hello All,

I know we just released Spark 2.4.1, but in light of fixing SPARK-27453 I was wondering if it might make sense to follow up quickly with 2.4.2.  Without this fix its very hard to build a datasource that correctly handles partitioning without using unstable APIs.  There are also a few other fixes that have trickled in since 2.4.1.

If there are no objections, I'd like to start the process shortly.

Michael


--
Ryan Blue
Software Engineer
Netflix


--
Ryan Blue
Software Engineer
Netflix
Reply | Threaded
Open this post in threaded view
|

Re: Spark 2.4.2

Michael Armbrust
Thanks Ryan. To me the "test" for putting things in a maintenance release is really a trade-off between benefit and risk (along with some caveats, like user facing surface should not grow). The benefits here are fairly large (now it is possible to plug in partition aware data sources) and the risk is very low (no change in behavior by default).

And bugs aren't usually fixed with a configuration flag to turn on the fix.
 
Agree, this should be on by default in master. That would just tip the risk balance for me in a maintenance release.

On Tue, Apr 16, 2019 at 4:55 PM Ryan Blue <[hidden email]> wrote:
Spark has a lot of strange behaviors already that we don't fix in patch releases. And bugs aren't usually fixed with a configuration flag to turn on the fix.

That said, I don't have a problem with this commit making it into a patch release. This is a small change and looks safe enough to me. I was just a little surprised since I was expecting a correctness issue if this is prompting a release. I'm definitely on the side of case-by-case judgments on what to allow in patch releases and this looks fine.

On Tue, Apr 16, 2019 at 4:27 PM Michael Armbrust <[hidden email]> wrote:
I would argue that its confusing enough to a user for options from DataFrameWriter to be silently dropped when instantiating the data source to consider this a bug.  They asked for partitioning to occur, and we are doing nothing (not even telling them we can't).  I was certainly surprised by this behavior.  Do you have a different proposal about how this should be handled?

On Tue, Apr 16, 2019 at 4:23 PM Ryan Blue <[hidden email]> wrote:
Is this a bug fix? It looks like a new feature to me.

On Tue, Apr 16, 2019 at 4:13 PM Michael Armbrust <[hidden email]> wrote:
Hello All,

I know we just released Spark 2.4.1, but in light of fixing SPARK-27453 I was wondering if it might make sense to follow up quickly with 2.4.2.  Without this fix its very hard to build a datasource that correctly handles partitioning without using unstable APIs.  There are also a few other fixes that have trickled in since 2.4.1.

If there are no objections, I'd like to start the process shortly.

Michael


--
Ryan Blue
Software Engineer
Netflix


--
Ryan Blue
Software Engineer
Netflix
Reply | Threaded
Open this post in threaded view
|

Re: Spark 2.4.2

Xiao Li-2
We also found two regressions that were introduced in the maintenance release 2.4.1. Below are the fixes that were recently merged:


On Tue, Apr 16, 2019 at 5:04 PM Michael Armbrust <[hidden email]> wrote:
Thanks Ryan. To me the "test" for putting things in a maintenance release is really a trade-off between benefit and risk (along with some caveats, like user facing surface should not grow). The benefits here are fairly large (now it is possible to plug in partition aware data sources) and the risk is very low (no change in behavior by default).

And bugs aren't usually fixed with a configuration flag to turn on the fix.
 
Agree, this should be on by default in master. That would just tip the risk balance for me in a maintenance release.

On Tue, Apr 16, 2019 at 4:55 PM Ryan Blue <[hidden email]> wrote:
Spark has a lot of strange behaviors already that we don't fix in patch releases. And bugs aren't usually fixed with a configuration flag to turn on the fix.

That said, I don't have a problem with this commit making it into a patch release. This is a small change and looks safe enough to me. I was just a little surprised since I was expecting a correctness issue if this is prompting a release. I'm definitely on the side of case-by-case judgments on what to allow in patch releases and this looks fine.

On Tue, Apr 16, 2019 at 4:27 PM Michael Armbrust <[hidden email]> wrote:
I would argue that its confusing enough to a user for options from DataFrameWriter to be silently dropped when instantiating the data source to consider this a bug.  They asked for partitioning to occur, and we are doing nothing (not even telling them we can't).  I was certainly surprised by this behavior.  Do you have a different proposal about how this should be handled?

On Tue, Apr 16, 2019 at 4:23 PM Ryan Blue <[hidden email]> wrote:
Is this a bug fix? It looks like a new feature to me.

On Tue, Apr 16, 2019 at 4:13 PM Michael Armbrust <[hidden email]> wrote:
Hello All,

I know we just released Spark 2.4.1, but in light of fixing SPARK-27453 I was wondering if it might make sense to follow up quickly with 2.4.2.  Without this fix its very hard to build a datasource that correctly handles partitioning without using unstable APIs.  There are also a few other fixes that have trickled in since 2.4.1.

If there are no objections, I'd like to start the process shortly.

Michael


--
Ryan Blue
Software Engineer
Netflix


--
Ryan Blue
Software Engineer
Netflix


--
https://databricks.com/sparkaisummit/north-america?utm_source=email&utm_medium=signature
Reply | Threaded
Open this post in threaded view
|

Re: Spark 2.4.2

cloud0fan
+1 for 2.4.2.

We made a mistake in SPARK-25250, and job may hang forever when fetch failure happens. The commit has been reverted from branch 2.4, it will be great to have a 2.4.2 soon to deliver it.

The new fix is being reviewed, but I don't think it's a blocker to 2.4.2. The originally reported problem is we may retry a failed task many times and waste resource, which is not a critical bug.

Thanks,
Wenchen

On Wed, Apr 17, 2019 at 8:14 AM Xiao Li <[hidden email]> wrote:
We also found two regressions that were introduced in the maintenance release 2.4.1. Below are the fixes that were recently merged:


On Tue, Apr 16, 2019 at 5:04 PM Michael Armbrust <[hidden email]> wrote:
Thanks Ryan. To me the "test" for putting things in a maintenance release is really a trade-off between benefit and risk (along with some caveats, like user facing surface should not grow). The benefits here are fairly large (now it is possible to plug in partition aware data sources) and the risk is very low (no change in behavior by default).

And bugs aren't usually fixed with a configuration flag to turn on the fix.
 
Agree, this should be on by default in master. That would just tip the risk balance for me in a maintenance release.

On Tue, Apr 16, 2019 at 4:55 PM Ryan Blue <[hidden email]> wrote:
Spark has a lot of strange behaviors already that we don't fix in patch releases. And bugs aren't usually fixed with a configuration flag to turn on the fix.

That said, I don't have a problem with this commit making it into a patch release. This is a small change and looks safe enough to me. I was just a little surprised since I was expecting a correctness issue if this is prompting a release. I'm definitely on the side of case-by-case judgments on what to allow in patch releases and this looks fine.

On Tue, Apr 16, 2019 at 4:27 PM Michael Armbrust <[hidden email]> wrote:
I would argue that its confusing enough to a user for options from DataFrameWriter to be silently dropped when instantiating the data source to consider this a bug.  They asked for partitioning to occur, and we are doing nothing (not even telling them we can't).  I was certainly surprised by this behavior.  Do you have a different proposal about how this should be handled?

On Tue, Apr 16, 2019 at 4:23 PM Ryan Blue <[hidden email]> wrote:
Is this a bug fix? It looks like a new feature to me.

On Tue, Apr 16, 2019 at 4:13 PM Michael Armbrust <[hidden email]> wrote:
Hello All,

I know we just released Spark 2.4.1, but in light of fixing SPARK-27453 I was wondering if it might make sense to follow up quickly with 2.4.2.  Without this fix its very hard to build a datasource that correctly handles partitioning without using unstable APIs.  There are also a few other fixes that have trickled in since 2.4.1.

If there are no objections, I'd like to start the process shortly.

Michael


--
Ryan Blue
Software Engineer
Netflix


--
Ryan Blue
Software Engineer
Netflix


--
https://databricks.com/sparkaisummit/north-america?utm_source=email&utm_medium=signature
Reply | Threaded
Open this post in threaded view
|

Re: Spark 2.4.2

Ryan Blue
In reply to this post by Xiao Li-2
Xiao, you're okay with deciding on inclusion in a patch release case-by-case? In the past, you've vehemently advocated that absolutely no patches that are not strictly bug fixes should go in. I think it is a good step to accept this change, but I want to make sure you're conscious that this clearly sets a new precedent.

On Tue, Apr 16, 2019 at 5:14 PM Xiao Li <[hidden email]> wrote:
We also found two regressions that were introduced in the maintenance release 2.4.1. Below are the fixes that were recently merged:


On Tue, Apr 16, 2019 at 5:04 PM Michael Armbrust <[hidden email]> wrote:
Thanks Ryan. To me the "test" for putting things in a maintenance release is really a trade-off between benefit and risk (along with some caveats, like user facing surface should not grow). The benefits here are fairly large (now it is possible to plug in partition aware data sources) and the risk is very low (no change in behavior by default).

And bugs aren't usually fixed with a configuration flag to turn on the fix.
 
Agree, this should be on by default in master. That would just tip the risk balance for me in a maintenance release.

On Tue, Apr 16, 2019 at 4:55 PM Ryan Blue <[hidden email]> wrote:
Spark has a lot of strange behaviors already that we don't fix in patch releases. And bugs aren't usually fixed with a configuration flag to turn on the fix.

That said, I don't have a problem with this commit making it into a patch release. This is a small change and looks safe enough to me. I was just a little surprised since I was expecting a correctness issue if this is prompting a release. I'm definitely on the side of case-by-case judgments on what to allow in patch releases and this looks fine.

On Tue, Apr 16, 2019 at 4:27 PM Michael Armbrust <[hidden email]> wrote:
I would argue that its confusing enough to a user for options from DataFrameWriter to be silently dropped when instantiating the data source to consider this a bug.  They asked for partitioning to occur, and we are doing nothing (not even telling them we can't).  I was certainly surprised by this behavior.  Do you have a different proposal about how this should be handled?

On Tue, Apr 16, 2019 at 4:23 PM Ryan Blue <[hidden email]> wrote:
Is this a bug fix? It looks like a new feature to me.

On Tue, Apr 16, 2019 at 4:13 PM Michael Armbrust <[hidden email]> wrote:
Hello All,

I know we just released Spark 2.4.1, but in light of fixing SPARK-27453 I was wondering if it might make sense to follow up quickly with 2.4.2.  Without this fix its very hard to build a datasource that correctly handles partitioning without using unstable APIs.  There are also a few other fixes that have trickled in since 2.4.1.

If there are no objections, I'd like to start the process shortly.

Michael


--
Ryan Blue
Software Engineer
Netflix


--
Ryan Blue
Software Engineer
Netflix


--
https://databricks.com/sparkaisummit/north-america?utm_source=email&utm_medium=signature


--
Ryan Blue
Software Engineer
Netflix
Reply | Threaded
Open this post in threaded view
|

Re: Spark 2.4.2

Xiao Li-2
Michael and I had an offline discussion about this PR https://github.com/apache/spark/pull/24365. He convinced me that this is a bug fix. The code changes of this bug fix are very tiny and the risk is very low. To avoid any behavior change in the patch releases, this PR also added a legacy flag whose default value is off. 


 

On Wed, Apr 17, 2019 at 8:55 AM Ryan Blue <[hidden email]> wrote:
Xiao, you're okay with deciding on inclusion in a patch release case-by-case? In the past, you've vehemently advocated that absolutely no patches that are not strictly bug fixes should go in. I think it is a good step to accept this change, but I want to make sure you're conscious that this clearly sets a new precedent.

On Tue, Apr 16, 2019 at 5:14 PM Xiao Li <[hidden email]> wrote:
We also found two regressions that were introduced in the maintenance release 2.4.1. Below are the fixes that were recently merged:


On Tue, Apr 16, 2019 at 5:04 PM Michael Armbrust <[hidden email]> wrote:
Thanks Ryan. To me the "test" for putting things in a maintenance release is really a trade-off between benefit and risk (along with some caveats, like user facing surface should not grow). The benefits here are fairly large (now it is possible to plug in partition aware data sources) and the risk is very low (no change in behavior by default).

And bugs aren't usually fixed with a configuration flag to turn on the fix.
 
Agree, this should be on by default in master. That would just tip the risk balance for me in a maintenance release.

On Tue, Apr 16, 2019 at 4:55 PM Ryan Blue <[hidden email]> wrote:
Spark has a lot of strange behaviors already that we don't fix in patch releases. And bugs aren't usually fixed with a configuration flag to turn on the fix.

That said, I don't have a problem with this commit making it into a patch release. This is a small change and looks safe enough to me. I was just a little surprised since I was expecting a correctness issue if this is prompting a release. I'm definitely on the side of case-by-case judgments on what to allow in patch releases and this looks fine.

On Tue, Apr 16, 2019 at 4:27 PM Michael Armbrust <[hidden email]> wrote:
I would argue that its confusing enough to a user for options from DataFrameWriter to be silently dropped when instantiating the data source to consider this a bug.  They asked for partitioning to occur, and we are doing nothing (not even telling them we can't).  I was certainly surprised by this behavior.  Do you have a different proposal about how this should be handled?

On Tue, Apr 16, 2019 at 4:23 PM Ryan Blue <[hidden email]> wrote:
Is this a bug fix? It looks like a new feature to me.

On Tue, Apr 16, 2019 at 4:13 PM Michael Armbrust <[hidden email]> wrote:
Hello All,

I know we just released Spark 2.4.1, but in light of fixing SPARK-27453 I was wondering if it might make sense to follow up quickly with 2.4.2.  Without this fix its very hard to build a datasource that correctly handles partitioning without using unstable APIs.  There are also a few other fixes that have trickled in since 2.4.1.

If there are no objections, I'd like to start the process shortly.

Michael


--
Ryan Blue
Software Engineer
Netflix


--
Ryan Blue
Software Engineer
Netflix


--
https://databricks.com/sparkaisummit/north-america?utm_source=email&utm_medium=signature


--
Ryan Blue
Software Engineer
Netflix


--
https://databricks.com/sparkaisummit/north-america?utm_source=email&utm_medium=signature
Reply | Threaded
Open this post in threaded view
|

Re: Spark 2.4.2

Ryan Blue
How is this a bug fix?

On Wed, Apr 17, 2019 at 9:30 AM Xiao Li <[hidden email]> wrote:
Michael and I had an offline discussion about this PR https://github.com/apache/spark/pull/24365. He convinced me that this is a bug fix. The code changes of this bug fix are very tiny and the risk is very low. To avoid any behavior change in the patch releases, this PR also added a legacy flag whose default value is off. 


 

On Wed, Apr 17, 2019 at 8:55 AM Ryan Blue <[hidden email]> wrote:
Xiao, you're okay with deciding on inclusion in a patch release case-by-case? In the past, you've vehemently advocated that absolutely no patches that are not strictly bug fixes should go in. I think it is a good step to accept this change, but I want to make sure you're conscious that this clearly sets a new precedent.

On Tue, Apr 16, 2019 at 5:14 PM Xiao Li <[hidden email]> wrote:
We also found two regressions that were introduced in the maintenance release 2.4.1. Below are the fixes that were recently merged:


On Tue, Apr 16, 2019 at 5:04 PM Michael Armbrust <[hidden email]> wrote:
Thanks Ryan. To me the "test" for putting things in a maintenance release is really a trade-off between benefit and risk (along with some caveats, like user facing surface should not grow). The benefits here are fairly large (now it is possible to plug in partition aware data sources) and the risk is very low (no change in behavior by default).

And bugs aren't usually fixed with a configuration flag to turn on the fix.
 
Agree, this should be on by default in master. That would just tip the risk balance for me in a maintenance release.

On Tue, Apr 16, 2019 at 4:55 PM Ryan Blue <[hidden email]> wrote:
Spark has a lot of strange behaviors already that we don't fix in patch releases. And bugs aren't usually fixed with a configuration flag to turn on the fix.

That said, I don't have a problem with this commit making it into a patch release. This is a small change and looks safe enough to me. I was just a little surprised since I was expecting a correctness issue if this is prompting a release. I'm definitely on the side of case-by-case judgments on what to allow in patch releases and this looks fine.

On Tue, Apr 16, 2019 at 4:27 PM Michael Armbrust <[hidden email]> wrote:
I would argue that its confusing enough to a user for options from DataFrameWriter to be silently dropped when instantiating the data source to consider this a bug.  They asked for partitioning to occur, and we are doing nothing (not even telling them we can't).  I was certainly surprised by this behavior.  Do you have a different proposal about how this should be handled?

On Tue, Apr 16, 2019 at 4:23 PM Ryan Blue <[hidden email]> wrote:
Is this a bug fix? It looks like a new feature to me.

On Tue, Apr 16, 2019 at 4:13 PM Michael Armbrust <[hidden email]> wrote:
Hello All,

I know we just released Spark 2.4.1, but in light of fixing SPARK-27453 I was wondering if it might make sense to follow up quickly with 2.4.2.  Without this fix its very hard to build a datasource that correctly handles partitioning without using unstable APIs.  There are also a few other fixes that have trickled in since 2.4.1.

If there are no objections, I'd like to start the process shortly.

Michael


--
Ryan Blue
Software Engineer
Netflix


--
Ryan Blue
Software Engineer
Netflix


--
https://databricks.com/sparkaisummit/north-america?utm_source=email&utm_medium=signature


--
Ryan Blue
Software Engineer
Netflix


--
https://databricks.com/sparkaisummit/north-america?utm_source=email&utm_medium=signature


--
Ryan Blue
Software Engineer
Netflix
Reply | Threaded
Open this post in threaded view
|

Re: Spark 2.4.2

Ryan Blue
Sorry, I should be more clear about what I'm trying to say here.

In the past, Xiao has taken the opposite stance. A good example is PR #21060 that was a very similar situation: behavior didn't match what was expected and there was low risk. There was a long argument and the patch didn't make it into 2.3 (to my knowledge).

What we call these low-risk behavior fixes doesn't matter. I called it a bug on #21060 but I'm applying Xiao's previous definition here to make a point. Whatever term we use, we clearly have times when we want to allow a patch because it is low risk and helps someone. Let's just be clear that that's perfectly fine.

On Wed, Apr 17, 2019 at 9:34 AM Ryan Blue <[hidden email]> wrote:
How is this a bug fix?

On Wed, Apr 17, 2019 at 9:30 AM Xiao Li <[hidden email]> wrote:
Michael and I had an offline discussion about this PR https://github.com/apache/spark/pull/24365. He convinced me that this is a bug fix. The code changes of this bug fix are very tiny and the risk is very low. To avoid any behavior change in the patch releases, this PR also added a legacy flag whose default value is off. 


 

On Wed, Apr 17, 2019 at 8:55 AM Ryan Blue <[hidden email]> wrote:
Xiao, you're okay with deciding on inclusion in a patch release case-by-case? In the past, you've vehemently advocated that absolutely no patches that are not strictly bug fixes should go in. I think it is a good step to accept this change, but I want to make sure you're conscious that this clearly sets a new precedent.

On Tue, Apr 16, 2019 at 5:14 PM Xiao Li <[hidden email]> wrote:
We also found two regressions that were introduced in the maintenance release 2.4.1. Below are the fixes that were recently merged:


On Tue, Apr 16, 2019 at 5:04 PM Michael Armbrust <[hidden email]> wrote:
Thanks Ryan. To me the "test" for putting things in a maintenance release is really a trade-off between benefit and risk (along with some caveats, like user facing surface should not grow). The benefits here are fairly large (now it is possible to plug in partition aware data sources) and the risk is very low (no change in behavior by default).

And bugs aren't usually fixed with a configuration flag to turn on the fix.
 
Agree, this should be on by default in master. That would just tip the risk balance for me in a maintenance release.

On Tue, Apr 16, 2019 at 4:55 PM Ryan Blue <[hidden email]> wrote:
Spark has a lot of strange behaviors already that we don't fix in patch releases. And bugs aren't usually fixed with a configuration flag to turn on the fix.

That said, I don't have a problem with this commit making it into a patch release. This is a small change and looks safe enough to me. I was just a little surprised since I was expecting a correctness issue if this is prompting a release. I'm definitely on the side of case-by-case judgments on what to allow in patch releases and this looks fine.

On Tue, Apr 16, 2019 at 4:27 PM Michael Armbrust <[hidden email]> wrote:
I would argue that its confusing enough to a user for options from DataFrameWriter to be silently dropped when instantiating the data source to consider this a bug.  They asked for partitioning to occur, and we are doing nothing (not even telling them we can't).  I was certainly surprised by this behavior.  Do you have a different proposal about how this should be handled?

On Tue, Apr 16, 2019 at 4:23 PM Ryan Blue <[hidden email]> wrote:
Is this a bug fix? It looks like a new feature to me.

On Tue, Apr 16, 2019 at 4:13 PM Michael Armbrust <[hidden email]> wrote:
Hello All,

I know we just released Spark 2.4.1, but in light of fixing SPARK-27453 I was wondering if it might make sense to follow up quickly with 2.4.2.  Without this fix its very hard to build a datasource that correctly handles partitioning without using unstable APIs.  There are also a few other fixes that have trickled in since 2.4.1.

If there are no objections, I'd like to start the process shortly.

Michael


--
Ryan Blue
Software Engineer
Netflix


--
Ryan Blue
Software Engineer
Netflix


--
https://databricks.com/sparkaisummit/north-america?utm_source=email&utm_medium=signature


--
Ryan Blue
Software Engineer
Netflix


--
https://databricks.com/sparkaisummit/north-america?utm_source=email&utm_medium=signature


--
Ryan Blue
Software Engineer
Netflix


--
Ryan Blue
Software Engineer
Netflix
Reply | Threaded
Open this post in threaded view
|

Re: Spark 2.4.2

Sean Owen-2
I think the 'only backport bug fixes to branches' principle remains sound. But what's a bug fix? Something that changes behavior to match what is explicitly supposed to happen, or implicitly supposed to happen -- implied by what other similar things do, by reasonable user expectations, or simply how it worked previously.

Is this a bug fix? I guess the criteria that matches is that behavior doesn't match reasonable user expectations? I don't know enough to have a strong opinion. I also don't think there is currently an objection to backporting it, whatever it's called.


Is the question whether this needs a new release? There's no harm in another point release, other than needing a volunteer release manager. One could say, wait a bit longer to see what more info comes in about 2.4.1. But given that 2.4.1 took like 2 months, it's reasonable to move towards a release cycle again. I don't see objection to that either (?)


The meta question remains: is a 'bug fix' definition even agreed, and being consistently applied? There aren't correct answers, only best guesses from each person's own experience, judgment and priorities. These can differ even when applied in good faith.

Sometimes the variance of opinion comes because people have different info that needs to be surfaced. Here, maybe it's best to share what about that offline conversation was convincing, for example.

I'd say it's also important to separate what one would prefer from what one can't live with(out). Assuming one trusts the intent and experience of the handful of others with an opinion, I'd defer to someone who wants X and will own it, even if I'm moderately against it. Otherwise we'd get little done.

In that light, it seems like both of the PRs at issue here are not _wrong_ to backport. This is a good pair that highlights why, when there isn't a clear reason to do / not do something (e.g. obvious errors, breaking public APIs) we give benefit-of-the-doubt in order to get it later.


On Wed, Apr 17, 2019 at 12:09 PM Ryan Blue <[hidden email]> wrote:
Sorry, I should be more clear about what I'm trying to say here.

In the past, Xiao has taken the opposite stance. A good example is PR #21060 that was a very similar situation: behavior didn't match what was expected and there was low risk. There was a long argument and the patch didn't make it into 2.3 (to my knowledge).

What we call these low-risk behavior fixes doesn't matter. I called it a bug on #21060 but I'm applying Xiao's previous definition here to make a point. Whatever term we use, we clearly have times when we want to allow a patch because it is low risk and helps someone. Let's just be clear that that's perfectly fine.

On Wed, Apr 17, 2019 at 9:34 AM Ryan Blue <[hidden email]> wrote:
How is this a bug fix?

On Wed, Apr 17, 2019 at 9:30 AM Xiao Li <[hidden email]> wrote:
Michael and I had an offline discussion about this PR https://github.com/apache/spark/pull/24365. He convinced me that this is a bug fix. The code changes of this bug fix are very tiny and the risk is very low. To avoid any behavior change in the patch releases, this PR also added a legacy flag whose default value is off. 

Reply | Threaded
Open this post in threaded view
|

Re: Spark 2.4.2

cloud0fan
I volunteer to be the release manager for 2.4.2, as I was also going to propose 2.4.2 because of the reverting of SPARK-25250. Is there any other ongoing bug fixes we want to include in 2.4.2? If no I'd like to start the release process today (CST).

Thanks,
Wenchen

On Thu, Apr 18, 2019 at 3:44 AM Sean Owen <[hidden email]> wrote:
I think the 'only backport bug fixes to branches' principle remains sound. But what's a bug fix? Something that changes behavior to match what is explicitly supposed to happen, or implicitly supposed to happen -- implied by what other similar things do, by reasonable user expectations, or simply how it worked previously.

Is this a bug fix? I guess the criteria that matches is that behavior doesn't match reasonable user expectations? I don't know enough to have a strong opinion. I also don't think there is currently an objection to backporting it, whatever it's called.


Is the question whether this needs a new release? There's no harm in another point release, other than needing a volunteer release manager. One could say, wait a bit longer to see what more info comes in about 2.4.1. But given that 2.4.1 took like 2 months, it's reasonable to move towards a release cycle again. I don't see objection to that either (?)


The meta question remains: is a 'bug fix' definition even agreed, and being consistently applied? There aren't correct answers, only best guesses from each person's own experience, judgment and priorities. These can differ even when applied in good faith.

Sometimes the variance of opinion comes because people have different info that needs to be surfaced. Here, maybe it's best to share what about that offline conversation was convincing, for example.

I'd say it's also important to separate what one would prefer from what one can't live with(out). Assuming one trusts the intent and experience of the handful of others with an opinion, I'd defer to someone who wants X and will own it, even if I'm moderately against it. Otherwise we'd get little done.

In that light, it seems like both of the PRs at issue here are not _wrong_ to backport. This is a good pair that highlights why, when there isn't a clear reason to do / not do something (e.g. obvious errors, breaking public APIs) we give benefit-of-the-doubt in order to get it later.


On Wed, Apr 17, 2019 at 12:09 PM Ryan Blue <[hidden email]> wrote:
Sorry, I should be more clear about what I'm trying to say here.

In the past, Xiao has taken the opposite stance. A good example is PR #21060 that was a very similar situation: behavior didn't match what was expected and there was low risk. There was a long argument and the patch didn't make it into 2.3 (to my knowledge).

What we call these low-risk behavior fixes doesn't matter. I called it a bug on #21060 but I'm applying Xiao's previous definition here to make a point. Whatever term we use, we clearly have times when we want to allow a patch because it is low risk and helps someone. Let's just be clear that that's perfectly fine.

On Wed, Apr 17, 2019 at 9:34 AM Ryan Blue <[hidden email]> wrote:
How is this a bug fix?

On Wed, Apr 17, 2019 at 9:30 AM Xiao Li <[hidden email]> wrote:
Michael and I had an offline discussion about this PR https://github.com/apache/spark/pull/24365. He convinced me that this is a bug fix. The code changes of this bug fix are very tiny and the risk is very low. To avoid any behavior change in the patch releases, this PR also added a legacy flag whose default value is off. 

Reply | Threaded
Open this post in threaded view
|

Re: Spark 2.4.2

Sean Owen-2
There's only one other item on my radar, which is considering updating
Jackson to 2.9 in branch-2.4 to get security fixes. Pros: it's come up
a few times now that there are a number of CVEs open for 2.6.7. Cons:
not clear they affect Spark, and Jackson 2.6->2.9 does change Jackson
behavior non-trivially. That said back-porting the update PR to 2.4
worked out OK locally. Any strong opinions on this one?

On Wed, Apr 17, 2019 at 7:49 PM Wenchen Fan <[hidden email]> wrote:

>
> I volunteer to be the release manager for 2.4.2, as I was also going to propose 2.4.2 because of the reverting of SPARK-25250. Is there any other ongoing bug fixes we want to include in 2.4.2? If no I'd like to start the release process today (CST).
>
> Thanks,
> Wenchen
>
> On Thu, Apr 18, 2019 at 3:44 AM Sean Owen <[hidden email]> wrote:
>>
>> I think the 'only backport bug fixes to branches' principle remains sound. But what's a bug fix? Something that changes behavior to match what is explicitly supposed to happen, or implicitly supposed to happen -- implied by what other similar things do, by reasonable user expectations, or simply how it worked previously.
>>
>> Is this a bug fix? I guess the criteria that matches is that behavior doesn't match reasonable user expectations? I don't know enough to have a strong opinion. I also don't think there is currently an objection to backporting it, whatever it's called.
>>
>>
>> Is the question whether this needs a new release? There's no harm in another point release, other than needing a volunteer release manager. One could say, wait a bit longer to see what more info comes in about 2.4.1. But given that 2.4.1 took like 2 months, it's reasonable to move towards a release cycle again. I don't see objection to that either (?)
>>
>>
>> The meta question remains: is a 'bug fix' definition even agreed, and being consistently applied? There aren't correct answers, only best guesses from each person's own experience, judgment and priorities. These can differ even when applied in good faith.
>>
>> Sometimes the variance of opinion comes because people have different info that needs to be surfaced. Here, maybe it's best to share what about that offline conversation was convincing, for example.
>>
>> I'd say it's also important to separate what one would prefer from what one can't live with(out). Assuming one trusts the intent and experience of the handful of others with an opinion, I'd defer to someone who wants X and will own it, even if I'm moderately against it. Otherwise we'd get little done.
>>
>> In that light, it seems like both of the PRs at issue here are not _wrong_ to backport. This is a good pair that highlights why, when there isn't a clear reason to do / not do something (e.g. obvious errors, breaking public APIs) we give benefit-of-the-doubt in order to get it later.
>>
>>
>> On Wed, Apr 17, 2019 at 12:09 PM Ryan Blue <[hidden email]> wrote:
>>>
>>> Sorry, I should be more clear about what I'm trying to say here.
>>>
>>> In the past, Xiao has taken the opposite stance. A good example is PR #21060 that was a very similar situation: behavior didn't match what was expected and there was low risk. There was a long argument and the patch didn't make it into 2.3 (to my knowledge).
>>>
>>> What we call these low-risk behavior fixes doesn't matter. I called it a bug on #21060 but I'm applying Xiao's previous definition here to make a point. Whatever term we use, we clearly have times when we want to allow a patch because it is low risk and helps someone. Let's just be clear that that's perfectly fine.
>>>
>>> On Wed, Apr 17, 2019 at 9:34 AM Ryan Blue <[hidden email]> wrote:
>>>>
>>>> How is this a bug fix?
>>>>
>>>> On Wed, Apr 17, 2019 at 9:30 AM Xiao Li <[hidden email]> wrote:
>>>>>
>>>>> Michael and I had an offline discussion about this PR https://github.com/apache/spark/pull/24365. He convinced me that this is a bug fix. The code changes of this bug fix are very tiny and the risk is very low. To avoid any behavior change in the patch releases, this PR also added a legacy flag whose default value is off.
>>>>>

---------------------------------------------------------------------
To unsubscribe e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Spark 2.4.2

rxin
For Jackson - are you worrying about JSON parsing for users or internal Spark functionality breaking?

On Wed, Apr 17, 2019 at 6:02 PM Sean Owen <[hidden email]> wrote:
There's only one other item on my radar, which is considering updating
Jackson to 2.9 in branch-2.4 to get security fixes. Pros: it's come up
a few times now that there are a number of CVEs open for 2.6.7. Cons:
not clear they affect Spark, and Jackson 2.6->2.9 does change Jackson
behavior non-trivially. That said back-porting the update PR to 2.4
worked out OK locally. Any strong opinions on this one?

On Wed, Apr 17, 2019 at 7:49 PM Wenchen Fan <[hidden email]> wrote:
>
> I volunteer to be the release manager for 2.4.2, as I was also going to propose 2.4.2 because of the reverting of SPARK-25250. Is there any other ongoing bug fixes we want to include in 2.4.2? If no I'd like to start the release process today (CST).
>
> Thanks,
> Wenchen
>
> On Thu, Apr 18, 2019 at 3:44 AM Sean Owen <[hidden email]> wrote:
>>
>> I think the 'only backport bug fixes to branches' principle remains sound. But what's a bug fix? Something that changes behavior to match what is explicitly supposed to happen, or implicitly supposed to happen -- implied by what other similar things do, by reasonable user expectations, or simply how it worked previously.
>>
>> Is this a bug fix? I guess the criteria that matches is that behavior doesn't match reasonable user expectations? I don't know enough to have a strong opinion. I also don't think there is currently an objection to backporting it, whatever it's called.
>>
>>
>> Is the question whether this needs a new release? There's no harm in another point release, other than needing a volunteer release manager. One could say, wait a bit longer to see what more info comes in about 2.4.1. But given that 2.4.1 took like 2 months, it's reasonable to move towards a release cycle again. I don't see objection to that either (?)
>>
>>
>> The meta question remains: is a 'bug fix' definition even agreed, and being consistently applied? There aren't correct answers, only best guesses from each person's own experience, judgment and priorities. These can differ even when applied in good faith.
>>
>> Sometimes the variance of opinion comes because people have different info that needs to be surfaced. Here, maybe it's best to share what about that offline conversation was convincing, for example.
>>
>> I'd say it's also important to separate what one would prefer from what one can't live with(out). Assuming one trusts the intent and experience of the handful of others with an opinion, I'd defer to someone who wants X and will own it, even if I'm moderately against it. Otherwise we'd get little done.
>>
>> In that light, it seems like both of the PRs at issue here are not _wrong_ to backport. This is a good pair that highlights why, when there isn't a clear reason to do / not do something (e.g. obvious errors, breaking public APIs) we give benefit-of-the-doubt in order to get it later.
>>
>>
>> On Wed, Apr 17, 2019 at 12:09 PM Ryan Blue <[hidden email]> wrote:
>>>
>>> Sorry, I should be more clear about what I'm trying to say here.
>>>
>>> In the past, Xiao has taken the opposite stance. A good example is PR #21060 that was a very similar situation: behavior didn't match what was expected and there was low risk. There was a long argument and the patch didn't make it into 2.3 (to my knowledge).
>>>
>>> What we call these low-risk behavior fixes doesn't matter. I called it a bug on #21060 but I'm applying Xiao's previous definition here to make a point. Whatever term we use, we clearly have times when we want to allow a patch because it is low risk and helps someone. Let's just be clear that that's perfectly fine.
>>>
>>> On Wed, Apr 17, 2019 at 9:34 AM Ryan Blue <[hidden email]> wrote:
>>>>
>>>> How is this a bug fix?
>>>>
>>>> On Wed, Apr 17, 2019 at 9:30 AM Xiao Li <[hidden email]> wrote:
>>>>>
>>>>> Michael and I had an offline discussion about this PR https://github.com/apache/spark/pull/24365. He convinced me that this is a bug fix. The code changes of this bug fix are very tiny and the risk is very low. To avoid any behavior change in the patch releases, this PR also added a legacy flag whose default value is off.
>>>>>

---------------------------------------------------------------------
To unsubscribe e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Spark 2.4.2

Sean Owen-2
For users that would inherit Jackson and use it directly, or whose
dependencies do. Spark itself (with modifications) should be OK with
the change.
It's risky and normally wouldn't backport, except that I've heard a
few times about concerns about CVEs affecting Databind, so wondering
who else out there might have an opinion. I'm not pushing for it
necessarily.

On Wed, Apr 17, 2019 at 6:18 PM Reynold Xin <[hidden email]> wrote:

>
> For Jackson - are you worrying about JSON parsing for users or internal Spark functionality breaking?
>
> On Wed, Apr 17, 2019 at 6:02 PM Sean Owen <[hidden email]> wrote:
>>
>> There's only one other item on my radar, which is considering updating
>> Jackson to 2.9 in branch-2.4 to get security fixes. Pros: it's come up
>> a few times now that there are a number of CVEs open for 2.6.7. Cons:
>> not clear they affect Spark, and Jackson 2.6->2.9 does change Jackson
>> behavior non-trivially. That said back-porting the update PR to 2.4
>> worked out OK locally. Any strong opinions on this one?
>>
>> On Wed, Apr 17, 2019 at 7:49 PM Wenchen Fan <[hidden email]> wrote:
>> >
>> > I volunteer to be the release manager for 2.4.2, as I was also going to propose 2.4.2 because of the reverting of SPARK-25250. Is there any other ongoing bug fixes we want to include in 2.4.2? If no I'd like to start the release process today (CST).
>> >
>> > Thanks,
>> > Wenchen
>> >
>> > On Thu, Apr 18, 2019 at 3:44 AM Sean Owen <[hidden email]> wrote:
>> >>
>> >> I think the 'only backport bug fixes to branches' principle remains sound. But what's a bug fix? Something that changes behavior to match what is explicitly supposed to happen, or implicitly supposed to happen -- implied by what other similar things do, by reasonable user expectations, or simply how it worked previously.
>> >>
>> >> Is this a bug fix? I guess the criteria that matches is that behavior doesn't match reasonable user expectations? I don't know enough to have a strong opinion. I also don't think there is currently an objection to backporting it, whatever it's called.
>> >>
>> >>
>> >> Is the question whether this needs a new release? There's no harm in another point release, other than needing a volunteer release manager. One could say, wait a bit longer to see what more info comes in about 2.4.1. But given that 2.4.1 took like 2 months, it's reasonable to move towards a release cycle again. I don't see objection to that either (?)
>> >>
>> >>
>> >> The meta question remains: is a 'bug fix' definition even agreed, and being consistently applied? There aren't correct answers, only best guesses from each person's own experience, judgment and priorities. These can differ even when applied in good faith.
>> >>
>> >> Sometimes the variance of opinion comes because people have different info that needs to be surfaced. Here, maybe it's best to share what about that offline conversation was convincing, for example.
>> >>
>> >> I'd say it's also important to separate what one would prefer from what one can't live with(out). Assuming one trusts the intent and experience of the handful of others with an opinion, I'd defer to someone who wants X and will own it, even if I'm moderately against it. Otherwise we'd get little done.
>> >>
>> >> In that light, it seems like both of the PRs at issue here are not _wrong_ to backport. This is a good pair that highlights why, when there isn't a clear reason to do / not do something (e.g. obvious errors, breaking public APIs) we give benefit-of-the-doubt in order to get it later.
>> >>
>> >>
>> >> On Wed, Apr 17, 2019 at 12:09 PM Ryan Blue <[hidden email]> wrote:
>> >>>
>> >>> Sorry, I should be more clear about what I'm trying to say here.
>> >>>
>> >>> In the past, Xiao has taken the opposite stance. A good example is PR #21060 that was a very similar situation: behavior didn't match what was expected and there was low risk. There was a long argument and the patch didn't make it into 2.3 (to my knowledge).
>> >>>
>> >>> What we call these low-risk behavior fixes doesn't matter. I called it a bug on #21060 but I'm applying Xiao's previous definition here to make a point. Whatever term we use, we clearly have times when we want to allow a patch because it is low risk and helps someone. Let's just be clear that that's perfectly fine.
>> >>>
>> >>> On Wed, Apr 17, 2019 at 9:34 AM Ryan Blue <[hidden email]> wrote:
>> >>>>
>> >>>> How is this a bug fix?
>> >>>>
>> >>>> On Wed, Apr 17, 2019 at 9:30 AM Xiao Li <[hidden email]> wrote:
>> >>>>>
>> >>>>> Michael and I had an offline discussion about this PR https://github.com/apache/spark/pull/24365. He convinced me that this is a bug fix. The code changes of this bug fix are very tiny and the risk is very low. To avoid any behavior change in the patch releases, this PR also added a legacy flag whose default value is off.
>> >>>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe e-mail: [hidden email]
>>

---------------------------------------------------------------------
To unsubscribe e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Spark 2.4.2

rxin
We should have shaded all Spark’s dependencies :(

On Wed, Apr 17, 2019 at 11:47 PM Sean Owen <[hidden email]> wrote:
For users that would inherit Jackson and use it directly, or whose
dependencies do. Spark itself (with modifications) should be OK with
the change.
It's risky and normally wouldn't backport, except that I've heard a
few times about concerns about CVEs affecting Databind, so wondering
who else out there might have an opinion. I'm not pushing for it
necessarily.

On Wed, Apr 17, 2019 at 6:18 PM Reynold Xin <[hidden email]> wrote:
>
> For Jackson - are you worrying about JSON parsing for users or internal Spark functionality breaking?
>
> On Wed, Apr 17, 2019 at 6:02 PM Sean Owen <[hidden email]> wrote:
>>
>> There's only one other item on my radar, which is considering updating
>> Jackson to 2.9 in branch-2.4 to get security fixes. Pros: it's come up
>> a few times now that there are a number of CVEs open for 2.6.7. Cons:
>> not clear they affect Spark, and Jackson 2.6->2.9 does change Jackson
>> behavior non-trivially. That said back-porting the update PR to 2.4
>> worked out OK locally. Any strong opinions on this one?
>>
>> On Wed, Apr 17, 2019 at 7:49 PM Wenchen Fan <[hidden email]> wrote:
>> >
>> > I volunteer to be the release manager for 2.4.2, as I was also going to propose 2.4.2 because of the reverting of SPARK-25250. Is there any other ongoing bug fixes we want to include in 2.4.2? If no I'd like to start the release process today (CST).
>> >
>> > Thanks,
>> > Wenchen
>> >
>> > On Thu, Apr 18, 2019 at 3:44 AM Sean Owen <[hidden email]> wrote:
>> >>
>> >> I think the 'only backport bug fixes to branches' principle remains sound. But what's a bug fix? Something that changes behavior to match what is explicitly supposed to happen, or implicitly supposed to happen -- implied by what other similar things do, by reasonable user expectations, or simply how it worked previously.
>> >>
>> >> Is this a bug fix? I guess the criteria that matches is that behavior doesn't match reasonable user expectations? I don't know enough to have a strong opinion. I also don't think there is currently an objection to backporting it, whatever it's called.
>> >>
>> >>
>> >> Is the question whether this needs a new release? There's no harm in another point release, other than needing a volunteer release manager. One could say, wait a bit longer to see what more info comes in about 2.4.1. But given that 2.4.1 took like 2 months, it's reasonable to move towards a release cycle again. I don't see objection to that either (?)
>> >>
>> >>
>> >> The meta question remains: is a 'bug fix' definition even agreed, and being consistently applied? There aren't correct answers, only best guesses from each person's own experience, judgment and priorities. These can differ even when applied in good faith.
>> >>
>> >> Sometimes the variance of opinion comes because people have different info that needs to be surfaced. Here, maybe it's best to share what about that offline conversation was convincing, for example.
>> >>
>> >> I'd say it's also important to separate what one would prefer from what one can't live with(out). Assuming one trusts the intent and experience of the handful of others with an opinion, I'd defer to someone who wants X and will own it, even if I'm moderately against it. Otherwise we'd get little done.
>> >>
>> >> In that light, it seems like both of the PRs at issue here are not _wrong_ to backport. This is a good pair that highlights why, when there isn't a clear reason to do / not do something (e.g. obvious errors, breaking public APIs) we give benefit-of-the-doubt in order to get it later.
>> >>
>> >>
>> >> On Wed, Apr 17, 2019 at 12:09 PM Ryan Blue <[hidden email]> wrote:
>> >>>
>> >>> Sorry, I should be more clear about what I'm trying to say here.
>> >>>
>> >>> In the past, Xiao has taken the opposite stance. A good example is PR #21060 that was a very similar situation: behavior didn't match what was expected and there was low risk. There was a long argument and the patch didn't make it into 2.3 (to my knowledge).
>> >>>
>> >>> What we call these low-risk behavior fixes doesn't matter. I called it a bug on #21060 but I'm applying Xiao's previous definition here to make a point. Whatever term we use, we clearly have times when we want to allow a patch because it is low risk and helps someone. Let's just be clear that that's perfectly fine.
>> >>>
>> >>> On Wed, Apr 17, 2019 at 9:34 AM Ryan Blue <[hidden email]> wrote:
>> >>>>
>> >>>> How is this a bug fix?
>> >>>>
>> >>>> On Wed, Apr 17, 2019 at 9:30 AM Xiao Li <[hidden email]> wrote:
>> >>>>>
>> >>>>> Michael and I had an offline discussion about this PR https://github.com/apache/spark/pull/24365. He convinced me that this is a bug fix. The code changes of this bug fix are very tiny and the risk is very low. To avoid any behavior change in the patch releases, this PR also added a legacy flag whose default value is off.
>> >>>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe e-mail: [hidden email]
>>
Reply | Threaded
Open this post in threaded view
|

Re: Spark 2.4.2

Michael Heuer
+100


On Apr 18, 2019, at 1:48 AM, Reynold Xin <[hidden email]> wrote:

We should have shaded all Spark’s dependencies :(

On Wed, Apr 17, 2019 at 11:47 PM Sean Owen <[hidden email]> wrote:
For users that would inherit Jackson and use it directly, or whose
dependencies do. Spark itself (with modifications) should be OK with
the change.
It's risky and normally wouldn't backport, except that I've heard a
few times about concerns about CVEs affecting Databind, so wondering
who else out there might have an opinion. I'm not pushing for it
necessarily.

On Wed, Apr 17, 2019 at 6:18 PM Reynold Xin <[hidden email]> wrote:
>
> For Jackson - are you worrying about JSON parsing for users or internal Spark functionality breaking?
>
> On Wed, Apr 17, 2019 at 6:02 PM Sean Owen <[hidden email]> wrote:
>>
>> There's only one other item on my radar, which is considering updating
>> Jackson to 2.9 in branch-2.4 to get security fixes. Pros: it's come up
>> a few times now that there are a number of CVEs open for 2.6.7. Cons:
>> not clear they affect Spark, and Jackson 2.6->2.9 does change Jackson
>> behavior non-trivially. That said back-porting the update PR to 2.4
>> worked out OK locally. Any strong opinions on this one?
>>
>> On Wed, Apr 17, 2019 at 7:49 PM Wenchen Fan <[hidden email]> wrote:
>> >
>> > I volunteer to be the release manager for 2.4.2, as I was also going to propose 2.4.2 because of the reverting of SPARK-25250. Is there any other ongoing bug fixes we want to include in 2.4.2? If no I'd like to start the release process today (CST).
>> >
>> > Thanks,
>> > Wenchen
>> >
>> > On Thu, Apr 18, 2019 at 3:44 AM Sean Owen <[hidden email]> wrote:
>> >>
>> >> I think the 'only backport bug fixes to branches' principle remains sound. But what's a bug fix? Something that changes behavior to match what is explicitly supposed to happen, or implicitly supposed to happen -- implied by what other similar things do, by reasonable user expectations, or simply how it worked previously.
>> >>
>> >> Is this a bug fix? I guess the criteria that matches is that behavior doesn't match reasonable user expectations? I don't know enough to have a strong opinion. I also don't think there is currently an objection to backporting it, whatever it's called.
>> >>
>> >>
>> >> Is the question whether this needs a new release? There's no harm in another point release, other than needing a volunteer release manager. One could say, wait a bit longer to see what more info comes in about 2.4.1. But given that 2.4.1 took like 2 months, it's reasonable to move towards a release cycle again. I don't see objection to that either (?)
>> >>
>> >>
>> >> The meta question remains: is a 'bug fix' definition even agreed, and being consistently applied? There aren't correct answers, only best guesses from each person's own experience, judgment and priorities. These can differ even when applied in good faith.
>> >>
>> >> Sometimes the variance of opinion comes because people have different info that needs to be surfaced. Here, maybe it's best to share what about that offline conversation was convincing, for example.
>> >>
>> >> I'd say it's also important to separate what one would prefer from what one can't live with(out). Assuming one trusts the intent and experience of the handful of others with an opinion, I'd defer to someone who wants X and will own it, even if I'm moderately against it. Otherwise we'd get little done.
>> >>
>> >> In that light, it seems like both of the PRs at issue here are not _wrong_ to backport. This is a good pair that highlights why, when there isn't a clear reason to do / not do something (e.g. obvious errors, breaking public APIs) we give benefit-of-the-doubt in order to get it later.
>> >>
>> >>
>> >> On Wed, Apr 17, 2019 at 12:09 PM Ryan Blue <[hidden email]> wrote:
>> >>>
>> >>> Sorry, I should be more clear about what I'm trying to say here.
>> >>>
>> >>> In the past, Xiao has taken the opposite stance. A good example is PR #21060 that was a very similar situation: behavior didn't match what was expected and there was low risk. There was a long argument and the patch didn't make it into 2.3 (to my knowledge).
>> >>>
>> >>> What we call these low-risk behavior fixes doesn't matter. I called it a bug on #21060 but I'm applying Xiao's previous definition here to make a point. Whatever term we use, we clearly have times when we want to allow a patch because it is low risk and helps someone. Let's just be clear that that's perfectly fine.
>> >>>
>> >>> On Wed, Apr 17, 2019 at 9:34 AM Ryan Blue <[hidden email]> wrote:
>> >>>>
>> >>>> How is this a bug fix?
>> >>>>
>> >>>> On Wed, Apr 17, 2019 at 9:30 AM Xiao Li <[hidden email]> wrote:
>> >>>>>
>> >>>>> Michael and I had an offline discussion about this PR https://github.com/apache/spark/pull/24365. He convinced me that this is a bug fix. The code changes of this bug fix are very tiny and the risk is very low. To avoid any behavior change in the patch releases, this PR also added a legacy flag whose default value is off.
>> >>>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe e-mail: [hidden email]
>>

Reply | Threaded
Open this post in threaded view
|

Re: Spark 2.4.2

Felix Cheung
Re shading - same argument I’ve made earlier today in a PR...

(Context- in many cases Spark has light or indirect dependencies but bringing them into the process breaks users code easily)



From: Michael Heuer <[hidden email]>
Sent: Thursday, April 18, 2019 6:41 AM
To: Reynold Xin
Cc: Sean Owen; Michael Armbrust; Ryan Blue; Spark Dev List; Wenchen Fan; Xiao Li
Subject: Re: Spark 2.4.2
 
+100


On Apr 18, 2019, at 1:48 AM, Reynold Xin <[hidden email]> wrote:

We should have shaded all Spark’s dependencies :(

On Wed, Apr 17, 2019 at 11:47 PM Sean Owen <[hidden email]> wrote:
For users that would inherit Jackson and use it directly, or whose
dependencies do. Spark itself (with modifications) should be OK with
the change.
It's risky and normally wouldn't backport, except that I've heard a
few times about concerns about CVEs affecting Databind, so wondering
who else out there might have an opinion. I'm not pushing for it
necessarily.

On Wed, Apr 17, 2019 at 6:18 PM Reynold Xin <[hidden email]> wrote:
>
> For Jackson - are you worrying about JSON parsing for users or internal Spark functionality breaking?
>
> On Wed, Apr 17, 2019 at 6:02 PM Sean Owen <[hidden email]> wrote:
>>
>> There's only one other item on my radar, which is considering updating
>> Jackson to 2.9 in branch-2.4 to get security fixes. Pros: it's come up
>> a few times now that there are a number of CVEs open for 2.6.7. Cons:
>> not clear they affect Spark, and Jackson 2.6->2.9 does change Jackson
>> behavior non-trivially. That said back-porting the update PR to 2.4
>> worked out OK locally. Any strong opinions on this one?
>>
>> On Wed, Apr 17, 2019 at 7:49 PM Wenchen Fan <[hidden email]> wrote:
>> >
>> > I volunteer to be the release manager for 2.4.2, as I was also going to propose 2.4.2 because of the reverting of SPARK-25250. Is there any other ongoing bug fixes we want to include in 2.4.2? If no I'd like to start the release process today (CST).
>> >
>> > Thanks,
>> > Wenchen
>> >
>> > On Thu, Apr 18, 2019 at 3:44 AM Sean Owen <[hidden email]> wrote:
>> >>
>> >> I think the 'only backport bug fixes to branches' principle remains sound. But what's a bug fix? Something that changes behavior to match what is explicitly supposed to happen, or implicitly supposed to happen -- implied by what other similar things do, by reasonable user expectations, or simply how it worked previously.
>> >>
>> >> Is this a bug fix? I guess the criteria that matches is that behavior doesn't match reasonable user expectations? I don't know enough to have a strong opinion. I also don't think there is currently an objection to backporting it, whatever it's called.
>> >>
>> >>
>> >> Is the question whether this needs a new release? There's no harm in another point release, other than needing a volunteer release manager. One could say, wait a bit longer to see what more info comes in about 2.4.1. But given that 2.4.1 took like 2 months, it's reasonable to move towards a release cycle again. I don't see objection to that either (?)
>> >>
>> >>
>> >> The meta question remains: is a 'bug fix' definition even agreed, and being consistently applied? There aren't correct answers, only best guesses from each person's own experience, judgment and priorities. These can differ even when applied in good faith.
>> >>
>> >> Sometimes the variance of opinion comes because people have different info that needs to be surfaced. Here, maybe it's best to share what about that offline conversation was convincing, for example.
>> >>
>> >> I'd say it's also important to separate what one would prefer from what one can't live with(out). Assuming one trusts the intent and experience of the handful of others with an opinion, I'd defer to someone who wants X and will own it, even if I'm moderately against it. Otherwise we'd get little done.
>> >>
>> >> In that light, it seems like both of the PRs at issue here are not _wrong_ to backport. This is a good pair that highlights why, when there isn't a clear reason to do / not do something (e.g. obvious errors, breaking public APIs) we give benefit-of-the-doubt in order to get it later.
>> >>
>> >>
>> >> On Wed, Apr 17, 2019 at 12:09 PM Ryan Blue <[hidden email]> wrote:
>> >>>
>> >>> Sorry, I should be more clear about what I'm trying to say here.
>> >>>
>> >>> In the past, Xiao has taken the opposite stance. A good example is PR #21060 that was a very similar situation: behavior didn't match what was expected and there was low risk. There was a long argument and the patch didn't make it into 2.3 (to my knowledge).
>> >>>
>> >>> What we call these low-risk behavior fixes doesn't matter. I called it a bug on #21060 but I'm applying Xiao's previous definition here to make a point. Whatever term we use, we clearly have times when we want to allow a patch because it is low risk and helps someone. Let's just be clear that that's perfectly fine.
>> >>>
>> >>> On Wed, Apr 17, 2019 at 9:34 AM Ryan Blue <[hidden email]> wrote:
>> >>>>
>> >>>> How is this a bug fix?
>> >>>>
>> >>>> On Wed, Apr 17, 2019 at 9:30 AM Xiao Li <[hidden email]> wrote:
>> >>>>>
>> >>>>> Michael and I had an offline discussion about this PR https://github.com/apache/spark/pull/24365. He convinced me that this is a bug fix. The code changes of this bug fix are very tiny and the risk is very low. To avoid any behavior change in the patch releases, this PR also added a legacy flag whose default value is off.
>> >>>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe e-mail: [hidden email]
>>

Reply | Threaded
Open this post in threaded view
|

Re: Spark 2.4.2

cloud0fan
I've cut RC1. If people think we must upgrade Jackson in 2.4, I can cut RC2 shortly.

Thanks,
Wenchen

On Fri, Apr 19, 2019 at 3:32 AM Felix Cheung <[hidden email]> wrote:
Re shading - same argument I’ve made earlier today in a PR...

(Context- in many cases Spark has light or indirect dependencies but bringing them into the process breaks users code easily)



From: Michael Heuer <[hidden email]>
Sent: Thursday, April 18, 2019 6:41 AM
To: Reynold Xin
Cc: Sean Owen; Michael Armbrust; Ryan Blue; Spark Dev List; Wenchen Fan; Xiao Li
Subject: Re: Spark 2.4.2
 
+100


On Apr 18, 2019, at 1:48 AM, Reynold Xin <[hidden email]> wrote:

We should have shaded all Spark’s dependencies :(

On Wed, Apr 17, 2019 at 11:47 PM Sean Owen <[hidden email]> wrote:
For users that would inherit Jackson and use it directly, or whose
dependencies do. Spark itself (with modifications) should be OK with
the change.
It's risky and normally wouldn't backport, except that I've heard a
few times about concerns about CVEs affecting Databind, so wondering
who else out there might have an opinion. I'm not pushing for it
necessarily.

On Wed, Apr 17, 2019 at 6:18 PM Reynold Xin <[hidden email]> wrote:
>
> For Jackson - are you worrying about JSON parsing for users or internal Spark functionality breaking?
>
> On Wed, Apr 17, 2019 at 6:02 PM Sean Owen <[hidden email]> wrote:
>>
>> There's only one other item on my radar, which is considering updating
>> Jackson to 2.9 in branch-2.4 to get security fixes. Pros: it's come up
>> a few times now that there are a number of CVEs open for 2.6.7. Cons:
>> not clear they affect Spark, and Jackson 2.6->2.9 does change Jackson
>> behavior non-trivially. That said back-porting the update PR to 2.4
>> worked out OK locally. Any strong opinions on this one?
>>
>> On Wed, Apr 17, 2019 at 7:49 PM Wenchen Fan <[hidden email]> wrote:
>> >
>> > I volunteer to be the release manager for 2.4.2, as I was also going to propose 2.4.2 because of the reverting of SPARK-25250. Is there any other ongoing bug fixes we want to include in 2.4.2? If no I'd like to start the release process today (CST).
>> >
>> > Thanks,
>> > Wenchen
>> >
>> > On Thu, Apr 18, 2019 at 3:44 AM Sean Owen <[hidden email]> wrote:
>> >>
>> >> I think the 'only backport bug fixes to branches' principle remains sound. But what's a bug fix? Something that changes behavior to match what is explicitly supposed to happen, or implicitly supposed to happen -- implied by what other similar things do, by reasonable user expectations, or simply how it worked previously.
>> >>
>> >> Is this a bug fix? I guess the criteria that matches is that behavior doesn't match reasonable user expectations? I don't know enough to have a strong opinion. I also don't think there is currently an objection to backporting it, whatever it's called.
>> >>
>> >>
>> >> Is the question whether this needs a new release? There's no harm in another point release, other than needing a volunteer release manager. One could say, wait a bit longer to see what more info comes in about 2.4.1. But given that 2.4.1 took like 2 months, it's reasonable to move towards a release cycle again. I don't see objection to that either (?)
>> >>
>> >>
>> >> The meta question remains: is a 'bug fix' definition even agreed, and being consistently applied? There aren't correct answers, only best guesses from each person's own experience, judgment and priorities. These can differ even when applied in good faith.
>> >>
>> >> Sometimes the variance of opinion comes because people have different info that needs to be surfaced. Here, maybe it's best to share what about that offline conversation was convincing, for example.
>> >>
>> >> I'd say it's also important to separate what one would prefer from what one can't live with(out). Assuming one trusts the intent and experience of the handful of others with an opinion, I'd defer to someone who wants X and will own it, even if I'm moderately against it. Otherwise we'd get little done.
>> >>
>> >> In that light, it seems like both of the PRs at issue here are not _wrong_ to backport. This is a good pair that highlights why, when there isn't a clear reason to do / not do something (e.g. obvious errors, breaking public APIs) we give benefit-of-the-doubt in order to get it later.
>> >>
>> >>
>> >> On Wed, Apr 17, 2019 at 12:09 PM Ryan Blue <[hidden email]> wrote:
>> >>>
>> >>> Sorry, I should be more clear about what I'm trying to say here.
>> >>>
>> >>> In the past, Xiao has taken the opposite stance. A good example is PR #21060 that was a very similar situation: behavior didn't match what was expected and there was low risk. There was a long argument and the patch didn't make it into 2.3 (to my knowledge).
>> >>>
>> >>> What we call these low-risk behavior fixes doesn't matter. I called it a bug on #21060 but I'm applying Xiao's previous definition here to make a point. Whatever term we use, we clearly have times when we want to allow a patch because it is low risk and helps someone. Let's just be clear that that's perfectly fine.
>> >>>
>> >>> On Wed, Apr 17, 2019 at 9:34 AM Ryan Blue <[hidden email]> wrote:
>> >>>>
>> >>>> How is this a bug fix?
>> >>>>
>> >>>> On Wed, Apr 17, 2019 at 9:30 AM Xiao Li <[hidden email]> wrote:
>> >>>>>
>> >>>>> Michael and I had an offline discussion about this PR https://github.com/apache/spark/pull/24365. He convinced me that this is a bug fix. The code changes of this bug fix are very tiny and the risk is very low. To avoid any behavior change in the patch releases, this PR also added a legacy flag whose default value is off.
>> >>>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe e-mail: [hidden email]
>>

12