Re: PostgreSQL 17 Release Management Team & Feature Freeze

On Mon, 8 Apr 2024 at 20:15, Tomas Vondra <tomas.vondra@enterprisedb.com> wrote:
>
>
> On 4/8/24 17:48, Matthias van de Meent wrote:
>> On Mon, 8 Apr 2024 at 17:21, Tomas Vondra <tomas.vondra@enterprisedb.com> wrote:
>>>
>>> Maybe it'd be better to start by expanding the existing rule about not
>>> committing patches introduced for the first time in the last CF.
>>
>> I don't think adding more hurdles about what to include into the next
>> release is a good solution. Why the March CF, and not earlier? Or
>> later? How about unregistered patches? Changes to the docs? Bug fixes?
>> The March CF already has a submission deadline of "before march", so
>> that already puts a soft limit on the patches considered for the april
>> deadline.
>>
>>> What if
>>> we said that patches in the last CF must not go through significant
>>> changes, and if they do it'd mean the patch is moved to the next CF?
>>
>> I also think there is already a big issue with a lack of interest in
>> getting existing patches from non-committers committed, reducing the
>> set of patches that could be considered is just cheating the numbers
>> and discouraging people from contributing. For one, I know I have
>> motivation issues keeping up with reviewing other people's patches
>> when none (well, few, as of this CF) of my patches get reviewed
>> materially and committed. I don't see how shrinking the window of
>> opportunity for significant review from 9 to 7 months is going to help
>> there.
>>
>
> I 100% understand how frustrating the lack of progress can be, and I
> agree we need to do better. I tried to move a number of stuck patches
> this CF, and I hope (and plan) to do more of this in the future.

Thanks in advance.

> But I don't quite see how would this rule modification change the
> situation for non-committers. AFAIK we already have the rule that
> (complex) patches should not appear in the last CF for the first time,
> and I'd argue that a substantial rework of a complex patch is not that
> far from a completely new patch. Sure, the reworks may be based on a
> thorough review, so there's a lot of nuance. But still, is it possible
> to properly review if it gets reworked at the very end of the CF?

Possible? Probably, if there is a good shared understanding of why the
previous patch version's approach didn't work well, and if the next
approach is well understood as well. But it's not likely, that I'll
agree with.

But the main issue I have with your suggestion is that the March
commitfest effectively contains all new patches starting from January,
and the leftovers not committed by February. If we start banning all
new patches and those with significant reworks from the March
commitfest, then combined with the lack of CF in May there wouldn't be
any chance for new patches in the first half of the year, and an
effective block on rewrites for 6 months- the next CF is only in July.
Sure, there is a bit of leeway there, as some patches get committed
before the commitfest they're registered in is marked active, but our
development workflow is already quite hostile to incidental
contributor's patches [^0], and increasing the periods in which
authors shouldn't expect their patches to be reviewed due to a major
release that's planned in >5 months is probably not going to help the
case.

> > So, I think that'd be counter-productive, as this would get the
> > perverse incentive to band-aid over (architectural) issues to limit
> > churn inside the patch, rather than fix those issues in a way that's
> > appropriate for the project as a whole.
> >
>
> Surely those architectural shortcomings should be identified in a review
> - which however requires time to do properly, and thus is an argument
> for ensuring there's enough time for such review (which is in direct
> conflict with the last-minute crush, IMO).
>
> Once again, I 100% agree we need to do better in handling patches from
> non-committers, absolutely no argument there. But does it require this
> last-minute crush? I don't think so, it can't be at the expense of
> proper review and getting it right.

Agreed on this, 100%, but as mentioned above, the March commitfest
contains more than just "last minute crushes" [^1]. I don't think we
should throw out the baby with the bathwater here.

> A complex patch needs to be
> submitted early in the cycle, not in the last CF. If it's submitted
> early, but does not receive enough interest/reviews, I think we need to
> fix & improve that - not to rework/push it at the very end.

Agree on all this, too.

-Matthias


[^0] (see e.g. the EXPLAIN SERIALIZE patch thread [0], where the
original author did not have the time capacity to maintain the
patchset over months:
https://www.postgresql.org/message-id/ca0adb0e-fa4e-c37e-1cd7-91170b18cae1@gmx.de

[^1] Are there metrics on how many of the committed patches this CF
were new, only registered this CF, and if they're more or less
distributed to the feature freeze when compared to longer-running
patchess? I can probably build these statistics by extracting the data
from the webpages, but that's quite tedious.
A manual count gets me 68 new patches (~50% of all committed
registered patches); distribution comparison isn't in my time budget.