PostgreSQL's commitfest clog
Part of the review problem is clerical in nature: patches must be tracked along with their review status. Some projects, like the Linux kernel, take a distributed approach; review status is tracked in the patches themselves and subsystem maintainers are expected to keep up with which patches are ready to be merged. PostgreSQL developers, naturally, prefer to keep that information in a central database. Roughly every other month, outstanding patches are gathered for a month-long commitfest, during which the project makes a decision on the fate of each one of them. Each commitfest has a designated manager who is responsible for ensuring that all patches have been dealt with by the end of the commitfest.
That is the intended result, anyway.
What actually happens, as Simon Riggs recently pointed
out on the PostgreSQL Hackers mailing list, is that a lot of patches
languish in the queue with no firm decision being made; this
can happen as the result of a lack of reviews or a failure of the author to
respond, among other reasons. Riggs noted that the
2021-09 commitfest,
which is scheduled for September, has
273 patches queued (since increased to 279): "Of those, about 50
items have been waiting more than one year, and about 25 entries waiting
for more than two years
". The community has been working hard to
clear the queue during each commitfest, Riggs said, but still
"it's overflowing
".
A look at past commitfests (which can be viewed at commitfest.postgresql.org) shows that a great many patches are "dealt with" by deferring them to the next commitfest. The recently concluded 2021-07 commitfest considered 342 patches; of those, 233 (just over 2/3) were deferred to the next commitfest. When the punting rate is that high, actually clearing out the commitfest queue becomes a distant prospect at best.
There is a longstanding expectation within the PostgreSQL project that anybody submitting a patch for consideration in a given commitfest should take the time to review somebody else's patch, preferably one of similar complexity. In theory, that would balance the numbers of submitters and reviewers; in practice it does not seem to be getting the job done. Part of the problem, almost certainly, is that some submitters just never quite get around to fulfilling that side of the bargain; life is busy after all. One of the commitfest manager's jobs is to encourage developers to do reviews; that is, needless to say, a task that is even less fun than patch review. In the discussion, Noah Misch suggested an obvious technical solution: track each submitter's review balance in the database to make it clear who is not living up to expectations. But, as Tomas Vondra pointed out, there are a lot of subjective questions about what constitutes a review, equivalent complexity, and more.
Even if the rule were fully observed, though, it seems unlikely that the problem could go away. Many patches need input from multiple reviewers; they may also go through the review process many times with changes in response to feedback in between. Thus, the number of needed reviews is sure to exceed the number of submitted patches by a significant margin.
Bruce Momjian suggested
that part of the problem, in the last year at least, is the complete lack
of in-person developer meetings. Greg Stark agreed: "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
". He also noted, though,
that the number of patches in this category isn't sufficient to explain
the size of the backlog. Michael Banck suggested
holding virtual meetings to make decisions on patches; there are few
developers out there who are clamoring for more online meetings, but enough
might be convinced to attend one to make some progress possible.
One thing that almost everybody seemed to agree on is that many of the patches that slide from one commitfest to another simply should not be there. According to Tom Lane:
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.
Robert Haas expressed a similar point of view:
I think [commitfest managers] 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.
Improving the project's ability to close dead-end patches might not be
easy.
Arguably, it is up to the commitfest manager to make that decision; Riggs
suggested
that the manager only be allowed to defer consideration on 50% of the
submitted patches. But Lane (in the message quoted above) noted that only
managers who are "assertive enough and senior enough
" have
been able to "kill off patches that didn't look like they were going
to go anywhere
" and said that perhaps this should not be the
commitfest manager's job in any case. Certainly disappointing dozens of
submitters over the course of a month — and, undoubtedly, hearing their
thoughts on the matter — is not going to make the job of commitfest manager
more appealing.
One other idea that came up a few times was to place a limit on the number of commitfests that a patch could be allowed to slide through before being removed. This approach has the advantage of being relatively automatic and objective; nobody would have to step up as the evil maintainer who decided to reject a whole pile of patches. It would also bring a natural end to patches that nobody can find a way to care about.
The conversation wound down without reaching any solid conclusions.
Perhaps this issue, too, will be deferred to the next commitfest for a
decision. But the discussion may at least motivate some developers to
put more time into cleaning out the queue and reducing the backlog — in the
short term, at least. In the longer term, the PostgreSQL will have to
continue with a shortage of reviewers, just like many other projects.
