Обсуждение: Commitfest 2023-09 starts soon
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.
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
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
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. > >
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
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.
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. >> >> > > >
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
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
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
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
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
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