Thread: Commitfest 2024-01 first week update

Commitfest 2024-01 first week update

From
vignesh C
Date:
Hi,

Here's a quick status report after the first week:
Status summary:
  Needs review: 238.
  Waiting on Author: 44.
  Ready for Committer: 27.
  Committed: 36.
  Moved to next CF: 1.
  Withdrawn: 2.
  Returned with Feedback: 3.
  Rejected: 1.
Total: 352.

Here is a list of "Needs review" entries for which there has not been
much communication on the thread and needs help in proceeding further.
Hackers please pick one of these and review/share your suggestions,
that will be helpful in taking it forward:
Fixes for non-atomic read of read of control file on ext4 + ntfs | Thomas Munro
Recovering from detoast-related catcache invalidations | Tom Lane
Dump-restore loosing 'attnotnull' bit for DEFERRABLE PRIMARY KEY
column(s) | Amul sul
pgbench: allow to cancel queries during benchmark | Yugo Nagata
vacuumdb/clusterdb/reindexdb: allow specifying objects to process in
all databases | Nathan Bossart
Add additional extended protocol commands to psql: \parse and \bindx |
Anthonin Bonnefoy
Function to log backtrace of postgres processes | Vignesh C/Bharath Rupireddy
Check consistency of GUC defaults between .sample.conf and
pg_settings.boot_val | Nathan Bossart
archive modules loose ends | Nathan Bossart
make pg_ctl more friendly | Crisp Lee
locked reads for atomics | Nathan Bossart
common signal handler protection | Nathan Bossart
Reuse child_relids in try_partitionwise_join | Ashutosh Bapat

Here is a list of "Ready for Committer" entries for which there has
not been much communication on the thread and needs help in proceeding
further. If any of the committers has some time to spare, please help
us on these:
Skip hidden files in serverside utilities | Daniel Gustafsson
Unlinking Parallel Hash Join inner batch files sooner | Thomas Munro
pg_basebackup: mention that spread checkpoints are the default | Michael Banck
Evaluate arguments of correlated SubPlans in the referencing ExprState
| Andres Freund
Cross-database SERIALIZABLE safe snapshots | Thomas Munro
Add support function for range containment operators | Kim Johan Andersson
Refactor pipe_read_line as a more generic interface for reading
arbitrary strings off a pipe | Daniel Gustafsson
Use atomic ops for unlogged LSN | John Morris
functions to compute size of schemas/AMs (and maybe \dn++ and \dA++) |
Justin Pryzby
Improve Boolean Predicate JSON Path Docs | David Wheeler
Add PQsendPipelineSync() to libpq | Anton Kirilov
Trigger violates foreign key constraint | Laurenz Albe

If you have submitted a patch and it's in "Waiting for author" state,
please get the patch to "Needs review" state soon if you can, as
that's where people are most likely to be looking for things to
review.

I have pinged many threads that are in "Ready for Committer", "Needs
review" state and don't apply, compile warning-free, or pass
check-world.  I have also updated the status of the patches which were
not in the correct state.

I will also be pinging the patch owners to review a few patches who
have submitted one or more patches but have not picked any of the
patches for review.

Regards,
Vignesh



Re: Commitfest 2024-01 first week update

From
Jelte Fennema-Nio
Date:
On Mon, 8 Jan 2024 at 07:22, vignesh C <vignesh21@gmail.com> wrote:
> Here is a list of "Needs review" entries for which there has not been
> much communication on the thread and needs help in proceeding further.

Thank you for creating these lists. It's definitely helpful to see
what to focus my reviewing effort on. I noticed they are missing one
of my commitfest entries though:

Add non-blocking version of PQcancel | Jelte Fennema-Nio

imho this patch is pretty much finished, but it has only received
English spelling feedback in the last 8 months (I addressed the
feedback).



Re: Commitfest 2024-01 first week update

From
vignesh C
Date:
On Mon, 8 Jan 2024 at 22:50, Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
>
> On Mon, 8 Jan 2024 at 07:22, vignesh C <vignesh21@gmail.com> wrote:
> > Here is a list of "Needs review" entries for which there has not been
> > much communication on the thread and needs help in proceeding further.
>
> Thank you for creating these lists. It's definitely helpful to see
> what to focus my reviewing effort on. I noticed they are missing one
> of my commitfest entries though:

This is just one list for this week, I will be focussing on a
different set of patches for review in the next week.

> Add non-blocking version of PQcancel | Jelte Fennema-Nio
>
> imho this patch is pretty much finished, but it has only received
> English spelling feedback in the last 8 months (I addressed the
> feedback).

Thanks, One of us will have a look at this patch.

Regards,
Vignesh



Re: Commitfest 2024-01 first week update

From
Robert Haas
Date:
Hi,

I think we need to be more aggressive about marking things returned
with feedback when they don't get updated. If a patch is waiting for
reviews for a long time, well, that's one thing. Maybe we eventually
close it due to lack of interest in reviewing it, but that should be
done cautiously, as it will understandably piss people off. But I
regularly find patches in the CommitFest which have been waiting on
author for multiple commitfests and are just repeatedly moved forward.
That's crazy to me. It makes the CommitFest application fill up with
junk that obscures what actually needs to be dealt with.

...Robert



Re: Commitfest 2024-01 first week update

From
vignesh C
Date:
On Wed, 10 Jan 2024 at 03:48, Robert Haas <robertmhaas@gmail.com> wrote:
>
> Hi,
>
> I think we need to be more aggressive about marking things returned
> with feedback when they don't get updated. If a patch is waiting for
> reviews for a long time, well, that's one thing. Maybe we eventually
> close it due to lack of interest in reviewing it, but that should be
> done cautiously, as it will understandably piss people off. But I
> regularly find patches in the CommitFest which have been waiting on
> author for multiple commitfests and are just repeatedly moved forward.
> That's crazy to me. It makes the CommitFest application fill up with
> junk that obscures what actually needs to be dealt with.

I also noticed that many patches get carried forward to the next
commitfest. I will start highlighting these kinds of patches to the
author, request them to act upon the patches and return the patch if
nothing is done in this commitfest. I'm also working on the needs
review patches, I have already withdrawn one of my patches yesterday
at [1] which got no response for a long time. I also got one on my
colleagues's patch to be withdrawn by requesting him internally at
[2]. I will be posting these kinds of patches in my weekly update. It
will be good if one of the senior folks reply whether to take it
forward or not, that will help me in taking a decision quickly on what
to do with these patches.

One kind suggestion for all the patches which have been inactive for a
long time is for the author to re-assess the patch once again and act
on the entry voluntarily which will help the committers and reviewers
in spending time on the real patch which requires focus.

[1] - https://www.postgresql.org/message-id/CALDaNm1AQZYgT0tALRrkvpP1Q%2B8%2Be7vkGCUjQ-jim1C0q3e%3DzA%40mail.gmail.com
[2] - https://www.postgresql.org/message-id/CAHut+PtBX9S146Zq1CQUQQJ3n-P3ZSV4w9AHfC-LDsX5T5uT9w@mail.gmail.com

Regards,
Vignesh



Re: Commitfest 2024-01 first week update

From
Daniel Gustafsson
Date:
> On 9 Jan 2024, at 23:18, Robert Haas <robertmhaas@gmail.com> wrote:

> I think we need to be more aggressive about marking things returned
> with feedback when they don't get updated.

I very much agree.  Having marked quite a lot of patches as RwF when being CFM
I can attest that it gets very little off-list pushback or angry emails.  While
it does happen, the overwhelming majority of responses are understanding and
positive, so no CFM should be worried about "being the bad guy".

--
Daniel Gustafsson




Re: Commitfest 2024-01 first week update

From
Alvaro Herrera
Date:
On 2024-Jan-10, Daniel Gustafsson wrote:

> > On 9 Jan 2024, at 23:18, Robert Haas <robertmhaas@gmail.com> wrote:
> 
> > I think we need to be more aggressive about marking things returned
> > with feedback when they don't get updated.
> 
> I very much agree.  Having marked quite a lot of patches as RwF when being CFM
> I can attest that it gets very little off-list pushback or angry emails.  While
> it does happen, the overwhelming majority of responses are understanding and
> positive, so no CFM should be worried about "being the bad guy".

I like this idea very much -- return patches when the author does not
respond AFTER receiving feedback or the patch rotting.

However, this time around I saw that a bunch of patches were returned or
threatened to be returned JUST BECAUSE nobody had replied to the thread,
with a justification like "you need to generate more interest in your
patch".  This is a TERRIBLE idea, and there's one reason why creating a
new commitfest entry in the following commitfest is no good: 

At the FOSDEM developer meeting, we do a run of CF patch triage, where
we check the topmost patches in order of number-of-commitfests.  If you
return an old patch and a new CF entry is created, this number is reset,
and we could quite possibly fail to detect some very old patch because
of this.  At times, the attention a patch gets during the CF triage is
sufficient to get the patch moving forward after long inactivity, so
this is not academic.  Case in point: [1].

So by all means let's return patches that rot or fail to get updated per
feedback.  But DO NOT return patches because of inactivity.

[1] https://postgr.es/m/202402011550.sfszd46247zi@alvherre.pgsql

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"Las cosas son buenas o malas segun las hace nuestra opinión" (Lisias)



Re: Commitfest 2024-01 first week update

From
vignesh C
Date:
On Sun, 4 Feb 2024 at 14:32, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2024-Jan-10, Daniel Gustafsson wrote:
>
> > > On 9 Jan 2024, at 23:18, Robert Haas <robertmhaas@gmail.com> wrote:
> >
> > > I think we need to be more aggressive about marking things returned
> > > with feedback when they don't get updated.
> >
> > I very much agree.  Having marked quite a lot of patches as RwF when being CFM
> > I can attest that it gets very little off-list pushback or angry emails.  While
> > it does happen, the overwhelming majority of responses are understanding and
> > positive, so no CFM should be worried about "being the bad guy".
>
> I like this idea very much -- return patches when the author does not
> respond AFTER receiving feedback or the patch rotting.
>
> However, this time around I saw that a bunch of patches were returned or
> threatened to be returned JUST BECAUSE nobody had replied to the thread,
> with a justification like "you need to generate more interest in your
> patch".  This is a TERRIBLE idea, and there's one reason why creating a
> new commitfest entry in the following commitfest is no good:

I have seen that most of the threads are being discussed and being
promptly updated. But very few of the entries become stale and just
move from one commitfest to another commitfest without anything being
done. For these kinds of entries, we were just trying to see if the
author or anybody is really interested or not in pursuing it.

We should do something about these kinds of entries, there were few
suggestions like tagging under a new category or so, can we add a new
status to park these entries something like "Waiting for direction".
The threads which have no discussion for 6 months or so can be flagged
to this new status and these can be discussed in one of the developer
meetings or so and conclude on these items.

Regards,
Vignesh



Re: Commitfest 2024-01 first week update

From
Jacob Champion
Date:
On Sun, Feb 4, 2024 at 6:51 AM vignesh C <vignesh21@gmail.com> wrote:
> We should do something about these kinds of entries, there were few
> suggestions like tagging under a new category or so, can we add a new
> status to park these entries something like "Waiting for direction".
> The threads which have no discussion for 6 months or so can be flagged
> to this new status and these can be discussed in one of the developer
> meetings or so and conclude on these items.

See also [1], with a patch suggestion. IIRC, there was also related
discussion on making the "resurrection" process single-step so that
you don't lose history, per Alvaro's concern.

--Jacob

[1] https://postgr.es/m/flat/CAAWbhmjM6hn_3sjVBUyqVKysWVAtsBOkaNt01QF3AMOCunuKMg%40mail.gmail.com



Re: Commitfest 2024-01 first week update

From
Alvaro Herrera
Date:
On 2024-Feb-04, vignesh C wrote:

> We should do something about these kinds of entries, there were few
> suggestions like tagging under a new category or so, can we add a new
> status to park these entries something like "Waiting for direction".
> The threads which have no discussion for 6 months or so can be flagged
> to this new status and these can be discussed in one of the developer
> meetings or so and conclude on these items.

Maybe a new status is appropriate ... I would suggest "Stalled".  Such a
patch still applies and has no pending feedback, but nobody seems
interested.  Making a patch no longer stalled means there's discussion
that leads to:

1. further development?  Perhaps the author just needed more direction.
2. a decision that it's not a feature we want, or maybe not in this
   form.  Then we close it as rejected.
3. a reviewer/committer finding time to provide additional feedback.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"The problem with the future is that it keeps turning into the present"
(Hobbes)



Re: Commitfest 2024-01 first week update

From
Daniel Gustafsson
Date:
> On 7 Feb 2024, at 13:37, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

> Maybe a new status is appropriate ... I would suggest "Stalled".  Such a
> patch still applies and has no pending feedback, but nobody seems
> interested.

Since the CF app knows when the last email in the thread was, the state of the
patch entry and the number of CF's which is has been present in; maybe we can
extend the app to highlight these patches in a way which doesn't add more
manual intervention?  It might yield a few false positives, but so will setting
it manually.

--
Daniel Gustafsson




Re: Commitfest 2024-01 first week update

From
Alvaro Herrera
Date:
On 2024-Feb-07, Daniel Gustafsson wrote:

> Since the CF app knows when the last email in the thread was, the
> state of the patch entry and the number of CF's which is has been
> present in; maybe we can extend the app to highlight these patches in
> a way which doesn't add more manual intervention?  It might yield a
> few false positives, but so will setting it manually.

Hmm, but suppose the author is posting new versions now and again
because of apply conflicts; and the CFM is pinging them about that, but
not providing any actionable feedback.  Such a thread would be active
enough, but the patch is still stalled.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"No hay hombre que no aspire a la plenitud, es decir,
la suma de experiencias de que un hombre es capaz"



Re: Commitfest 2024-01 first week update

From
Amit Kapila
Date:
On Wed, Feb 7, 2024 at 6:07 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2024-Feb-04, vignesh C wrote:
>
> > We should do something about these kinds of entries, there were few
> > suggestions like tagging under a new category or so, can we add a new
> > status to park these entries something like "Waiting for direction".
> > The threads which have no discussion for 6 months or so can be flagged
> > to this new status and these can be discussed in one of the developer
> > meetings or so and conclude on these items.
>
> Maybe a new status is appropriate ... I would suggest "Stalled".  Such a
> patch still applies and has no pending feedback, but nobody seems
> interested.  Making a patch no longer stalled means there's discussion
> that leads to:
>
> 1. further development?  Perhaps the author just needed more direction.
> 2. a decision that it's not a feature we want, or maybe not in this
>    form.  Then we close it as rejected.
> 3. a reviewer/committer finding time to provide additional feedback.
>

+1. This suggestion sounds reasonable. I think we probably need to
decide the time when the patch's status should be changed to
"Stalled".

--
With Regards,
Amit Kapila.



Re: Commitfest 2024-01 first week update

From
Daniel Gustafsson
Date:
> On 7 Feb 2024, at 14:15, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2024-Feb-07, Daniel Gustafsson wrote:
>
>> Since the CF app knows when the last email in the thread was, the
>> state of the patch entry and the number of CF's which is has been
>> present in; maybe we can extend the app to highlight these patches in
>> a way which doesn't add more manual intervention?  It might yield a
>> few false positives, but so will setting it manually.
>
> Hmm, but suppose the author is posting new versions now and again
> because of apply conflicts; and the CFM is pinging them about that, but
> not providing any actionable feedback.  Such a thread would be active
> enough, but the patch is still stalled.

If the patch author is actively rebasing the patch and thus generating traffic
on the thread I'm not sure I would call it stalled - there might not be enough
(or any) reviews but the activity alone might make someone interested.  Either
way, I'd personally prefer such false positives if it means reducing the manual
workload for the CFM.

--
Daniel Gustafsson