You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by Ryan Skraba <ry...@skraba.com> on 2020/01/31 16:28:45 UTC

Including "fastavro" in release 1.9.2

Hello!  There's a long standing PR[1][2] that provides an impressive
performance boost to deserializing Avro.

The author has been patient and reactive for a LONG time (thanks Martin!),
and it's inspired some really interesting discussion.

I'd like to merge and cherry-pick into release 1.9.2 -- as long as it's
turned off there shouldn't be any visible effect at all, and having it in a
release would be a great way to find regressions (if any) and encourage
some of the ideas brought up in the PR discussions.

Any objections?  Is this too big a change for a minor release?

I noted in the PR that it would be pretty cool if we had a "better" way of
noting experimental features (and that the Yetus annotations would be a
good start for 1.10!)

All my best, Ryan

[1]: https://issues.apache.org/jira/browse/AVRO-2247
[2]: https://github.com/apache/avro/pull/391

Re: Including "fastread" in release 1.9.2

Posted by Ryan Skraba <ry...@skraba.com>.
Sorry Ismael!  I retitled this email -- yes, we should definitely be
referring this feature to fastread and not fastavro.

I merged this morning a few minutes before your comment!  The fastread flag
has been cherry-picked to master and branch-1.9.  Thanks to the contributor
Martin, and the reviewers of the two PRs!  There was a lot to think about
in the discussion and some great ideas to build on!

I will create a JIRA (https://issues.apache.org/jira/browse/AVRO-2724) to
include better documentation about the existing experimental options...

I'm going to cut a RC1 tomorrow morning with at least a minimum doc for the
above and hopefully a requested fix for AVRO-2641 /
https://github.com/apache/avro/pull/728 (Thanks Fokko!)

All my best, Ryan



On Mon, Feb 3, 2020 at 9:38 AM Ismaël Mejía <ie...@gmail.com> wrote:

> +1
>
> Having experimental features backported sounds good to enable early testing
> and
> feedback, only condition should be not breaking backwards compatibility.
> Probably worth to mention in the release notes and to create a JIRA + PR to
> enable this as the default behavior for 1.10.  We have to pay attention to
> call
> this 'fastread' as the author of the PR does instead of fastavro [1]
> otherwise
> people get confused about the alternative python implementation.
>
> [1] https://github.com/fastavro/fastavro
>
> On Sun, Feb 2, 2020 at 5:14 PM Driesprong, Fokko <fo...@driesprong.frl>
> wrote:
>
> > Sounds like a plan to me. I don't have any objections.
> >
> > In the far future, I'd like to merge these readers, otherwise, you'll end
> > up with many configuration permutations. This makes it challenging to
> test
> > each permutation. But first having it as an option in 1.9 is a good idea,
> > so we can battle test the new reader.
> >
> > Cheers, Fokko
> >
> >
> > Op za 1 feb. 2020 om 03:06 schreef Sean Busbey <bu...@apache.org>:
> >
> > > unless the RM has an objection based on where they are in the RC
> > > creation process, I say go for it. (and if there is such an issue then
> > > let's get it ready for 1.9.3)
> > >
> > > On Fri, Jan 31, 2020 at 10:29 AM Ryan Skraba <ry...@skraba.com> wrote:
> > > >
> > > > Hello!  There's a long standing PR[1][2] that provides an impressive
> > > > performance boost to deserializing Avro.
> > > >
> > > > The author has been patient and reactive for a LONG time (thanks
> > > Martin!),
> > > > and it's inspired some really interesting discussion.
> > > >
> > > > I'd like to merge and cherry-pick into release 1.9.2 -- as long as
> it's
> > > > turned off there shouldn't be any visible effect at all, and having
> it
> > > in a
> > > > release would be a great way to find regressions (if any) and
> encourage
> > > > some of the ideas brought up in the PR discussions.
> > > >
> > > > Any objections?  Is this too big a change for a minor release?
> > > >
> > > > I noted in the PR that it would be pretty cool if we had a "better"
> way
> > > of
> > > > noting experimental features (and that the Yetus annotations would
> be a
> > > > good start for 1.10!)
> > > >
> > > > All my best, Ryan
> > > >
> > > > [1]: https://issues.apache.org/jira/browse/AVRO-2247
> > > > [2]: https://github.com/apache/avro/pull/391
> > >
> >
>

Re: Including "fastavro" in release 1.9.2

Posted by Ismaël Mejía <ie...@gmail.com>.
+1

Having experimental features backported sounds good to enable early testing
and
feedback, only condition should be not breaking backwards compatibility.
Probably worth to mention in the release notes and to create a JIRA + PR to
enable this as the default behavior for 1.10.  We have to pay attention to
call
this 'fastread' as the author of the PR does instead of fastavro [1]
otherwise
people get confused about the alternative python implementation.

[1] https://github.com/fastavro/fastavro

On Sun, Feb 2, 2020 at 5:14 PM Driesprong, Fokko <fo...@driesprong.frl>
wrote:

> Sounds like a plan to me. I don't have any objections.
>
> In the far future, I'd like to merge these readers, otherwise, you'll end
> up with many configuration permutations. This makes it challenging to test
> each permutation. But first having it as an option in 1.9 is a good idea,
> so we can battle test the new reader.
>
> Cheers, Fokko
>
>
> Op za 1 feb. 2020 om 03:06 schreef Sean Busbey <bu...@apache.org>:
>
> > unless the RM has an objection based on where they are in the RC
> > creation process, I say go for it. (and if there is such an issue then
> > let's get it ready for 1.9.3)
> >
> > On Fri, Jan 31, 2020 at 10:29 AM Ryan Skraba <ry...@skraba.com> wrote:
> > >
> > > Hello!  There's a long standing PR[1][2] that provides an impressive
> > > performance boost to deserializing Avro.
> > >
> > > The author has been patient and reactive for a LONG time (thanks
> > Martin!),
> > > and it's inspired some really interesting discussion.
> > >
> > > I'd like to merge and cherry-pick into release 1.9.2 -- as long as it's
> > > turned off there shouldn't be any visible effect at all, and having it
> > in a
> > > release would be a great way to find regressions (if any) and encourage
> > > some of the ideas brought up in the PR discussions.
> > >
> > > Any objections?  Is this too big a change for a minor release?
> > >
> > > I noted in the PR that it would be pretty cool if we had a "better" way
> > of
> > > noting experimental features (and that the Yetus annotations would be a
> > > good start for 1.10!)
> > >
> > > All my best, Ryan
> > >
> > > [1]: https://issues.apache.org/jira/browse/AVRO-2247
> > > [2]: https://github.com/apache/avro/pull/391
> >
>

Re: Including "fastavro" in release 1.9.2

Posted by "Driesprong, Fokko" <fo...@driesprong.frl>.
Sounds like a plan to me. I don't have any objections.

In the far future, I'd like to merge these readers, otherwise, you'll end
up with many configuration permutations. This makes it challenging to test
each permutation. But first having it as an option in 1.9 is a good idea,
so we can battle test the new reader.

Cheers, Fokko


Op za 1 feb. 2020 om 03:06 schreef Sean Busbey <bu...@apache.org>:

> unless the RM has an objection based on where they are in the RC
> creation process, I say go for it. (and if there is such an issue then
> let's get it ready for 1.9.3)
>
> On Fri, Jan 31, 2020 at 10:29 AM Ryan Skraba <ry...@skraba.com> wrote:
> >
> > Hello!  There's a long standing PR[1][2] that provides an impressive
> > performance boost to deserializing Avro.
> >
> > The author has been patient and reactive for a LONG time (thanks
> Martin!),
> > and it's inspired some really interesting discussion.
> >
> > I'd like to merge and cherry-pick into release 1.9.2 -- as long as it's
> > turned off there shouldn't be any visible effect at all, and having it
> in a
> > release would be a great way to find regressions (if any) and encourage
> > some of the ideas brought up in the PR discussions.
> >
> > Any objections?  Is this too big a change for a minor release?
> >
> > I noted in the PR that it would be pretty cool if we had a "better" way
> of
> > noting experimental features (and that the Yetus annotations would be a
> > good start for 1.10!)
> >
> > All my best, Ryan
> >
> > [1]: https://issues.apache.org/jira/browse/AVRO-2247
> > [2]: https://github.com/apache/avro/pull/391
>

Re: Including "fastavro" in release 1.9.2

Posted by Sean Busbey <bu...@apache.org>.
unless the RM has an objection based on where they are in the RC
creation process, I say go for it. (and if there is such an issue then
let's get it ready for 1.9.3)

On Fri, Jan 31, 2020 at 10:29 AM Ryan Skraba <ry...@skraba.com> wrote:
>
> Hello!  There's a long standing PR[1][2] that provides an impressive
> performance boost to deserializing Avro.
>
> The author has been patient and reactive for a LONG time (thanks Martin!),
> and it's inspired some really interesting discussion.
>
> I'd like to merge and cherry-pick into release 1.9.2 -- as long as it's
> turned off there shouldn't be any visible effect at all, and having it in a
> release would be a great way to find regressions (if any) and encourage
> some of the ideas brought up in the PR discussions.
>
> Any objections?  Is this too big a change for a minor release?
>
> I noted in the PR that it would be pretty cool if we had a "better" way of
> noting experimental features (and that the Yetus annotations would be a
> good start for 1.10!)
>
> All my best, Ryan
>
> [1]: https://issues.apache.org/jira/browse/AVRO-2247
> [2]: https://github.com/apache/avro/pull/391

Re: Including "fastavro" in release 1.9.2

Posted by Raymie Stata <rs...@yahoo.com.INVALID>.
Yes, it's time to get this in.

On Fri, Jan 31, 2020 at 8:28 AM Ryan Skraba <ry...@skraba.com> wrote:
>
> Hello!  There's a long standing PR[1][2] that provides an impressive
> performance boost to deserializing Avro.
>
> The author has been patient and reactive for a LONG time (thanks Martin!),
> and it's inspired some really interesting discussion.
>
> I'd like to merge and cherry-pick into release 1.9.2 -- as long as it's
> turned off there shouldn't be any visible effect at all, and having it in a
> release would be a great way to find regressions (if any) and encourage
> some of the ideas brought up in the PR discussions.
>
> Any objections?  Is this too big a change for a minor release?
>
> I noted in the PR that it would be pretty cool if we had a "better" way of
> noting experimental features (and that the Yetus annotations would be a
> good start for 1.10!)
>
> All my best, Ryan
>
> [1]: https://issues.apache.org/jira/browse/AVRO-2247
> [2]: https://github.com/apache/avro/pull/391