Thread: Commitfest overflow
There are 273 patches in the queue for the Sept Commitfest already, so it seems clear the queue is not being cleared down each CF as it was before. We've been trying hard, but it's overflowing. Of those, about 50 items have been waiting more than one year, and about 25 entries waiting for more than two years. One problem is that many entries don't have official reviewers, though many people have commented on Hackers, making it difficult to track down items that actually need work. That wastes a lot of time for would-be reviewers (or at least, it has done for me). Please can there be some additional coordination to actively clear down this problem? I won't attempt to come up with ideas to do this, but others may wish to suggest ways that avoid Committer burn-out? I will be happy to volunteer to be part of an organized approach that spreads out the work. Thanks -- Simon Riggs http://www.EnterpriseDB.com/
On Tue, Aug 3, 2021 at 04:53:40PM +0100, Simon Riggs wrote: > There are 273 patches in the queue for the Sept Commitfest already, so > it seems clear the queue is not being cleared down each CF as it was > before. We've been trying hard, but it's overflowing. > > Of those, about 50 items have been waiting more than one year, and > about 25 entries waiting for more than two years. > > One problem is that many entries don't have official reviewers, though > many people have commented on Hackers, making it difficult to track > down items that actually need work. That wastes a lot of time for > would-be reviewers (or at least, it has done for me). > > Please can there be some additional coordination to actively clear > down this problem? I won't attempt to come up with ideas to do this, > but others may wish to suggest ways that avoid Committer burn-out? > > I will be happy to volunteer to be part of an organized approach that > spreads out the work. I wonder if our lack of in-person developer meetings is causing some of our issues to not get closed. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com If only the physical world exists, free will is an illusion.
Bruce Momjian <bruce@momjian.us> writes:
> On Tue, Aug 3, 2021 at 04:53:40PM +0100, Simon Riggs wrote:
>> There are 273 patches in the queue for the Sept Commitfest already, so
>> it seems clear the queue is not being cleared down each CF as it was
>> before. We've been trying hard, but it's overflowing.
> I wonder if our lack of in-person developer meetings is causing some of
> our issues to not get closed.
I think there are a couple of things happening here:
1. There wasn't that much getting done during this CF because it's
summer and many people are on vacation (in the northern hemisphere
anyway).
2. As a community, we don't really have the strength of will to
flat-out reject patches. I think the dynamic is that individual
committers look at something, think "I don't like that, I'll go
work on some better-designed patch", and it just keeps slipping
to the next CF. In the past we've had some CFMs who were assertive
enough and senior enough to kill off patches that didn't look like
they were going to go anywhere. But that hasn't happened for
awhile, and I'm not sure it should be the CFM's job anyway.
(I hasten to add that I'm not trying to imply that all the
long-lingering patches are hopeless. But I think some of them are.)
I don't think there's much to be done about the vacation effect;
we just have to accept that the summer CF is likely to be less
productive than others. But I'd like to see some better-formalized
way of rejecting patches that aren't going anywhere. Maybe there
should be a time limit on how many CFs a patch is allowed to just
automatically slide through?
regards, tom lane
On Tue, 3 Aug 2021 at 17:13, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Bruce Momjian <bruce@momjian.us> writes: > > On Tue, Aug 3, 2021 at 04:53:40PM +0100, Simon Riggs wrote: > >> There are 273 patches in the queue for the Sept Commitfest already, so > >> it seems clear the queue is not being cleared down each CF as it was > >> before. We've been trying hard, but it's overflowing. > > > I wonder if our lack of in-person developer meetings is causing some of > > our issues to not get closed. > > I think there are a couple of things happening here: > > 1. There wasn't that much getting done during this CF because it's > summer and many people are on vacation (in the northern hemisphere > anyway). > > 2. As a community, we don't really have the strength of will to > flat-out reject patches. I think the dynamic is that individual > committers look at something, think "I don't like that, I'll go > work on some better-designed patch", and it just keeps slipping > to the next CF. In the past we've had some CFMs who were assertive > enough and senior enough to kill off patches that didn't look like > they were going to go anywhere. But that hasn't happened for > awhile, and I'm not sure it should be the CFM's job anyway. > > (I hasten to add that I'm not trying to imply that all the > long-lingering patches are hopeless. But I think some of them are.) > > I don't think there's much to be done about the vacation effect; > we just have to accept that the summer CF is likely to be less > productive than others. But I'd like to see some better-formalized > way of rejecting patches that aren't going anywhere. Maybe there > should be a time limit on how many CFs a patch is allowed to just > automatically slide through? I guess the big number is 233 patches moved to next CF, out of 342, or 68% deferred. Perhaps we need a budget of how many patches can be moved to next CF, i.e. CF mgr is responsible for ensuring that no more than ?50% of patches can be moved forwards. Any in excess of that need to join the kill list. I would still ask for someone to spend a little time triaging things, so as to direct people who volunteer to be so directed. Many will not want to be directed, but I'm sure there must be 5-10 people who would do that? (Volunteers?) -- Simon Riggs http://www.EnterpriseDB.com/
On Tue, Aug 3, 2021 at 9:13 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Bruce Momjian <bruce@momjian.us> writes:
> On Tue, Aug 3, 2021 at 04:53:40PM +0100, Simon Riggs wrote:
>> There are 273 patches in the queue for the Sept Commitfest already, so
>> it seems clear the queue is not being cleared down each CF as it was
>> before. We've been trying hard, but it's overflowing.
> I wonder if our lack of in-person developer meetings is causing some of
> our issues to not get closed.
I think there are a couple of things happening here:
1. There wasn't that much getting done during this CF because it's
summer and many people are on vacation (in the northern hemisphere
anyway).
2. As a community, we don't really have the strength of will to
flat-out reject patches. I think the dynamic is that individual
committers look at something, think "I don't like that, I'll go
work on some better-designed patch", and it just keeps slipping
to the next CF. In the past we've had some CFMs who were assertive
enough and senior enough to kill off patches that didn't look like
they were going to go anywhere. But that hasn't happened for
awhile, and I'm not sure it should be the CFM's job anyway.
(I hasten to add that I'm not trying to imply that all the
long-lingering patches are hopeless. But I think some of them are.)
I don't think there's much to be done about the vacation effect;
we just have to accept that the summer CF is likely to be less
productive than others. But I'd like to see some better-formalized
way of rejecting patches that aren't going anywhere. Maybe there
should be a time limit on how many CFs a patch is allowed to just
automatically slide through?
+1 for the idea of allowed CFs. Secondly we can think about the patches which
have not had a response from the author since long.
regards, tom lane
Ibrar Ahmed
On Tue, Aug 3, 2021 at 11:31 PM Simon Riggs <simon.riggs@enterprisedb.com> wrote:
On Tue, 3 Aug 2021 at 17:13, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Bruce Momjian <bruce@momjian.us> writes:
> > On Tue, Aug 3, 2021 at 04:53:40PM +0100, Simon Riggs wrote:
> >> There are 273 patches in the queue for the Sept Commitfest already, so
> >> it seems clear the queue is not being cleared down each CF as it was
> >> before. We've been trying hard, but it's overflowing.
>
> > I wonder if our lack of in-person developer meetings is causing some of
> > our issues to not get closed.
>
> I think there are a couple of things happening here:
>
> 1. There wasn't that much getting done during this CF because it's
> summer and many people are on vacation (in the northern hemisphere
> anyway).
>
> 2. As a community, we don't really have the strength of will to
> flat-out reject patches. I think the dynamic is that individual
> committers look at something, think "I don't like that, I'll go
> work on some better-designed patch", and it just keeps slipping
> to the next CF. In the past we've had some CFMs who were assertive
> enough and senior enough to kill off patches that didn't look like
> they were going to go anywhere. But that hasn't happened for
> awhile, and I'm not sure it should be the CFM's job anyway.
>
> (I hasten to add that I'm not trying to imply that all the
> long-lingering patches are hopeless. But I think some of them are.)
>
> I don't think there's much to be done about the vacation effect;
> we just have to accept that the summer CF is likely to be less
> productive than others. But I'd like to see some better-formalized
> way of rejecting patches that aren't going anywhere. Maybe there
> should be a time limit on how many CFs a patch is allowed to just
> automatically slide through?
I guess the big number is 233 patches moved to next CF, out of 342, or
68% deferred.
Perhaps we need a budget of how many patches can be moved to next CF,
i.e. CF mgr is responsible for ensuring that no more than ?50% of
patches can be moved forwards. Any in excess of that need to join the
kill list.
I would still ask for someone to spend a little time triaging things,
so as to direct people who volunteer to be so directed. Many will not
want to be directed, but I'm sure there must be 5-10 people who would
do that? (Volunteers?)
Count me as a Volunteer.
--
Simon Riggs http://www.EnterpriseDB.com/
Ibrar Ahmed
On 8/3/21 8:30 PM, Simon Riggs wrote: > On Tue, 3 Aug 2021 at 17:13, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> >> Bruce Momjian <bruce@momjian.us> writes: >>> On Tue, Aug 3, 2021 at 04:53:40PM +0100, Simon Riggs wrote: >>>> There are 273 patches in the queue for the Sept Commitfest already, so >>>> it seems clear the queue is not being cleared down each CF as it was >>>> before. We've been trying hard, but it's overflowing. >> >>> I wonder if our lack of in-person developer meetings is causing some of >>> our issues to not get closed. >> >> I think there are a couple of things happening here: >> >> 1. There wasn't that much getting done during this CF because it's >> summer and many people are on vacation (in the northern hemisphere >> anyway). >> >> 2. As a community, we don't really have the strength of will to >> flat-out reject patches. I think the dynamic is that individual >> committers look at something, think "I don't like that, I'll go >> work on some better-designed patch", and it just keeps slipping >> to the next CF. In the past we've had some CFMs who were assertive >> enough and senior enough to kill off patches that didn't look like >> they were going to go anywhere. But that hasn't happened for >> awhile, and I'm not sure it should be the CFM's job anyway. >> >> (I hasten to add that I'm not trying to imply that all the >> long-lingering patches are hopeless. But I think some of them are.) >> >> I don't think there's much to be done about the vacation effect; >> we just have to accept that the summer CF is likely to be less >> productive than others. But I'd like to see some better-formalized >> way of rejecting patches that aren't going anywhere. Maybe there >> should be a time limit on how many CFs a patch is allowed to just >> automatically slide through? > > I guess the big number is 233 patches moved to next CF, out of 342, or > 68% deferred. > > Perhaps we need a budget of how many patches can be moved to next CF, > i.e. CF mgr is responsible for ensuring that no more than ?50% of > patches can be moved forwards. Any in excess of that need to join the > kill list. > How would this be different from the CFM just rejecting patches? It does not matter if there's an explicit number of patches that we allow to be moved to the next CF - someone still needs to make the decision, and I agree with Tom it probably should not be CFM's job. IMO what the CFM can do is make an assessment, share it on the mailing list with a proposal to reject the patch, and leave the decision up to the patch author. Either the author accepts the view it's futile, or decides to keep working on it, solicits some reviews, etc. > I would still ask for someone to spend a little time triaging things, > so as to direct people who volunteer to be so directed. Many will not > want to be directed, but I'm sure there must be 5-10 people who would > do that? (Volunteers?) > regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, Aug 3, 2021 at 07:30:38PM +0100, Simon Riggs wrote: > I would still ask for someone to spend a little time triaging things, > so as to direct people who volunteer to be so directed. Many will not > want to be directed, but I'm sure there must be 5-10 people who would > do that? (Volunteers?) I am usually not around long enough to handle complex patches but I considered the query id patch important and needing guidance so I was able to help on that for PG 14, and I needed Álvaro Herrera's help to complete it. My point is that some of these patches are really complex to get them through, but I think a focused effort could make a big impact, though it will take months. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com If only the physical world exists, free will is an illusion.
On Tue, Aug 3, 2021 at 08:51:57PM +0200, Tomas Vondra wrote: > How would this be different from the CFM just rejecting patches? It does not > matter if there's an explicit number of patches that we allow to be moved to > the next CF - someone still needs to make the decision, and I agree with Tom > it probably should not be CFM's job. My experience with the query id patch is that it can't be rejected because everyone wants it, but it needs work to get it in a state that everyone approves of. Sometimes it is impossible for the patch author to figure that out, and I needed Álvaro Herrera's help on the query id patch, so even I wasn't able to figure it out alone. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com If only the physical world exists, free will is an illusion.
On 8/3/21 8:57 PM, Bruce Momjian wrote: > On Tue, Aug 3, 2021 at 08:51:57PM +0200, Tomas Vondra wrote: >> How would this be different from the CFM just rejecting patches? It does not >> matter if there's an explicit number of patches that we allow to be moved to >> the next CF - someone still needs to make the decision, and I agree with Tom >> it probably should not be CFM's job. > > My experience with the query id patch is that it can't be rejected > because everyone wants it, but it needs work to get it in a state that > everyone approves of. Sometimes it is impossible for the patch author > to figure that out, and I needed Álvaro Herrera's help on the query id > patch, so even I wasn't able to figure it out alone. > Yeah, and I'm sure this applies to various other patches too - we want the feature, but it requires more work, and it may not be clear how much and what's the path forward. But it's not clear to me whether you're arguing for CFM to assess this, or whether someone else should make this decision? IMHO asking the CFM to do this would be a tremendous burden - properly assessing 50+ patches is a lot of work, and probably requires a fairly experienced hacker ... regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, Aug 3, 2021 at 09:36:41PM +0200, Tomas Vondra wrote: > On 8/3/21 8:57 PM, Bruce Momjian wrote: > > On Tue, Aug 3, 2021 at 08:51:57PM +0200, Tomas Vondra wrote: > > > How would this be different from the CFM just rejecting patches? It does not > > > matter if there's an explicit number of patches that we allow to be moved to > > > the next CF - someone still needs to make the decision, and I agree with Tom > > > it probably should not be CFM's job. > > > > My experience with the query id patch is that it can't be rejected > > because everyone wants it, but it needs work to get it in a state that > > everyone approves of. Sometimes it is impossible for the patch author > > to figure that out, and I needed Álvaro Herrera's help on the query id > > patch, so even I wasn't able to figure it out alone. > > > > Yeah, and I'm sure this applies to various other patches too - we want the > feature, but it requires more work, and it may not be clear how much and > what's the path forward. > > But it's not clear to me whether you're arguing for CFM to assess this, or > whether someone else should make this decision? > > IMHO asking the CFM to do this would be a tremendous burden - properly > assessing 50+ patches is a lot of work, and probably requires a fairly > experienced hacker ... I don't think the CFM can do this --- I think it has to be a team effort, as was the query id patch. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com If only the physical world exists, free will is an illusion.
Tomas Vondra <tomas.vondra@enterprisedb.com> writes:
> But it's not clear to me whether you're arguing for CFM to assess this,
> or whether someone else should make this decision?
> IMHO asking the CFM to do this would be a tremendous burden - properly
> assessing 50+ patches is a lot of work, and probably requires a fairly
> experienced hacker ...
Yeah. I can recall that once or twice, Andres went through and triaged
all the open patches, which was tremendously helpful and I'm sure took a
serious investment of time. That way doesn't scale. But maybe if we
split the work among half a dozen or so senior hackers, it'd be a
tolerable amount of work per-person?
regards, tom lane
On Tue, Aug 3, 2021 at 03:42:57PM -0400, Tom Lane wrote: > Tomas Vondra <tomas.vondra@enterprisedb.com> writes: > > But it's not clear to me whether you're arguing for CFM to assess this, > > or whether someone else should make this decision? > > > IMHO asking the CFM to do this would be a tremendous burden - properly > > assessing 50+ patches is a lot of work, and probably requires a fairly > > experienced hacker ... > > Yeah. I can recall that once or twice, Andres went through and triaged > all the open patches, which was tremendously helpful and I'm sure took a > serious investment of time. That way doesn't scale. But maybe if we > split the work among half a dozen or so senior hackers, it'd be a > tolerable amount of work per-person? Yes, I think we are going to need to do something like this. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com If only the physical world exists, free will is an illusion.
On Tue, 3 Aug 2021 at 11:56, Bruce Momjian <bruce@momjian.us> wrote: > I wonder if our lack of in-person developer meetings is causing some of > our issues to not get closed. That's an interesting thought. Every year there are some especially contentious patches that don't get dealt with until the in-person meeting which pushes people to make a call. However it seems like that's only a handful and not really enough to explain the volume. Unless people just find it easier to focus on reviewing in the days leading up to the conferences and meetings. Fwiw I will have some time set aside to do some patch reviewing coming up -- though I don't know how much I can really help if the patches are the problem. -- greg
On Wed, Aug 4, 2021 at 8:23 AM Greg Stark <stark@mit.edu> wrote: > On Tue, 3 Aug 2021 at 11:56, Bruce Momjian <bruce@momjian.us> wrote: > > > I wonder if our lack of in-person developer meetings is causing some of > > our issues to not get closed. > > That's an interesting thought. Every year there are some especially > contentious patches that don't get dealt with until the in-person > meeting which pushes people to make a call. However it seems like > that's only a handful and not really enough to explain the volume. I think there might be a higher number of work-in-progress patches these days, which represent ongoing collaborative efforts, and are not expected to be committed soon, but are registered to attract the attention of humans and robots. Perhaps if there were a separate status for that, it would be clearer. Or perhaps they don't belong in the "commit" fest.
I think there might be a higher number of work-in-progress patches
these days, which represent ongoing collaborative efforts, and are not
expected to be committed soon, but are registered to attract the
attention of humans and robots. Perhaps if there were a separate
status for that, it would be clearer. Or perhaps they don't belong in
the "commit" fest.
I totally support this view on CF as a better way to track and process ongoing community activity in the one place than the mailing list. The fact is that complicated patches need many CFs to be reviewed, improved and committed. I'd vote for not-so-strict approach to what should be on CF and what should not. But probably some prioritization inside CF is indeed needed.
On Tue, Aug 03, 2021 at 12:13:44PM -0400, Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > On Tue, Aug 3, 2021 at 04:53:40PM +0100, Simon Riggs wrote: > >> There are 273 patches in the queue for the Sept Commitfest already, so > >> it seems clear the queue is not being cleared down each CF as it was > >> before. We've been trying hard, but it's overflowing. > 1. There wasn't that much getting done during this CF because it's > summer and many people are on vacation (in the northern hemisphere > anyway). > I don't think there's much to be done about the vacation effect; > we just have to accept that the summer CF is likely to be less > productive than others. Early commitfests recognized a rule that patch authors owed one review per patch registered in the commitfest. If authors were holding to that, then both submissions and reviews would slow during vacations, but the neglected fraction of the commitfest would be about the same. I think it would help to track each author's balance (reviews_done - reviews_owed).
> 5 авг. 2021 г., в 09:25, Noah Misch <noah@leadboat.com> написал(а): > > On Tue, Aug 03, 2021 at 12:13:44PM -0400, Tom Lane wrote: >> Bruce Momjian <bruce@momjian.us> writes: >>> On Tue, Aug 3, 2021 at 04:53:40PM +0100, Simon Riggs wrote: >>>> There are 273 patches in the queue for the Sept Commitfest already, so >>>> it seems clear the queue is not being cleared down each CF as it was >>>> before. We've been trying hard, but it's overflowing. > >> 1. There wasn't that much getting done during this CF because it's >> summer and many people are on vacation (in the northern hemisphere >> anyway). > >> I don't think there's much to be done about the vacation effect; >> we just have to accept that the summer CF is likely to be less >> productive than others. > > Early commitfests recognized a rule that patch authors owed one review per > patch registered in the commitfest. If authors were holding to that, then > both submissions and reviews would slow during vacations, but the neglected > fraction of the commitfest would be about the same. I think it would help to > track each author's balance (reviews_done - reviews_owed). +1 for tracking this. BTW when review is done? When first revision is published? Or when patch is committed\rollbacked? When the review is owed? At the moment when patch is submitted? Or when it is committed? Best regards, Andrey Borodin.
On Tue, Aug 03, 2021 at 11:55:50AM -0400, Bruce Momjian wrote: > On Tue, Aug 3, 2021 at 04:53:40PM +0100, Simon Riggs wrote: > > There are 273 patches in the queue for the Sept Commitfest already, so > > it seems clear the queue is not being cleared down each CF as it was > > before. We've been trying hard, but it's overflowing. > > > > Of those, about 50 items have been waiting more than one year, and > > about 25 entries waiting for more than two years. > > > > One problem is that many entries don't have official reviewers, though > > many people have commented on Hackers, making it difficult to track > > down items that actually need work. That wastes a lot of time for > > would-be reviewers (or at least, it has done for me). > > > > Please can there be some additional coordination to actively clear > > down this problem? I won't attempt to come up with ideas to do this, > > but others may wish to suggest ways that avoid Committer burn-out? > > > > I will be happy to volunteer to be part of an organized approach that > > spreads out the work. > > I wonder if our lack of in-person developer meetings is causing some of > our issues to not get closed. Well, or the lack of virtual developer meetings? Sure, in-person meetings are difficult now, but OTOH virtual meetings are very common these days and maybe could be tried as quarterly developer meetings or so and have the advantage that, modulo timezone problems, every (invited) developer could attend. Michael -- Michael Banck Projektleiter / Senior Berater Tel.: +49 2166 9901-171 Fax: +49 2166 9901-100 Email: michael.banck@credativ.de credativ GmbH, HRB Mönchengladbach 12080 USt-ID-Nummer: DE204566209 Trompeterallee 108, 41189 Mönchengladbach Geschäftsführung: Dr. Michael Meskes, Sascha Heuer, Geoff Richardson, Peter Lilley Unser Umgang mit personenbezogenen Daten unterliegt folgenden Bestimmungen: https://www.credativ.de/datenschutz
On 8/5/21 11:27 AM, Michael Banck wrote: > On Tue, Aug 03, 2021 at 11:55:50AM -0400, Bruce Momjian wrote: >> On Tue, Aug 3, 2021 at 04:53:40PM +0100, Simon Riggs wrote: >>> There are 273 patches in the queue for the Sept Commitfest already, so >>> it seems clear the queue is not being cleared down each CF as it was >>> before. We've been trying hard, but it's overflowing. >>> >>> ... >> >> I wonder if our lack of in-person developer meetings is causing some of >> our issues to not get closed. > > Well, or the lack of virtual developer meetings? Sure, in-person > meetings are difficult now, but OTOH virtual meetings are very common > these days and maybe could be tried as quarterly developer meetings or > so and have the advantage that, modulo timezone problems, every > (invited) developer could attend. > I'm quite tired of virtual meetings, but I agree the lack of meetings is noticeable so maybe we should give it a try. In fact, we'll probably need to do that anyway, because even if there's an in-person meeting early next year the in-person participation is likely to be limited. regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 8/5/21 8:39 AM, Andrey Borodin wrote: >> ... >> >> Early commitfests recognized a rule that patch authors owed one review per >> patch registered in the commitfest. If authors were holding to that, then >> both submissions and reviews would slow during vacations, but the neglected >> fraction of the commitfest would be about the same. I think it would help to >> track each author's balance (reviews_done - reviews_owed). > > +1 for tracking this. Yeah, I agree we should be stricter about this rule, but I'm somewhat skeptical about tracking it in the CF app - judging patch and review complexity seems quite subjective, etc. > BTW when review is done? When first revision is published? Or when patch is committed\rollbacked? > When the review is owed? At the moment when patch is submitted? Or when it is committed? > I think the rule is roughly that when you submit a patch to a CF, you're expected to review a patch of comparable complexity in the same CF. It's not tied to whether the patch is committed, etc. regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 8/5/21 8:49 AM, Tomas Vondra wrote: > On 8/5/21 11:27 AM, Michael Banck wrote: >> On Tue, Aug 03, 2021 at 11:55:50AM -0400, Bruce Momjian wrote: >>> On Tue, Aug 3, 2021 at 04:53:40PM +0100, Simon Riggs wrote: >>>> There are 273 patches in the queue for the Sept Commitfest already, so >>>> it seems clear the queue is not being cleared down each CF as it was >>>> before. We've been trying hard, but it's overflowing. >>>> >>>> ... >>> >>> I wonder if our lack of in-person developer meetings is causing some of >>> our issues to not get closed. >> >> Well, or the lack of virtual developer meetings? Sure, in-person >> meetings are difficult now, but OTOH virtual meetings are very common >> these days and maybe could be tried as quarterly developer meetings or >> so and have the advantage that, modulo timezone problems, every >> (invited) developer could attend. >> > > I'm quite tired of virtual meetings, but I agree the lack of meetings > is noticeable so maybe we should give it a try. In fact, we'll > probably need to do that anyway, because even if there's an in-person > meeting early next year the in-person participation is likely to be > limited. > > > +1 cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
On Tue, Aug 3, 2021 at 2:52 PM Tomas Vondra <tomas.vondra@enterprisedb.com> wrote: > How would this be different from the CFM just rejecting patches? It does > not matter if there's an explicit number of patches that we allow to be > moved to the next CF - someone still needs to make the decision, and I > agree with Tom it probably should not be CFM's job. > > IMO what the CFM can do is make an assessment, share it on the mailing > list with a proposal to reject the patch, and leave the decision up to > the patch author. Either the author accepts the view it's futile, or > decides to keep working on it, solicits some reviews, etc. I don't really agree with this. Not all decisions can or should be made collaboratively. In particular, I think CfM's have been pretty timid about bouncing stuff that isn't really showing signs of progress. If a patch has been reviewed at some point in the past, and say a month has gone by, and now we're beginning or ending a CommitFest, the patch should just be closed. Otherwise the CommitFest just fills up with clutter. It's not the worst thing in the world to ask on the thread, "hey, can we RwF this?" but it risks delaying a decision that ought to have been automatic. Patches that are not being updated regularly have no business being part of a CommitFest. Also, if it's clear that an idea has quite a bit of opposition, it should be RwF'd or rejected, even if the patch author doesn't agree. Nothing whatever keeps the patch author from continuing to argue about it, or indeed resubmitting the patch. But we don't get any value out of having a list of things that somebody should take a look at which includes things that have been looked at already and judged by multiple people to be not acceptable. I don't think a patch should be rejected on the strength of a single -1, but when 2 or 3 people have shown up to say either that they don't like the idea or that the implementation is way off base, it's not helpful to leave things in a state that suggests it needs more eyeballs. Arguing with people about the merits of a patch that has a 0% probability of being committed by anybody is one of my least favorite things. I have only so much time, and I want to invest the time I have in things that have a chance of getting someplace. I don't mind the time spent telling somebody whose patch is bad why it has no hope - and I think that's an important thing for us to do. But if somebody thinks, say, that full page writes are not needed and we should just remove them, I only want to explain why they are wrong. I don't want to argue about whether they are actually right. There are plenty of patches where somebody has ideas that may or may not be good and we should talk about it - but there are also some where talking isn't going to help, and those can easily consume massive amounts of time and energy. -- Robert Haas EDB: http://www.enterprisedb.com
On Thu, Aug 05, 2021 at 03:06:54PM +0200, Tomas Vondra wrote: > On 8/5/21 8:39 AM, Andrey Borodin wrote: > >>... > >> > >>Early commitfests recognized a rule that patch authors owed one review per > >>patch registered in the commitfest. If authors were holding to that, then > >>both submissions and reviews would slow during vacations, but the neglected > >>fraction of the commitfest would be about the same. I think it would help to > >>track each author's balance (reviews_done - reviews_owed). > > > >+1 for tracking this. > > Yeah, I agree we should be stricter about this rule, but I'm somewhat > skeptical about tracking it in the CF app - judging patch and review > complexity seems quite subjective, etc. The CF app presently lacks the data to track this, but with relevant feature additions, I suspect it would be the best place to manage tracking. Something like this: when a CF app user changes a patch status from Needs Review or Ready for Committer to anything else, invite that user to name a recipient of review credit. > >BTW when review is done? When first revision is published? Or when patch is committed\rollbacked? > >When the review is owed? At the moment when patch is submitted? Or when it is committed? Those are important questions. Here are answers that feel reasonable to me, but they may need refinement. Anyone can increase their reviews_done by sending a review that causes a patch to leave Needs Review status in the CF app for the first time in a given commitfest. Trivial defect reports, e.g. that the patch doesn't compile, are not reviews; the person setting Waiting on Author shall not assign review credit. Committers can increase their reviews_done by sending a review that causes a patch to leave Ready for Committer status for the first time in a given commitfest; however, nobody receives two credits for the same patch, in the same commitfest. Whenever a CF entry is increasing someone's reviews_done, it also increases reviews_owed for the entry's author. What do you think? (Consequences: each CF entry can increment reviews_done of up to two users per commitfest, but it increments reviews_owed just once. If the patch bounces between Needs Review and Waiting on Author several times in a single CF, that counts as a total of one review.) > I think the rule is roughly that when you submit a patch to a CF, you're > expected to review a patch of comparable complexity in the same CF. It's not > tied to whether the patch is committed, etc. Yep. Would we get reasonable incentives without tracking "comparable complexity" explicitly? That bit is harder to judge.
> On Tue, Aug 03, 2021 at 02:57:49PM -0400, Bruce Momjian wrote: > > On Tue, Aug 3, 2021 at 08:51:57PM +0200, Tomas Vondra wrote: > > How would this be different from the CFM just rejecting patches? It does not > > matter if there's an explicit number of patches that we allow to be moved to > > the next CF - someone still needs to make the decision, and I agree with Tom > > it probably should not be CFM's job. > > My experience with the query id patch is that it can't be rejected > because everyone wants it, but it needs work to get it in a state that > everyone approves of. Sometimes it is impossible for the patch author > to figure that out, and I needed Álvaro Herrera's help on the query id > patch, so even I wasn't able to figure it out alone. I know I'm late to the party, but I couldn't stop thinking about this thread. Since there seems to be no specific changes agreed upon in here, I wanted to add my 5c. It's indeed hard at times to use CF app efficiently due to cluttering of different sorts: * Patches slipping through CFs "waiting on author". Hopefully CFM can identify such cases without much efforts. * Patches slipping through CFs on "needs review", while, as you mention, sometimes it is hard for the author to figure something out and an attention of more knowledgeable hacker is needed. Most of the time this status means "needs a design review" (which should be probably even introduced as a real status), and such review is indeed a scarce resource in the community. Due to that a certain level of cluttering is unavoidable in the long term I believe, no matter how many hackers will go triaging things. * Bloat in the official reviewers field, which was mentioned at the start. I believe there were suggestions in the past to address this via resetting the field automatically on the start of every CF. As a side note, maybe one could go even further in resetting / automation and put the responsibility for moving patches between CF fully on shoulders of the authors.
On Tue, Aug 3, 2021 at 8:53 AM Simon Riggs <simon.riggs@enterprisedb.com> wrote:
There are 273 patches in the queue for the Sept Commitfest already, so
it seems clear the queue is not being cleared down each CF as it was
before. We've been trying hard, but it's overflowing.
I think one simple change here would be of some help. Every patch should come on to the commitfest in "Proposed" Status. Someone besides the author of the patch then needs to move it from "Proposed" to "Needs Review". That person does not have to be a reviewer or committer and is not obligating themselves to working on the patch (though likely they would provide input). They are simply, in the language of Robert's Rules of Order, seconding the motion.
Removal from the CF could be automatic after the 3rd CF, though the CF managers involved should, before returning it as "No Interest at this Time", skim the thread and ask whether any commenters on the thread would like to see it kept alive in the commitfest by seconding it. If so removed, the author should be encouraged to re-submit during the next version's development cycle.
David J.
On Tue, Aug 3, 2021 at 8:53 AM Simon Riggs <simon.riggs@enterprisedb.com> wrote:
There are 273 patches in the queue for the Sept Commitfest already, so
it seems clear the queue is not being cleared down each CF as it was
before. We've been trying hard, but it's overflowing.
Of those, about 50 items have been waiting more than one year, and
about 25 entries waiting for more than two years.
The "Last Modified" field should probably omitted. The fact that "Moved to Next CF" changes this date is unhelpful. I'd much rather replace it with a "last patch provided" date, or, better, an "days since" calculation instead. Having the system record the email addresses that replied to the thread instead of strictly looking at people who interact with the commitfest application would probably also be useful information for commitfest people looking to see where limited attention would be best spent.
David J.
On Thu, Aug 5, 2021 at 7:36 AM Robert Haas <robertmhaas@gmail.com> wrote:
Patches that are not being updated regularly have no
business being part of a CommitFest.
As the main issue seems to be "Needs Review" getting punted, the patch author rightly expects feedback before supplying new patches. If they are waiting for months, that isn't on them.
I don't think a patch should be
rejected on the strength of a single -1, but when 2 or 3 people have
shown up to say either that they don't like the idea or that the
implementation is way off base, it's not helpful to leave things in a
state that suggests it needs more eyeballs.
I would agree. One of those 3 people should then state in the thread where they put their -1 that they are, in addition, going to mark the patch as RwF or Rejected in the CF, and then go do just that. If that isn't done, and the CfM comes across this situation, they should probably request the person with the first -1 to make the change instead of doing it for them. And it doesn't have to be explicit -1s to qualify.
David J.