Rethinking the PostgreSQL CommitFest model

Many years ago, the PostgreSQL project started holding regular CommitFests to help tackle the work of reviewing and committing patches in a more organized fashion. That has served the project well, but some in the project are concerned that CommitFests are no longer meeting the needs of PostgreSQL or its contributors. A lengthy discussion on the pgsql-hackers mailing list turned up a number of complaints, a few suggestions for improvement, but little consensus or momentum toward a solution.

The CommitFest concept got its start in 2008, after a planned six-month development cycle took more than a year to complete. Dave Page detailed the plan in early 2008 to the mailing list, noting that it had been discussed previously and had received general approval. It would replace the traditional feature freeze with "a series of 'commit fests' throughout the cycle".

Whenever a commit fest is in progress, the focus will shift from development to review, feedback and commit of patches. Each fest will continue until all patches in the queue have either been committed to the CVS repository, returned to the author for additional work, or rejected outright, and until that has happened, no new patches will be considered.

Since then, the project has adopted the name CommitFest for the process of prioritizing review, feedback, and committing, and a web-based tracking tool (CommitFest application or CFA) written in Django with a (naturally) PostgreSQL database backend. CommitFests are held several times a year, usually odd-numbered months (currently five times a year since 2018) and are managed by a CommitFest Manager.

The concept of not considering new patches until all of the CommitFest patches have been dispensed with has not survived the test of time. Patches do pile up, and are automatically moved to the next CommitFest if not committed, rejected, or otherwise removed. The first CommitFest using the CFA was held in December 2014 through January 2015. It saw 79 submissions: 18 were committed, 21 were rejected, and 40 moved to the next CommitFest. Since that time, the possible states of a patch submission have been expanded to include "waiting on author", "ready for committer", "returned with feedback", and "withdrawn". The March 2024 CommitFest had a total of 331 submissions, with 132 committed, 7 returned with feedback, 13 withdrawn, and only 6 rejected. The majority of patches, 173, were moved to the next CommitFest. The upcoming CommitFest currently has 308 submissions.

No longer fit for purpose

On May 16, Robert Haas wrote that he believed CommitFests were no longer serving their purpose:

The original intent of CommitFests, and of commitfest.postgresql.org by extension, was to provide a place where patches could be registered to indicate that they needed to be reviewed, thus enabling patch authors and patch reviewers to find each other in a reasonably efficient way. I don't think it's working any more.

Haas complained that few patches submitted for CommitFests were actually ready for review. They were, he elaborated, in a variety of states that were unsuitable for review including patches "we've said we don't want but the author thinks we do" and "patches that have long-unresolved difficulties which the author doesn't know how to solve or is in no hurry to solve".

The chances that reviewers would find things to review that were the most in need of their attention "are pretty much nil". The CommitFest application had turned into a patch tracker, he said, and patch trackers "intrinsically tend to suck" because they are full of garbage nobody cares about and nobody wants to do the work to maintain them. "But our patch tracker sucks MORE, because it's not even intended to BE a general-purpose patch tracker."

There were three factors that have led to this state of affairs, he said. The first being that "we got tired of making people mad" by rejecting patches submitted to CommitFests even if they had no chance of being committed this time, or ever. The second problem, he said, was the addition of continuous integration (CI), which adds value for developers who register patches for a CommitFest even when they are unlikely to be committed. All of that culminated in the third problem, which is that the list "is now so long and messy that even the Commitfest manager probably [doesn't] manage to go through the whole thing thoroughly in a month".

He acknowledged that there was no easy answer, because keeping a large group of people organized is hard enough—especially when "nobody's really the boss"—but he wondered what ideas others might have to improve the situation. "I also feel like what we're doing right now can't possibly be the best that we can do".

Discussion

David G. Johnston suggested reusing the same CFA software but creating a new instance called "Collaboration" as a patch tracker, or just to add a new slot to the existing CFA called work in process (WIP). Then work could be pulled into a future CommitFest when ready. It would cause less angst, he noted, to move work to a WIP status than dropping it outright. The project would still have a problem with "too many WIP patches and not enough ability to resolve them" but putting a higher bar to get into a CommitFest while having a place to collaborate would be a step forward.

Even though patch trackers "tend to suck", Jacob Champion said, the project should just "embrace the need for a tracker and make it more helpful". Magnus Hagander proposed a similar idea, to have a parking lot for inactive patches that aren't ready for a CommitFest but "you also don't want to be dead". Tom Lane liked the idea of a "not-the-CF" application with CI runs and said that there appeared to be consensus around the parking lot idea. However, that was not the only problem and he urged people to keep thinking about what could be improved.

Daniel Gustafsson wanted clarification on what was broken about CommitFests. Was it the application, the process and workflow, or the assumption that the process and workflow were followed "which may or may not be backed by evidence"? His experience as a CommitFest Manager led him to believe that the problem was that the process and workflow were not being followed, which would be hard to fix just by applying better software.

"Some of us are abusing the process a bit", wrote Lane. He admitted that he was guilty of sticking patches into the CommitFest application, "because it's a near-zero-friction way to run CI on them, and I'm too lazy to learn how to do that otherwise". Those patches, however, would spend little time in the application and were not really the problem. The stuff that hangs around seems to be the problem, he said—and that is caused by a fear of rejecting patches, which causes people to punt them to the next CommitFest.

Greg Stark disagreed with the idea that lack of rejection was the problem. He said he had tried to find patches with negative reviews to reject, but did not find many of that type. Instead, he found patches with generally positive reviews, but complex situations, and patches that weren't getting any reviews at all.

Joe Conway asked if the project should have a policy that no patches are automatically moved forward from one CommitFest to another. Therefore "the author needs to care enough to register for the next one". Champion did not like that idea. He said that it would disadvantage those who do not work on PostgreSQL as their day job; and "not having time to shepherd a patch is not the same as not caring". Conway said that the word "care" was a poor choice, but still wanted to force authors to consider whether they had time to shepherd the patch for the next round.

Peter Eisentraut wrote that he thought having to resubmit would be effective, but it would double the annoyance for contributors whose patches were not being reviewed in the first place. Tomas Vondra agreed, and said that having to resubmit patches "just to have a chance someone might take a look" would make him question whether he wanted to submit patches at all.

Eisentraut proposed a new state for patches called "Unclear". The CommitFest Manager could set any patch that was doubtful to "Unclear", so reviewers could focus on patches that definitely need review. "I think, if we consider the core mission of the commitfest app, we need to be more protective of the Needs Review state." Lane agreed that would make sense, and Gustafsson wrote that being protective of the Needs Review state was a good summary of what the project needed to focus on in improving CommitFests.

Great ideas, but...

The discussion produced many insights into why CommitFests are less effective, and a number of interesting solutions. The question is, what's next? Dmitry Dolgov observed that the thread had produced many good takes, but he was concerned about next steps. If memory serves, he said, similar discussions crop up every couple of years but "not many things have changed due to those discussions". How could the project make sure this time was different, he asked.

Lane argued that "if you take the long view" things have changed for the better. Part of the point of CommitFests, he said, was to help people learn to become good committers. Lane noted that PostgreSQL had around ten active committers through 2012, but it had crept up to about two dozen today. "My point here is not that things are great, but just that we are indeed improving, and I hope we can continue to. Let's not be defeatist about it." The fundamental problem has not changed, though: "too many patch submissions, too few committers". Lane suggested a divide-and-conquer strategy, having senior committers split up the list of patches and see what it would take to move the patches along. That might not be possible before each CommitFest, he said, but the project could try to make that happen once per year.

Haas liked the idea of a yearly review, but countered that the project had not significantly improved the CommitFests for many years. He said it was not possible, or necessary, to solve all the problems overnight, but the project needed to admit there is a problem that is "an existential threat to the project". He was surprised that the number of committers had grown substantially, but doubted that the new-patch-submitter experience had improved. "On the plus side, I think we make more of an effort not to be a jerk to newcomers than we used to."

Vondra asked when the yearly review might take place. He pointed out that the project used to do a "triage" at the FOSDEM PGDay meeting, but it was only an informal effort with a small subset of people. And it would be somewhat late in the development cycle. Lane said that he pictured a few days to a week, but the question would be finding a week when people are available. Conway proposed doing a review at PGConf.eu. Lane said that might work, but pointed out that the throughput of a group of reviewers working together in a meeting was less than the same reviewers working independently. However, "the consensus of a meeting is more likely to be taken seriously than a single person's opinion, senior or not" which made the idea attractive. The thread trailed off with Lane agreeing it would be a good idea to do the work in subgroups of three or four people.

What's next?

As of this writing, nothing concrete has emerged to "fix" the process or application and the discussion has largely dried up. There seems to be general consensus around the problems with CommitFests and some agreement on how to solve them. The question is whether anyone takes the next step toward implementing solutions.