Re: Postgres 9.6 scariest patch tournament

Hackers, lurkers,

The PostgreSQL Project needs you!

The Release Management Team would like your input regarding the patch or
patches which, in your opinion, are the most likely sources of major
bugs or instabilities in PostgreSQL 9.6.

Please submit your answers before May 1st using this form:
https://docs.google.com/forms/d/1xNNqhXC116wCMnomqGz9RQ7OuVwZqAcEre7iiU6pT20/viewform

If, for some reason, you prefer not to fill that form or have further
input on the topic, you can correspond via private email to one or more
members of the RMT,

Robert Haas <robertmhaas(at)gmail(dot)com>
Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Noah Misch <nmisch(at)leadboat(dot)com>

The RMT will publish aggregate, unattributed results after the poll
closes.

Thanks,

--
Álvaro Herrera

On Mon, 18 Apr 2016 15:37:21 -0300
Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:

> Hackers, lurkers,
>
> The PostgreSQL Project needs you!
>
> The Release Management Team would like your input regarding the patch or
> patches which, in your opinion, are the most likely sources of major
> bugs or instabilities in PostgreSQL 9.6.

Wow ... this may be the most unusual survey I've seen in a while.

--
Bill Moran

On 04/18/2016 11:37 AM, Alvaro Herrera wrote:
> Hackers, lurkers,
>
> The PostgreSQL Project needs you!
>
> The Release Management Team would like your input regarding the patch or
> patches which, in your opinion, are the most likely sources of major
> bugs or instabilities in PostgreSQL 9.6.
>
> Please submit your answers before May 1st using this form:
> https://docs.google.com/forms/d/1xNNqhXC116wCMnomqGz9RQ7OuVwZqAcEre7iiU6pT20/viewform
>
> If, for some reason, you prefer not to fill that form or have further
> input on the topic, you can correspond via private email to one or more
> members of the RMT,
>
> Robert Haas <robertmhaas(at)gmail(dot)com>
> Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
> Noah Misch <nmisch(at)leadboat(dot)com>
>
> The RMT will publish aggregate, unattributed results after the poll
> closes.

We should send the owner of the scariest patch something as a prize.
Maybe a plastic skeleton or something ...

--
--
Josh Berkus
Red Hat OSAS
(any opinions are my own)

On 04/18/2016 04:22 PM, Josh berkus wrote:
>
> We should send the owner of the scariest patch something as a prize.
> Maybe a plastic skeleton or something ...

A mouse.

-Chap

On Mon, Apr 18, 2016 at 1:22 PM, Josh berkus <josh(at)agliodbs(dot)com> wrote:
> We should send the owner of the scariest patch something as a prize.
> Maybe a plastic skeleton or something ...

I think it was a good idea to call it the scariest patch rather than
something more severe sounding. Having the poll only be half-serious
is a good way to avoid self-censorship, and emphasizes that we're
concerned about bugs that cause serious instability to the system as a
whole. We're less concerned about the overall number of bugs in any
given patch.

I would have appreciated more scope to say how confident I am in my
prediction, and how scary in absolute terms I consider the scariest
patches to be.

--
Peter Geoghegan

Peter Geoghegan wrote:
> On Mon, Apr 18, 2016 at 1:22 PM, Josh berkus <josh(at)agliodbs(dot)com> wrote:
> > We should send the owner of the scariest patch something as a prize.
> > Maybe a plastic skeleton or something ...
>
> I think it was a good idea to call it the scariest patch rather than
> something more severe sounding. Having the poll only be half-serious
> is a good way to avoid self-censorship, and emphasizes that we're
> concerned about bugs that cause serious instability to the system as a
> whole. We're less concerned about the overall number of bugs in any
> given patch.

This guy reads my mind. Where's my tinfoil hat?

> I would have appreciated more scope to say how confident I am in my
> prediction, and how scary in absolute terms I consider the scariest
> patches to be.

It was purposefully ambiguous. Maybe it should have been stated
explicitely.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
> Peter Geoghegan wrote:
>> I would have appreciated more scope to say how confident I am in my
>> prediction, and how scary in absolute terms I consider the scariest
>> patches to be.

> It was purposefully ambiguous. Maybe it should have been stated
> explicitely.

I was thinking about complaining that "scariest" and "most bugs" are
not the same thing. Features you can turn off are not very scary,
even if they're full of bugs (cough ... parallel query ... cough),
because we could just ship 'em disabled by default until there's more
reason to trust them. What I find scary is patches that can break
existing use-cases with no simple workaround. I'm not sure which one
to vote for yet.

regards, tom lane

On Tue, Apr 19, 2016 at 04:06:55PM -0400, Tom Lane wrote:
> Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
> > Peter Geoghegan wrote:
> >> I would have appreciated more scope to say how confident I am in
> >> my prediction, and how scary in absolute terms I consider the
> >> scariest patches to be.
>
> > It was purposefully ambiguous. Maybe it should have been stated
> > explicitely.
>
> I was thinking about complaining that "scariest" and "most bugs" are
> not the same thing. Features you can turn off are not very scary,
> even if they're full of bugs (cough ... parallel query ... cough),
> because we could just ship 'em disabled by default until there's
> more reason to trust them. What I find scary is patches that can
> break existing use-cases with no simple workaround. I'm not sure
> which one to vote for yet.

There's space on the ballot for up to three, and it appears to be a
ranked choice or similar preference system.

Cheers,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

On Tue, Apr 19, 2016 at 12:37 PM, Alvaro Herrera
<alvherre(at)2ndquadrant(dot)com> wrote:
> This guy reads my mind. Where's my tinfoil hat?

Heh. Well, I'm generally not in favor of communicating concerns
without an obligation to defend them, but it could work well in tiny
doses. Offering hackers a low-risk way to take a position greatly
reduces the "knew-it-all-along" effect. We may then be more accurate
in assessing our own ability to anticipate problems.

There is very simple Malthusian logic [1] that explains why we'll
usually be wrong, which is:

Why are hackers bad at anticipating where bugs will be? Because if
they weren't, then there wouldn't be any bugs.

Please don't interpret my remarks as showing flippancy about bugs.
(The same should be said about the whole "scary patches" poll,
actually.)

>> I would have appreciated more scope to say how confident I am in my
>> prediction, and how scary in absolute terms I consider the scariest
>> patches to be.
>
> It was purposefully ambiguous. Maybe it should have been stated
> explicitely.

I voted, and my vote probably just slightly reinforced the
conventional wisdom about where to look for problems -- it was not a
vote for parallel query, since I agree with Tom's assessment of the
risks there. I think you can probably guess what I voted for.

I wouldn't have expressed a similar sentiment on this list, because
that would probably turn out to be just jumping on the bandwagon.
There is a good chance that the patch will be totally fine in the end,
anyway. It was probably very carefully reviewed, precisely because it
touches critical parts of the system. And, it works in a way that
generalizes from an existing well-tested mechanism.

My vote represented "I certainly hope this patch has no bugs in it"
this time around. Next time, it might be "this patch almost certainly
has lots of undiscovered bugs", which might well be an original
insight for the release team if it's in my area of expertise (chances
are good that those bugs are not critically important if it gets to
that). Rarely, the message will be "I'm deeply concerned about the
*lasting* repercussions of having merged this patch". And so, yes, I
think that we might want to be clearer about looking for nuances like
that.

[1] http://www.scottaaronson.com/blog/?p=418
--
Peter Geoghegan

On Mon, Apr 18, 2016 at 03:37:21PM -0300, Alvaro Herrera wrote:
> The PostgreSQL Project needs you!
>
> The Release Management Team would like your input regarding the patch or
> patches which, in your opinion, are the most likely sources of major
> bugs or instabilities in PostgreSQL 9.6.
>
> Please submit your answers before May 1st using this form:
> https://docs.google.com/forms/d/1xNNqhXC116wCMnomqGz9RQ7OuVwZqAcEre7iiU6pT20/viewform

Reminder: the survey closes this weekend. Identify those scary patches!

On Mon, Apr 18, 2016 at 03:37:21PM -0300, Alvaro Herrera wrote:
> The RMT will publish aggregate, unattributed results after the poll
> closes.

Thanks for voting. Join me in congratulating our top finishers:

1. fd31cd2 Dont vacuum all-frozen pages.
2. "Parallel Query"
3(tie). 3fc6e2d Make the upper part of the planner work by generating and comparing Paths.
3(tie). 848ef42 Add the "snapshot too old" feature

On Wed, May 4, 2016 at 9:41 PM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> On Mon, Apr 18, 2016 at 03:37:21PM -0300, Alvaro Herrera wrote:
>> The RMT will publish aggregate, unattributed results after the poll
>> closes.
>
> Thanks for voting. Join me in congratulating our top finishers:
>
> 1. fd31cd2 Dont vacuum all-frozen pages.
> 2. "Parallel Query"
> 3(tie). 3fc6e2d Make the upper part of the planner work by generating and comparing Paths.
> 3(tie). 848ef42 Add the "snapshot too old" feature

Congratulations Kevin, Tom, me, and me!

I feel like I went to the Olympics and won both the gold *and* silver
medals in the same event. Beat that!

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 05/04/2016 06:56 PM, Robert Haas wrote:
> On Wed, May 4, 2016 at 9:41 PM, Noah Misch <noah(at)leadboat(dot)com> wrote:
>> On Mon, Apr 18, 2016 at 03:37:21PM -0300, Alvaro Herrera wrote:
>>> The RMT will publish aggregate, unattributed results after the poll
>>> closes.
>>
>> Thanks for voting. Join me in congratulating our top finishers:
>>
>> 1. fd31cd2 Dont vacuum all-frozen pages.
>> 2. "Parallel Query"
>> 3(tie). 3fc6e2d Make the upper part of the planner work by generating and comparing Paths.
>> 3(tie). 848ef42 Add the "snapshot too old" feature
>
> Congratulations Kevin, Tom, me, and me!
>
> I feel like I went to the Olympics and won both the gold *and* silver
> medals in the same event. Beat that!
>

Maybe we *should* call this 10.0. That way people will be ready for
lots of breakage. ;-b

--
--
Josh Berkus
Red Hat OSAS
(any opinions are my own)

Noah Misch wrote:
> On Mon, Apr 18, 2016 at 03:37:21PM -0300, Alvaro Herrera wrote:
> > The RMT will publish aggregate, unattributed results after the poll
> > closes.

Here are some more detailed results. We got 15 valid replies. One
person voted twice, mentioning the same patches both times in slightly
different order; I considered the second reply only.

"Parallel Query" got many mentions; some of them were specific commits
(such as "parallel infrastructure", "parallel joins", "parallel
aggregates") and others were more generic. For the generic mentions I
just chose a few of the most salient patches, but didn't include either
parallel aggregates nor parallel joins in that list. "LWLock tranches"
and "Freeze Map" also resulted in several commits appearing in the list
below. This distorts the results somewhat. I considered redoing the
results once I noticed the problem, but didn't really want to invest
*too* much time.

I chose to ignore the scariness rating in this tabular report.

┌───────┬──────────────────────────────────────────┬─────────────────────────┬─────────────────────────────────────────────────────────────────────────────┐
│ count │ sha1 │ committer │ subject │
├───────┼──────────────────────────────────────────┼─────────────────────────┼─────────────────────────────────────────────────────────────────────────────┤
│ 8 │ fd31cd265138019dcccc9b5fe53043670898bc9f │ rhaas(at)postgresql(dot)org │ Don't vacuum all-frozen pages. │
│ 6 │ 924bcf4f16d54c55310b28f77686608684734f42 │ rhaas(at)postgresql(dot)org │ Create an infrastructure for parallel computation in PostgreSQL. │
│ 5 │ f0661c4e8c44c0ec7acd4ea7c82e85b265447398 │ rhaas(at)postgresql(dot)org │ Make sequential scans parallel-aware. │
│ 5 │ ee7ca559fcf404f9a3bd99da85c8f4ea9fbc2e92 │ rhaas(at)postgresql(dot)org │ Add a C API for parallel heap scans. │
│ 3 │ a892234f830e832110f63fc0a2afce2fb21d1584 │ rhaas(at)postgresql(dot)org │ Change the format of the VM fork to add a second bit per page. │
│ 3 │ 3fc6e2d7f5b652b417fa6937c34de2438d60fa9f │ tgl(at)sss(dot)pgh(dot)pa(dot)us │ Make the upper part of the planner work by generating and comparing Paths. │
│ 3 │ 848ef42bb8c7909c9d7baa38178d4a209906e7c1 │ kgrittn(at)postgresql(dot)org │ Add the "snapshot too old" feature │
│ 2 │ bb140506df605fab58f48926ee1db1f80bdafb59 │ teodor(at)sigaev(dot)ru │ Phrase full text search. │
│ 2 │ 9cd00c457e6a1ebb984167ac556a9961812a683c │ andres(at)anarazel(dot)de │ Checkpoint sorting and balancing. │
│ 1 │ 3bd909b220930f21d6e15833a17947be749e7fde │ rhaas(at)postgresql(dot)org │ Add a Gather executor node. │
│ 1 │ c319991bcad02a2e99ddac3f42762b0f6fa8d52a │ rhaas(at)postgresql(dot)org │ Use separate lwlock tranches for buffer, lock, and predicate lock managers. │
│ 1 │ 1c1a7cbd6a1600c97dfcd9b5dc78a23b5db9bbf6 │ tgl(at)sss(dot)pgh(dot)pa(dot)us │ Sync our copy of the timezone library with IANA release tzcode2016c. │
│ 1 │ 65578341af1ae50e52e0f45e691ce88ad5a1b9b1 │ teodor(at)sigaev(dot)ru │ Add Generic WAL interface │
│ 1 │ c09b18f21c52cbcf8718d6c267c84fcfea3739a9 │ alvherre(at)alvh(dot)no-ip(dot)org │ Support crosstabview in psql │
│ 1 │ e4106b2528727c4b48639c0e12bf2f70a766b910 │ rhaas(at)postgresql(dot)org │ postgres_fdw: Push down joins to remote servers. │
│ 1 │ 428b1d6b29ca599c5700d4bc4f4ce4c5880369bf │ andres(at)anarazel(dot)de │ Allow to trigger kernel writeback after a configurable number of writes. │
│ 1 │ 6150a1b08a9fe7ead2b25240be46dddeae9d98e1 │ rhaas(at)postgresql(dot)org │ Move buffer I/O and content LWLocks out of the main tranche. │
│ 1 │ 48354581a49c30f5757c203415aa8412d85b0f70 │ andres(at)anarazel(dot)de │ Allow Pin/UnpinBuffer to operate in a lockfree manner. │
│ 1 │ 7a542700df25eaf97b794bff63606176433dcdda │ sfrost(at)snowman(dot)net │ Create default roles │
│ 1 │ 013ebc0a7b7ea9c1b1ab7a3d4dd75ea121ea8ba7 │ teodor(at)sigaev(dot)ru │ Microvacuum for GIST │
│ 1 │ 53be0b1add7064ca5db3cd884302dfc3268d884e │ rhaas(at)postgresql(dot)org │ Provide much better wait information in pg_stat_activity. │
│ 1 │ 0711803775a37e0bf39d7efdd1e34d9d7e640ea1 │ rhaas(at)postgresql(dot)org │ Use quicksort, not replacement selection, for external sorting. │
│ 1 │ b319356f0e94a6482c726cf4af96597c211d8d6e │ rhaas(at)postgresql(dot)org │ Migrate PGPROC's backendLock into PGPROC itself, using a new tranche. │
│ 1 │ 11c8669c0cc7abc7a7702594cf13452c378d2517 │ rhaas(at)postgresql(dot)org │ Add parallel query support functions for assorted aggregates. │
│ 1 │ 2251179e6ad3a865d2f55e1832fab34608fcce43 │ rhaas(at)postgresql(dot)org │ Migrate replication slot I/O locks into a separate tranche. │
│ 1 │ 45be99f8cd5d606086e0a458c9c72910ba8a613d │ rhaas(at)postgresql(dot)org │ Support parallel joins, and make related improvements. │
│ 1 │ 7117685461af50f50c03f43e6a622284c8d54694 │ magnus(at)hagander(dot)net │ Implement backup API functions for non-exclusive backups │
│ 1 │ 98a64d0bd713cb89e61bef6432befc4b7b5da59e │ andres(at)anarazel(dot)de │ Introduce WaitEventSet API. │
└───────┴──────────────────────────────────────────┴─────────────────────────┴─────────────────────────────────────────────────────────────────────────────┘

It's clear that Robert is winner, even if you discount the distortion
caused by counting one entry as multiple patches.

--
Álvaro Herrera

Alvaro Herrera wrote:

> "Parallel Query" got many mentions; some of them were specific commits
> (such as "parallel infrastructure", "parallel joins", "parallel
> aggregates") and others were more generic. For the generic mentions I
> just chose a few of the most salient patches, but didn't include either
> parallel aggregates nor parallel joins in that list. "LWLock tranches"
> and "Freeze Map" also resulted in several commits appearing in the list
> below. This distorts the results somewhat. I considered redoing the
> results once I noticed the problem, but didn't really want to invest
> *too* much time.

After looking at commits mentioning "parallel", I'm surprised that this
one didn't turn up specifically as a scary commit:

│ a1c1af2a1f6099c039f145c1edb52257f315be51 │ rhaas(at)postgresql(dot)org │ Introduce group locking to prevent parallel processes from deadlocking. │

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

This is raw, in case anyone wants to look more closely.

alvherre=# select level, count(*), patch, subject from scary left join commits on patch = sha1 group by level, patch, subject order by level asc, count(*) desc;
┌───────┬───────┬───────────────────────────────────────────────────────────────┬────────────────────────────────────────────────────────────────────────────┐
│ level │ count │ patch │ subject │
├───────┼───────┼───────────────────────────────────────────────────────────────┼────────────────────────────────────────────────────────────────────────────┤
│ 1 │ 3 │ fd31cd265138019dcccc9b5fe53043670898bc9f │ Don't vacuum all-frozen pages. │
│ 1 │ 2 │ 3fc6e2d7f5b652b417fa6937c34de2438d60fa9f │ Make the upper part of the planner work by generating and comparing Paths. │
│ 1 │ 1 │ Change the format of the VM fork to add a second bit per page │ │
│ 1 │ 1 │ Don't VACUUM all-frozen pages │ │
│ 1 │ 1 │ 65578341af1ae50e52e0f45e691ce88ad5a1b9b1 │ Add Generic WAL interface │
│ 1 │ 1 │ 9cd00c457e6a1ebb984167ac556a9961812a683c │ Checkpoint sorting and balancing. │
│ 1 │ 1 │ 924bcf4f16d54c55310b28f77686608684734f42 │ Create an infrastructure for parallel computation in PostgreSQL. │
│ 1 │ 1 │ Freeze Map │ │
│ 1 │ 1 │ parallelism, gather node... │ │
│ 1 │ 1 │ Parallel query │ │
│ 1 │ 1 │ Snapshot too old │ │
│ 1 │ 1 │ Snapshot Too Old │ │
│ 2 │ 2 │ bb140506df605fab58f48926ee1db1f80bdafb59 │ Phrase full text search. │
│ 2 │ 1 │ \crosstabview │ │
│ 2 │ 1 │ 1c1a7cbd6a1600c97dfcd9b5dc78a23b5db9bbf6 │ Sync our copy of the timezone library with IANA release tzcode2016c. │
│ 2 │ 1 │ 3fc6e2d7f5b652b417fa6937c34de2438d60fa9f │ Make the upper part of the planner work by generating and comparing Paths. │
│ 2 │ 1 │ 428b1d6b29ca599c5700d4bc4f4ce4c5880369bf │ Allow to trigger kernel writeback after a configurable number of writes. │
│ 2 │ 1 │ 48354581a49c30f5757c203415aa8412d85b0f70 │ Allow Pin/UnpinBuffer to operate in a lockfree manner. │
│ 2 │ 1 │ 848ef42bb8c7909c9d7baa38178d4a209906e7c1 │ Add the "snapshot too old" feature │
│ 2 │ 1 │ fd31cd265138019dcccc9b5fe53043670898bc9f │ Don't vacuum all-frozen pages. │
│ 2 │ 1 │ Freeze map │ │
│ 2 │ 1 │ LWLock tranches │ │
│ 2 │ 1 │ Parallelism Stuff (many patches) │ │
│ 2 │ 1 │ Parallel Query │ │
│ 3 │ 1 │ 013ebc0a7b7ea9c1b1ab7a3d4dd75ea121ea8ba7 │ Microvacuum for GIST │
│ 3 │ 1 │ 0711803775a37e0bf39d7efdd1e34d9d7e640ea1 │ Use quicksort, not replacement selection, for external sorting. │
│ 3 │ 1 │ 45be99f8cd5d606086e0a458c9c72910ba8a613d │ Support parallel joins, and make related improvements. │
│ 3 │ 1 │ 53be0b1add7064ca5db3cd884302dfc3268d884e │ Provide much better wait information in pg_stat_activity. │
│ 3 │ 1 │ 98a64d0bd713cb89e61bef6432befc4b7b5da59e │ Introduce WaitEventSet API. │
│ 3 │ 1 │ Checkpoint sorting & flushin │ │
│ 3 │ 1 │ Default Roles │ │
│ 3 │ 1 │ fd31cd265138019dcccc9b5fe53043670898bc9f │ Don't vacuum all-frozen pages. │
│ 3 │ 1 │ Parallel aggregate │ │
│ 3 │ 1 │ parallel query │ │
│ 3 │ 1 │ postgres_fdw join push down │ │
│ 3 │ 1 │ Updated backup APIs │ │
└───────┴───────┴───────────────────────────────────────────────────────────────┴────────────────────────────────────────────────────────────────────────────┘
(36 filas)

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services