PostgreSQL's commitfest clog

While it may seem like the number of developers would be the limiting factor in a free-software project, the truth of the matter is that, for all but the smallest of project, the scarcest resource is reviewer time. Lots of people like to crank out code; rather fewer can find the time to take a close look at somebody else's patches. Free-software projects have taken a number of different approaches to address the review problem; the PostgreSQL developer community is currently struggling with its review load and considering changes to its commitfest process in response.

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.

---

to post comments