Re: PostgreSQL 17 Release Management Team & Feature Freeze

On 4/9/24 11:25, Matthias van de Meent wrote:
> 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.
> 

But I don't think I suggested to ban such patches from the March CF
entirely. Surely those patches can still be submitted, reviewed, and
even reworked in the last CF. All I said is it might be better to treat
those patches as not committable by default. Sorry if that wasn't clear
enough ...

Would this be an additional restriction? I'm not quite sure. Surely if
the intent is to only commit patches that we agree are in "sufficiently
good" shape, and getting them into such shape requires time & reviews,
this would not be a significant change.

FWIW I'm not a huge fan of hard unbreakable rules, so there should be
some leeway when justified, but the bar would be somewhat higher (clear
consensus, RMT having a chance to say no, ...).

>>> 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.
> 

OK

> -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
> 

Yeah, this is terrible :-(

> [^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.

I suppose there are, or would be possible to get from the CF app. And
perhaps it'd be good to look at some of this, I think a lot of the
discussion here is based on very subjective perceptions of how the
process works.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company