Thread: Commitfest 2023-09 starts soon

Commitfest 2023-09 starts soon

From
Peter Eisentraut
Date:
Commitfest 2023-09 (https://commitfest.postgresql.org/44/) starts in 
less than 28 hours.

If you have any patches you would like considered, be sure to add them 
in good time.

All patch authors, and especially experienced hackers, are requested to 
make sure the patch status is up to date.  If the patch is still being 
worked on, it might not need to be in "Needs review".  Conversely, if 
you are hoping for a review but the status is "Waiting on author", then 
it will likely be ignored by reviewers.

There are a number of patches carried over from the PG16 development 
cycle that have been in "Waiting on author" for several months.  I will 
aggressively prune those after the start of this commitfest if there 
hasn't been any author activity by then.



Re: Commitfest 2023-09 starts soon

From
Aleksander Alekseev
Date:
Hi Peter,

> Commitfest 2023-09 (https://commitfest.postgresql.org/44/) starts in
> less than 28 hours.
>
> If you have any patches you would like considered, be sure to add them
> in good time.
>
> All patch authors, and especially experienced hackers, are requested to
> make sure the patch status is up to date.  If the patch is still being
> worked on, it might not need to be in "Needs review".  Conversely, if
> you are hoping for a review but the status is "Waiting on author", then
> it will likely be ignored by reviewers.
>
> There are a number of patches carried over from the PG16 development
> cycle that have been in "Waiting on author" for several months.  I will
> aggressively prune those after the start of this commitfest if there
> hasn't been any author activity by then.

The "64-bit TOAST value ID" [1] is one of such "Waiting on author"
patches I've been reviewing. See the last 2-3 messages in the thread.
I believe it's safe to mark it as RwF for now.

[1]: https://commitfest.postgresql.org/44/4296/

-- 
Best regards,
Aleksander Alekseev



Re: Commitfest 2023-09 starts soon

From
Aleksander Alekseev
Date:
Hi,

> > There are a number of patches carried over from the PG16 development
> > cycle that have been in "Waiting on author" for several months.  I will
> > aggressively prune those after the start of this commitfest if there
> > hasn't been any author activity by then.
>
> The "64-bit TOAST value ID" [1] is one of such "Waiting on author"
> patches I've been reviewing. See the last 2-3 messages in the thread.
> I believe it's safe to mark it as RwF for now.
>
> [1]: https://commitfest.postgresql.org/44/4296/

This was the one that I could name off the top of my head.

1. Flush SLRU counters in checkpointer process
https://commitfest.postgresql.org/44/4120/

Similarly, I suggest marking it as RwF

2. Allow logical replication via inheritance root table
https://commitfest.postgresql.org/44/4225/

This one seems to be in active development. Changing status to "Needs
review" since it definitely could use more code review.

3. ResourceOwner refactoring
https://commitfest.postgresql.org/44/3982/

The patch is in good shape but requires actions from Heikki. I suggest
keeping it as is for now.

4. Move SLRU data into the regular buffer pool
https://commitfest.postgresql.org/44/3514/

Rotted one and for a long time. Suggestion: RwF

5. A minor adjustment to get_cheapest_path_for_pathkeys
https://commitfest.postgresql.org/44/4286/

Doesn't seem to be valuable. Suggestion: Rejected.

I will apply corresponding status changes if there will be no objections.

-- 
Best regards,
Aleksander Alekseev



Re: Commitfest 2023-09 starts soon

From
Peter Eisentraut
Date:
Okay, here we go, starting with:

Status summary: Needs review: 227. Waiting on Author: 37. Ready for 
Committer: 30. Committed: 40. Rejected: 1. Returned with Feedback: 1. 
Withdrawn: 1. Total: 337.

(which is less than CF 2023-07)

I have also already applied one round of the waiting-on-author-pruning 
described below (not included in the above figures).


On 31.08.23 10:36, Peter Eisentraut wrote:
> Commitfest 2023-09 (https://commitfest.postgresql.org/44/) starts in 
> less than 28 hours.
> 
> If you have any patches you would like considered, be sure to add them 
> in good time.
> 
> All patch authors, and especially experienced hackers, are requested to 
> make sure the patch status is up to date.  If the patch is still being 
> worked on, it might not need to be in "Needs review".  Conversely, if 
> you are hoping for a review but the status is "Waiting on author", then 
> it will likely be ignored by reviewers.
> 
> There are a number of patches carried over from the PG16 development 
> cycle that have been in "Waiting on author" for several months.  I will 
> aggressively prune those after the start of this commitfest if there 
> hasn't been any author activity by then.
> 
> 




Re: Commitfest 2023-09 starts soon

From
Aleksander Alekseev
Date:
Hi Peter,

> Okay, here we go, starting with:
>
> Status summary: Needs review: 227. Waiting on Author: 37. Ready for
> Committer: 30. Committed: 40. Rejected: 1. Returned with Feedback: 1.
> Withdrawn: 1. Total: 337.
>
> (which is less than CF 2023-07)
>
> I have also already applied one round of the waiting-on-author-pruning
> described below (not included in the above figures).

* Index SLRUs by 64-bit integers rather than by 32-bit integers
https://commitfest.postgresql.org/44/3489/

The status here was changed to "Needs Review". These patches are in
good shape and previously were marked as "Ready for Committer".
Actually I thought Heikki would commit them to PG16, but it didn't
happen. If there are no objections, I will return the RfC status in a
bit since it seems to be more appropriate in this case.

-- 
Best regards,
Aleksander Alekseev



Re: Commitfest 2023-09 starts soon

From
Peter Eisentraut
Date:
On 04.09.23 15:22, Aleksander Alekseev wrote:
> Hi Peter,
> 
>> Okay, here we go, starting with:
>>
>> Status summary: Needs review: 227. Waiting on Author: 37. Ready for
>> Committer: 30. Committed: 40. Rejected: 1. Returned with Feedback: 1.
>> Withdrawn: 1. Total: 337.
>>
>> (which is less than CF 2023-07)
>>
>> I have also already applied one round of the waiting-on-author-pruning
>> described below (not included in the above figures).
> 
> * Index SLRUs by 64-bit integers rather than by 32-bit integers
> https://commitfest.postgresql.org/44/3489/
> 
> The status here was changed to "Needs Review". These patches are in
> good shape and previously were marked as "Ready for Committer".
> Actually I thought Heikki would commit them to PG16, but it didn't
> happen. If there are no objections, I will return the RfC status in a
> bit since it seems to be more appropriate in this case.

The patch was first set to "Ready for Committer" on 2023-03-29, and if I 
pull up the thread in the web archive view, that is in the middle of the 
page.  So as a committer, I would expect that someone would review 
whatever happened in the second half of that thread before turning it 
over to committer.

As a general rule, if significant additional discussion or patch posting 
happens after a patch is set to "Ready for Committer", I'm setting it 
back to "Needs review" until someone actually re-reviews it.

I also notice that you are listed as both author and reviewer of that 
patch, which I think shouldn't be done.  It appears that you are in fact 
the author, so I would recommend that you remove yourself from the 
reviewers.




Re: Commitfest 2023-09 starts soon

From
Peter Eisentraut
Date:
I have done several passes to make sure that patch statuses are more 
accurate.  As explained in a nearby message, I have set several patches 
back from "Ready to Committer" to "Needs review" if additional 
discussion happened past the first status change.  I have also in 
several cases removed reviewers from a patch entry if they haven't been 
active on the thread in several months.  (Feel free to sign up again, 
but don't "block" the patch.)

I was going to say, unfortunately that means that there is now even more 
work to do, but in fact, it seems this has all kind of balanced out, and 
I hope it's now more accurate for someone wanting to pick up some work:

Status summary: Needs review: 224. Waiting on Author: 24. Ready for 
Committer: 31. Committed: 41. Returned with Feedback: 14. Withdrawn: 2. 
Rejected: 2. Total: 338.

(And yes, the total number of patches has grown since the commitfest has 
started!?!)


On 01.09.23 14:55, Peter Eisentraut wrote:
> Okay, here we go, starting with:
> 
> Status summary: Needs review: 227. Waiting on Author: 37. Ready for 
> Committer: 30. Committed: 40. Rejected: 1. Returned with Feedback: 1. 
> Withdrawn: 1. Total: 337.
> 
> (which is less than CF 2023-07)
> 
> I have also already applied one round of the waiting-on-author-pruning 
> described below (not included in the above figures).
> 
> 
> On 31.08.23 10:36, Peter Eisentraut wrote:
>> Commitfest 2023-09 (https://commitfest.postgresql.org/44/) starts in 
>> less than 28 hours.
>>
>> If you have any patches you would like considered, be sure to add them 
>> in good time.
>>
>> All patch authors, and especially experienced hackers, are requested 
>> to make sure the patch status is up to date.  If the patch is still 
>> being worked on, it might not need to be in "Needs review".  
>> Conversely, if you are hoping for a review but the status is "Waiting 
>> on author", then it will likely be ignored by reviewers.
>>
>> There are a number of patches carried over from the PG16 development 
>> cycle that have been in "Waiting on author" for several months.  I 
>> will aggressively prune those after the start of this commitfest if 
>> there hasn't been any author activity by then.
>>
>>
> 
> 
> 




Re: Commitfest 2023-09 starts soon

From
Aleksander Alekseev
Date:
Hi Peter,

> The patch was first set to "Ready for Committer" on 2023-03-29, and if I
> pull up the thread in the web archive view, that is in the middle of the
> page.  So as a committer, I would expect that someone would review
> whatever happened in the second half of that thread before turning it
> over to committer.
>
> As a general rule, if significant additional discussion or patch posting
> happens after a patch is set to "Ready for Committer", I'm setting it
> back to "Needs review" until someone actually re-reviews it.

OK, fair enough.

> I also notice that you are listed as both author and reviewer of that
> patch, which I think shouldn't be done.  It appears that you are in fact
> the author, so I would recommend that you remove yourself from the
> reviewers.

Sometimes I start as a reviewer and then for instance add my own
patches to the thread. In cases like this I end up being both an
author and a reviewer, but it doesn't mean that I review my own
patches :)

In this particular case IMO it would be appropriate to remove myself
from the list of reviewers. So I did.

Thanks!

-- 
Best regards,
Aleksander Alekseev



Re: Commitfest 2023-09 starts soon

From
Matthias van de Meent
Date:
On Thu, 31 Aug 2023 at 14:35, Aleksander Alekseev
<aleksander@timescale.com> wrote:
>
> Hi,
> > On Thu, 31 Aug 2023 at 11:37, Peter Eisentraut <peter@eisentraut.org> wrote:
> > > There are a number of patches carried over from the PG16 development
> > > cycle that have been in "Waiting on author" for several months.  I will
> > > aggressively prune those after the start of this commitfest if there
> > > hasn't been any author activity by then.
> >
> > [1 patch]
>
> This was the one that I could name off the top of my head.
>
> [5 more patches]
>
> I will apply corresponding status changes if there will be no objections.

On Mon, 4 Sept 2023 at [various], Aleksander Alekseev
<aleksander@timescale.com> wrote:
>
> Hi,
>
> > [various patches]
>
> A consensus was reached [1] to mark this patch as RwF for now. There
> are many patches to be reviewed and this one doesn't seem to be in the
> best shape, so we have to prioritise. Please feel free re-submitting
> the patch for the next commitfest.

I'm a bit confused about your use of "consensus". True, there was no
objection, but it looks like no patch author or reviewer was informed
(cc-ed or directly referenced) that the patch was being discussed
before achieving this "consensus", and the "consensus" was reached
within 4 days, of which 2 weekend, in a thread that has (until now)
involved only you and Peter E.

Usually, you'd expect discussion about a patch to happen on the
patch's thread before any action is taken (or at least a mention on
that thread), but quite clearly that hasn't happened here.
Are patch authors expected to follow any and all discussion on threads
with "Commitfest" in the title?
If so, shouldn't the relevant wiki pages be updated, and/or the
-hackers community be updated by mail in a new thread about these
policy changes?

Kind regards,

Matthias van de Meent
Neon (https://neon.tech)

[0] https://wiki.postgresql.org/wiki/Submitting_a_Patch



Re: Commitfest 2023-09 starts soon

From
Aleksander Alekseev
Date:
Hi Matthias,

> I'm a bit confused about your use of "consensus". True, there was no
> objection, but it looks like no patch author or reviewer was informed
> (cc-ed or directly referenced) that the patch was being discussed
> before achieving this "consensus", and the "consensus" was reached
> within 4 days, of which 2 weekend, in a thread that has (until now)
> involved only you and Peter E.
>
> Usually, you'd expect discussion about a patch to happen on the
> patch's thread before any action is taken (or at least a mention on
> that thread), but quite clearly that hasn't happened here.
> Are patch authors expected to follow any and all discussion on threads
> with "Commitfest" in the title?
> If so, shouldn't the relevant wiki pages be updated, and/or the
> -hackers community be updated by mail in a new thread about these
> policy changes?

I understand your disappointment and assure you that no one is acting
with bad intentions here. Also please note that English is a second
language for many of us which represents a challenge when it comes to
expressing thoughts on the mailing list. We have a common goal here,
to make PostgreSQL an even better system than it is now.

The patches under question were in "Waiting for Author" state for a
*long* time and the authors were notified about this. We could toss
such patches from one CF to another month after month or mark as RwF
and let the author know that no one is going to review that patch
until the author takes the actions. It's been noted that the letter
approach is more productive in the long run. The discussion can
continue in the same thread and the same thread can be registered for
the upcoming CF.

This being said, Peter is the CF manager, so he has every right to
change the status of the patches under questions if he disagrees.

-- 
Best regards,
Aleksander Alekseev



Re: Commitfest 2023-09 starts soon

From
Matthias van de Meent
Date:
On Mon, 4 Sept 2023 at 18:19, Aleksander Alekseev
<aleksander@timescale.com> wrote:
>
> Hi Matthias,
>
> > I'm a bit confused about your use of "consensus". True, there was no
> > objection, but it looks like no patch author or reviewer was informed
> > (cc-ed or directly referenced) that the patch was being discussed
> > before achieving this "consensus", and the "consensus" was reached
> > within 4 days, of which 2 weekend, in a thread that has (until now)
> > involved only you and Peter E.
> >
> > Usually, you'd expect discussion about a patch to happen on the
> > patch's thread before any action is taken (or at least a mention on
> > that thread), but quite clearly that hasn't happened here.
> > Are patch authors expected to follow any and all discussion on threads
> > with "Commitfest" in the title?
> > If so, shouldn't the relevant wiki pages be updated, and/or the
> > -hackers community be updated by mail in a new thread about these
> > policy changes?
>
> I understand your disappointment and assure you that no one is acting
> with bad intentions here. Also please note that English is a second
> language for many of us which represents a challenge when it comes to
> expressing thoughts on the mailing list. We have a common goal here,
> to make PostgreSQL an even better system than it is now.
>
> The patches under question were in "Waiting for Author" state for a
> *long* time and the authors were notified about this. We could toss
> such patches from one CF to another month after month or mark as RwF
> and let the author know that no one is going to review that patch
> until the author takes the actions. It's been noted that the letter
> approach is more productive in the long run.

This far I agree - we can't keep patches around with issues if they're
not being worked on. And I do appreciate your work on pruning dead or
stale patches. But:

> The discussion can
> continue in the same thread and the same thread can be registered for
> the upcoming CF.

This is one of my major concerns here: Patch resolution is being
discussed on -hackers, but outside of the thread used to discuss that
patch (as indicated in the CF app), and without apparent author
inclusion.To me, that feels like going behind the author's back, and I
don't think that this should be normalized.

As mentioned in the earlier mail, my other concern is the use of
"consensus" in this context. You link to a message on -hackers, with
no visible agreements. As a patch author myself, if a lack of comments
on my patch in an otherwise unrelated thread is "consensus", then I'll
probably move all patches that have yet to be commented on to RfC, as
there'd be "consensus" that they should be included as-is in
PostgreSQL. But I digress.

I think it would be better to just remove the "consensus" part of your
mail, and just put down the real reason why each patch is being RfC-ed
or rejected. That is, don't imply that there are hackers that OK-ed it
when there are none, and inform patch authors directly about the
reasons why the patch is being revoked; so without "see consensus in
[0]".

Kind regards,

Matthias van de Meent



Re: Commitfest 2023-09 starts soon

From
Melanie Plageman
Date:
On Mon, Sep 4, 2023 at 1:01 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> I have done several passes to make sure that patch statuses are more
> accurate.  As explained in a nearby message, I have set several patches
> back from "Ready to Committer" to "Needs review" if additional
> discussion happened past the first status change.  I have also in
> several cases removed reviewers from a patch entry if they haven't been
> active on the thread in several months.  (Feel free to sign up again,
> but don't "block" the patch.)

I had originally planned to write thread summaries for all status="Needs
review" patches in the commitfest. However, ISTM these summaries are
largely useful for reaching consensus on changing the status of these
patches (setting them to "waiting on author", "returned with feedback",
etc). Since Peter has already updated all of the statuses, I've decided
not to write summaries and instead spend that time doing review.

However, I thought it might be useful to provide a list of all of the
"Needs review" entries which, at the time of writing, have had zero
reviews or replies (except from the author):

Document efficient self-joins / UPDATE LIMIT techniques.
https://commitfest.postgresql.org/44/4539/

pg_basebackup: Always return valid temporary slot names
https://commitfest.postgresql.org/44/4534/

pg_resetwal tests, logging, and docs update
https://commitfest.postgresql.org/44/4533/

Streaming I/O, vectored I/O
https://commitfest.postgresql.org/44/4532/

Improving the heapgetpage function improves performance in common scenarios
https://commitfest.postgresql.org/44/4524/

CI speed improvements for FreeBSD
https://commitfest.postgresql.org/44/4520/

Implementation of distinct in Window Aggregates: take two
https://commitfest.postgresql.org/44/4519/

Improve pg_restore toc file parsing and format for better performances
https://commitfest.postgresql.org/44/4509/

Fix false report of wraparound in pg_serial
https://commitfest.postgresql.org/44/4516/

Simplify create_merge_append_path a bit for clarity
https://commitfest.postgresql.org/44/4496/

Fix bogus Asserts in calc_non_nestloop_required_outer
https://commitfest.postgresql.org/44/4478/

Retiring is_pushed_down
https://commitfest.postgresql.org/44/4458/

Flush disk write caches by default on macOS and Windows
https://commitfest.postgresql.org/44/4453/

Add last_commit_lsn to pg_stat_database
https://commitfest.postgresql.org/44/4355/

Optimizing "boundary cases" during backward scan B-Tree index descents
https://commitfest.postgresql.org/44/4380/

XLog size reductions: Reduced XLog record header size
https://commitfest.postgresql.org/44/4386/

Unified file access using virtual file descriptors
https://commitfest.postgresql.org/44/4420/

Optimise index range scan by performing first check with upper page boundary
https://commitfest.postgresql.org/44/4434/

Revises for the check of parameterized partial paths
https://commitfest.postgresql.org/44/4425/

Opportunistically pruning page before update
https://commitfest.postgresql.org/44/4384/

Checks in RegisterBackgroundWorker()
https://commitfest.postgresql.org/44/4514/

Allow direct lookups of SpecialJoinInfo by ojrelid
https://commitfest.postgresql.org/44/4313/

Parent/child context relation in pg_get_backend_memory_contexts()
https://commitfest.postgresql.org/44/4379/

Support Right Semi Join
https://commitfest.postgresql.org/44/4284/

bug: ANALYZE progress report with inheritance tables
https://commitfest.postgresql.org/44/4144/

archive modules loose ends
https://commitfest.postgresql.org/44/4192/

- Melanie



Re: Commitfest 2023-09 starts soon

From
Aleksander Alekseev
Date:
Hi,

> I think it would be better to just remove the "consensus" part of your
> mail, and just put down the real reason why each patch is being RfC-ed
> or rejected. That is, don't imply that there are hackers that OK-ed it
> when there are none, and inform patch authors directly about the
> reasons why the patch is being revoked; so without "see consensus in
> [0]".

That's fair enough. I will use "It's been decided" or something like
this next time to avoid any confusion.

-- 
Best regards,
Aleksander Alekseev