Thread: [HACKERS] Commit fest 2017-11
Hi all, At the moment of writing this email, it is 9PM AoE (Anywhere on Earth) 31st of October. This means that the next commit fest will begin in 3 hours, and that any hackers willing to register patches for this commit fest have roughly three hours to do so (plus/minus N hours). This current score is the following: Needs review: 125. Waiting on Author: 22. Ready for Committer: 39. This represents a total of 186 patches still pending for review, for the second largest commit fest ever. Anybody willing to take the hat of the commit fest manager? If nobody, I am fine to take the hat as default choice this time. Thanks, -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Nov 1, 2017 at 9:04 AM, Michael Paquier <michael.paquier@gmail.com> wrote: > Anybody willing to take the hat of the commit fest manager? If nobody, > I am fine to take the hat as default choice this time. And now it is open. Let's the fest begin. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Nov 1, 2017 at 11:30 PM, Michael Paquier <michael.paquier@gmail.com> wrote: > On Wed, Nov 1, 2017 at 9:04 AM, Michael Paquier > <michael.paquier@gmail.com> wrote: >> Anybody willing to take the hat of the commit fest manager? If nobody, >> I am fine to take the hat as default choice this time. > > And now it is open. Let's the fest begin. The current commit fest is coming to an end, and as many may have noticed, I have begun classifying patches depending on their status. This will likely take a couple of days. As a last push, I would like to point out that there are 22 patches marked as ready for committer: https://commitfest.postgresql.org/15/?status=3. -- Michael
On Tue, Nov 28, 2017 at 11:28 AM, Michael Paquier <michael.paquier@gmail.com> wrote: > The current commit fest is coming to an end, and as many may have > noticed, I have begun classifying patches depending on their status. > This will likely take a couple of days. As a last push, I would like > to point out that there are 22 patches marked as ready for committer: > https://commitfest.postgresql.org/15/?status=3. All patches not marked as ready for committer have been classified, by either being marked as returned with feedback or moved to the next CF. I may have made some mistakes of course, hence if you feel that the status of your patch is not appropriate, feel free to update it as you think is best-suited. I would like to point out that the status of each patch on the CF app was rather representative to the status on their respective thread (only a couple had incorrect statuses, at least I thought so), and that's a nice improvement. One thing that could be improved in my opinion is that patch authors should try more to move a patch to a following commit fest once the end gets close...This would leverage slightly the load of work for the CFM. Remains 22 patches as of now, exactly *one* for each committer. Thanks Fabien for pointing that out to me :) -- Michael
From: Michael Paquier [mailto:michael.paquier@gmail.com] > All patches not marked as ready for committer have been classified, by either > being marked as returned with feedback or moved to the next CF. > I may have made some mistakes of course, hence if you feel that the status > of your patch is not appropriate, feel free to update it as you think is > best-suited. Thanks a lot for your tough work. This should have been much harder than I can imagine... > improvement. One thing that could be improved in my opinion is that patch > authors should try more to move a patch to a following commit fest once > the end gets close...This would leverage slightly the load of work for the > CFM. I'm sorry about this. I should have moved "Statement-level rollback" to the next CF. Now I tried that, successfully markingit as "waiting on author", but the patch doesn't move to the next CF when I then change the status as "Move to nextCF." How can I move the patch to next CF? Regards Takayuki Tsunakawa
On 2017/11/30 14:29, Tsunakawa, Takayuki wrote: > From: Michael Paquier [mailto:michael.paquier@gmail.com] >> All patches not marked as ready for committer have been classified, by either >> being marked as returned with feedback or moved to the next CF. >> I may have made some mistakes of course, hence if you feel that the status >> of your patch is not appropriate, feel free to update it as you think is >> best-suited. > > Thanks a lot for your tough work. This should have been much harder than I can imagine... +1. Thank you, Michael! Regards, Amit
On Thu, Nov 30, 2017 at 2:29 PM, Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com> wrote: > Now I tried that, successfully marking it as "waiting on author", but the patch doesn't move to the next CF when I thenchange the status as "Move to next CF." How can I move the patch to next CF? If you have a patch "waiting on author" that you would like to move to the next commit fest, just switch its status back temporarily to "needs review", and then do the move. Yes, that's unnecessary complication but I am not going to fight against the design of the CF app. -- Michael
From: Michael Paquier [mailto:michael.paquier@gmail.com] > If you have a patch "waiting on author" that you would like to move to the > next commit fest, just switch its status back temporarily to "needs review", > and then do the move. Yes, that's unnecessary complication but I am not > going to fight against the design of the CF app. I could do it successfully! Thank you. Regards Takayuki Tsunakawa
Michael, thank you for your hard work! > 30 нояб. 2017 г., в 10:39, Michael Paquier <michael.paquier@gmail.com> написал(а): > > On Thu, Nov 30, 2017 at 2:29 PM, Tsunakawa, Takayuki > <tsunakawa.takay@jp.fujitsu.com> wrote: >> Now I tried that, successfully marking it as "waiting on author", but the patch doesn't move to the next CF when I thenchange the status as "Move to next CF." How can I move the patch to next CF? > > If you have a patch "waiting on author" that you would like to move to > the next commit fest, just switch its status back temporarily to > "needs review", and then do the move. Yes, that's unnecessary > complication but I am not going to fight against the design of the CF > app. I want to move also "Covering B-tree indexes (aka INCLUDE)" . Seems like we have common view with Peter Geoghegan and Anastasiathat found drawback will be fixed before next CF. If there is no objections, I'll put "needs review" to move. Best regards, Andrey Borodin.
On Thu, Nov 30, 2017 at 2:53 PM, Andrey Borodin <x4mmm@yandex-team.ru> wrote: > I want to move also "Covering B-tree indexes (aka INCLUDE)" . Seems like we have common view with Peter Geoghegan and Anastasiathat found drawback will be fixed before next CF. > > If there is no objections, I'll put "needs review" to move. Of course, feel free. -- Michael
On 11/30/2017 05:51 AM, Michael Paquier wrote:> One thing that could be improved in my > opinion is that patch authors should try more to move a patch to a > following commit fest once the end gets close...This would leverage > slightly the load of work for the CFM. Thanks for the suggestion. I had no idea that I could do that. Andreas
And the last 21 patches have been classified as well. Here is the
final score for this time:
Committed: 55.
Moved to next CF: 103.
Rejected: 1.
Returned with Feedback: 47.
Total: 206.
Thanks to all the contributors for this session! The CF is now closed.
Thanks for the CF management.
Attached a small graph of the end status of patch at the end of each CF.
Thank you for the graph!
It would be interesting to see statistics not by patches count, but by their complexity.
For rough measure of complexity we can use number of affected lines. I expect that
statistics would be even more distressing: small patches can be committed faster,
while large patches are traversing from one CF to another during long time. Interesting
to check whether it's true...
------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
On Thu, Mar 29, 2018 at 10:06 AM, Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:
Hi, Fabien!On Fri, Dec 1, 2017 at 9:10 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:And the last 21 patches have been classified as well. Here is the
final score for this time:
Committed: 55.
Moved to next CF: 103.
Rejected: 1.
Returned with Feedback: 47.
Total: 206.
Thanks to all the contributors for this session! The CF is now closed.
Thanks for the CF management.
Attached a small graph of the end status of patch at the end of each CF.Thank you for the graph!It would be interesting to see statistics not by patches count, but by their complexity.For rough measure of complexity we can use number of affected lines. I expect thatstatistics would be even more distressing: small patches can be committed faster,while large patches are traversing from one CF to another during long time. Interestingto check whether it's true...
I think that's very hard to do given that we simply don't have the data today. It's not that simple to analyze the patches in the archives, because some are single file, some are spread across multiple etc. I fear that anything trying to work off that would actually make the stats even more inaccurate. This is the pattern I've seen whenever I've treid tha tbefore.
I wonder if we should consider adding a field to the CF app *specifically* to track things like this. What I'm thinking is a field that's set (or at least verified) by the person who flags a patch as committed with choices like Trivial/Simple/Medium/Complex (trivial being things like typo fixes etc, which today can hugely skew the stats).
Would people who update the CF app be willing to put in that effort (however small it is, it has to be done consistently to be of any value) in order to be able to track such statistics?
It would only help for the future of course, unless somebody wants to go back and backfill existing patches with such information (which they might be).
On Thu, Mar 29, 2018 at 11:19 AM, Magnus Hagander <magnus@hagander.net> wrote:
On Thu, Mar 29, 2018 at 10:06 AM, Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:Hi, Fabien!On Fri, Dec 1, 2017 at 9:10 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:And the last 21 patches have been classified as well. Here is the
final score for this time:
Committed: 55.
Moved to next CF: 103.
Rejected: 1.
Returned with Feedback: 47.
Total: 206.
Thanks to all the contributors for this session! The CF is now closed.
Thanks for the CF management.
Attached a small graph of the end status of patch at the end of each CF.Thank you for the graph!It would be interesting to see statistics not by patches count, but by their complexity.For rough measure of complexity we can use number of affected lines. I expect thatstatistics would be even more distressing: small patches can be committed faster,while large patches are traversing from one CF to another during long time. Interestingto check whether it's true...I think that's very hard to do given that we simply don't have the data today. It's not that simple to analyze the patches in the archives, because some are single file, some are spread across multiple etc. I fear that anything trying to work off that would actually make the stats even more inaccurate. This is the pattern I've seen whenever I've treid tha tbefore.I wonder if we should consider adding a field to the CF app *specifically* to track things like this. What I'm thinking is a field that's set (or at least verified) by the person who flags a patch as committed with choices like Trivial/Simple/Medium/Complex (trivial being things like typo fixes etc, which today can hugely skew the stats).Would people who update the CF app be willing to put in that effort (however small it is, it has to be done consistently to be of any value) in order to be able to track such statistics?It would only help for the future of course, unless somebody wants to go back and backfill existing patches with such information (which they might be).
We have http://commitfest.cputube.org/ which automatically applies patches and runs
regression tests. This tool is probably not handle all the cases. In particular
it doesn't work when patchset in one thread is depending on patchset in another
thread. But it would be definitely enough for statistics.
I also think we should eventually integrate functionality of http://commitfest.cputube.org/
into our commitfest application..
------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
On 29 March 2018 at 09:19, Magnus Hagander <magnus@hagander.net> wrote: > > > On Thu, Mar 29, 2018 at 10:06 AM, Alexander Korotkov > <a.korotkov@postgrespro.ru> wrote: >> >> Hi, Fabien! >> >> On Fri, Dec 1, 2017 at 9:10 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote: >>>> >>>> And the last 21 patches have been classified as well. Here is the >>>> final score for this time: >>>> Committed: 55. >>>> Moved to next CF: 103. >>>> Rejected: 1. >>>> Returned with Feedback: 47. >>>> Total: 206. >>>> >>>> Thanks to all the contributors for this session! The CF is now closed. >>> >>> >>> Thanks for the CF management. >>> >>> Attached a small graph of the end status of patch at the end of each CF. >> >> >> Thank you for the graph! >> It would be interesting to see statistics not by patches count, but by >> their complexity. >> For rough measure of complexity we can use number of affected lines. I >> expect that >> statistics would be even more distressing: small patches can be committed >> faster, >> while large patches are traversing from one CF to another during long >> time. Interesting >> to check whether it's true... >> > > I think that's very hard to do given that we simply don't have the data > today. It's not that simple to analyze the patches in the archives, because > some are single file, some are spread across multiple etc. I fear that > anything trying to work off that would actually make the stats even more > inaccurate. This is the pattern I've seen whenever I've treid tha tbefore. > > I wonder if we should consider adding a field to the CF app *specifically* > to track things like this. What I'm thinking is a field that's set (or at > least verified) by the person who flags a patch as committed with choices > like Trivial/Simple/Medium/Complex (trivial being things like typo fixes > etc, which today can hugely skew the stats). > > Would people who update the CF app be willing to put in that effort (however > small it is, it has to be done consistently to be of any value) in order to > be able to track such statistics? > > It would only help for the future of course, unless somebody wants to go > back and backfill existing patches with such information (which they might > be). The focus of this is on the Committers, which seems wrong. I suggest someone does another analysis that shows how many patch reviews have been conducted by patch authors, so we can highlight people who are causing the problem yet not helping solve the problem. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, Mar 29, 2018 at 09:38:28AM +0100, Simon Riggs wrote: > I suggest someone does another analysis that shows how many patch > reviews have been conducted by patch authors, so we can highlight > people who are causing the problem yet not helping solve the problem. This data is already partially available. Just go to the bottom of the CF app, then click on "Reports" -> "Author Stats". This does not give a trend of the patch difficulty though. -- Michael
Attachment
On Thu, Mar 29, 2018 at 10:37 AM, Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:
On Thu, Mar 29, 2018 at 11:19 AM, Magnus Hagander <magnus@hagander.net> wrote:On Thu, Mar 29, 2018 at 10:06 AM, Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:Hi, Fabien!On Fri, Dec 1, 2017 at 9:10 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:And the last 21 patches have been classified as well. Here is the
final score for this time:
Committed: 55.
Moved to next CF: 103.
Rejected: 1.
Returned with Feedback: 47.
Total: 206.
Thanks to all the contributors for this session! The CF is now closed.
Thanks for the CF management.
Attached a small graph of the end status of patch at the end of each CF.Thank you for the graph!It would be interesting to see statistics not by patches count, but by their complexity.For rough measure of complexity we can use number of affected lines. I expect thatstatistics would be even more distressing: small patches can be committed faster,while large patches are traversing from one CF to another during long time. Interestingto check whether it's true...I think that's very hard to do given that we simply don't have the data today. It's not that simple to analyze the patches in the archives, because some are single file, some are spread across multiple etc. I fear that anything trying to work off that would actually make the stats even more inaccurate. This is the pattern I've seen whenever I've treid tha tbefore.I wonder if we should consider adding a field to the CF app *specifically* to track things like this. What I'm thinking is a field that's set (or at least verified) by the person who flags a patch as committed with choices like Trivial/Simple/Medium/Complex (trivial being things like typo fixes etc, which today can hugely skew the stats).Would people who update the CF app be willing to put in that effort (however small it is, it has to be done consistently to be of any value) in order to be able to track such statistics?It would only help for the future of course, unless somebody wants to go back and backfill existing patches with such information (which they might be).We have http://commitfest.cputube.org/ which automatically applies patches and runsregression tests. This tool is probably not handle all the cases. In particularit doesn't work when patchset in one thread is depending on patchset in anotherthread. But it would be definitely enough for statistics.I also think we should eventually integrate functionality of http://commitfest.cputube.org/ into our commitfest application..
Yes, there is (stalled, but still) work in progress on that one. I think that's a separate thing though.
On Thu, Mar 29, 2018 at 10:38 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
-- The focus of this is on the Committers, which seems wrong.On 29 March 2018 at 09:19, Magnus Hagander <magnus@hagander.net> wrote:
>
>
> On Thu, Mar 29, 2018 at 10:06 AM, Alexander Korotkov
> <a.korotkov@postgrespro.ru> wrote:
>>
>> Hi, Fabien!
>>
>> On Fri, Dec 1, 2017 at 9:10 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
>>>>
>>>> And the last 21 patches have been classified as well. Here is the
>>>> final score for this time:
>>>> Committed: 55.
>>>> Moved to next CF: 103.
>>>> Rejected: 1.
>>>> Returned with Feedback: 47.
>>>> Total: 206.
>>>>
>>>> Thanks to all the contributors for this session! The CF is now closed.
>>>
>>>
>>> Thanks for the CF management.
>>>
>>> Attached a small graph of the end status of patch at the end of each CF.
>>
>>
>> Thank you for the graph!
>> It would be interesting to see statistics not by patches count, but by
>> their complexity.
>> For rough measure of complexity we can use number of affected lines. I
>> expect that
>> statistics would be even more distressing: small patches can be committed
>> faster,
>> while large patches are traversing from one CF to another during long
>> time. Interesting
>> to check whether it's true...
>>
>
> I think that's very hard to do given that we simply don't have the data
> today. It's not that simple to analyze the patches in the archives, because
> some are single file, some are spread across multiple etc. I fear that
> anything trying to work off that would actually make the stats even more
> inaccurate. This is the pattern I've seen whenever I've treid tha tbefore.
>
> I wonder if we should consider adding a field to the CF app *specifically*
> to track things like this. What I'm thinking is a field that's set (or at
> least verified) by the person who flags a patch as committed with choices
> like Trivial/Simple/Medium/Complex (trivial being things like typo fixes
> etc, which today can hugely skew the stats).
>
> Would people who update the CF app be willing to put in that effort (however
> small it is, it has to be done consistently to be of any value) in order to
> be able to track such statistics?
>
> It would only help for the future of course, unless somebody wants to go
> back and backfill existing patches with such information (which they might
> be).
I suggest someone does another analysis that shows how many patch
reviews have been conducted by patch authors, so we can highlight
people who are causing the problem yet not helping solve the problem.
I have exactly such analysis available. The problem with it is that it cannot take complexity into account.
Reviewing a one-letter typo patch is not the same as reviewing MERGE, JIT or parallel query... But right now, we don't have enough proper data to differentiate those.
The idea would not to put the focus on the committer necessarily, but on the person who marks the patch as committed in the CF app. We can of course have the submitter also input this as a metadata, but in the end it's going to be the reviewers and committers who are the only ones who can judge what the *actual* complexity of a patch is/was.
Hello I can not find "Reports" in bottom any page of CF app... Such stats covers only reviews marked in CF app? Through "Comment"->"Review" buttons? I'm afraid this statistics will beinaccurate for new users (like me). Wiki page https://wiki.postgresql.org/wiki/Reviewing_a_Patch say > Send reviews as replies to the email containing the patch And nothing about changes in CF app. regards, Sergei
On Thu, Mar 29, 2018 at 11:47 AM, Sergei Kornilov <sk@zsrv.org> wrote:
Hello
I can not find "Reports" in bottom any page of CF app...
Such stats covers only reviews marked in CF app? Through "Comment"->"Review" buttons? I'm afraid this statistics will be inaccurate for new users (like me). Wiki page https://wiki.postgresql.org/wiki/Reviewing_a_Patch say
> Send reviews as replies to the email containing the patch
And nothing about changes in CF app.
That report is currently only available to CF managers. One of the main reasons for that is that the underlying data is very uncertain and we don't want people to draw "random" conclusions for it.
And yes, they only cover things marked in the CF app.
On Thu, Mar 29, 2018 at 4:19 AM, Magnus Hagander <magnus@hagander.net> wrote: > I wonder if we should consider adding a field to the CF app *specifically* > to track things like this. What I'm thinking is a field that's set (or at > least verified) by the person who flags a patch as committed with choices > like Trivial/Simple/Medium/Complex (trivial being things like typo fixes > etc, which today can hugely skew the stats). I think this would be pretty subjective ... and there are also a LOT of patches that don't go through the CF process. The ratio of commits to commitfest entries is at least 5:1. If we only track what gets registered in the CF app we're ignoring a huge amount of stuff. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, May 22, 2018 at 7:57 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Mar 29, 2018 at 4:19 AM, Magnus Hagander <magnus@hagander.net> wrote:
> I wonder if we should consider adding a field to the CF app *specifically*
> to track things like this. What I'm thinking is a field that's set (or at
> least verified) by the person who flags a patch as committed with choices
> like Trivial/Simple/Medium/Complex (trivial being things like typo fixes
> etc, which today can hugely skew the stats).
I think this would be pretty subjective ... and there are also a LOT
of patches that don't go through the CF process. The ratio of commits
to commitfest entries is at least 5:1. If we only track what gets
registered in the CF app we're ignoring a huge amount of stuff.
Definitely. It's easier to add structured data there than in the commit message though -- but we could also define a standard to add it to the commit messages. Or some inbetween. But whichever way we do it, it's likely going to lead to a non-trivial amount of work to maintain. So the big question is, is the data we can get out of it worth it?
Hi, On 2018-05-22 19:59:29 +0200, Magnus Hagander wrote: > So the big question is, is the data we can get out of it worth it? I can't see it being worth it personally. Greetings, Andres Freund
Hi, On 05/22/2018 01:57 PM, Robert Haas wrote: > On Thu, Mar 29, 2018 at 4:19 AM, Magnus Hagander <magnus@hagander.net> wrote: >> I wonder if we should consider adding a field to the CF app *specifically* >> to track things like this. What I'm thinking is a field that's set (or at >> least verified) by the person who flags a patch as committed with choices >> like Trivial/Simple/Medium/Complex (trivial being things like typo fixes >> etc, which today can hugely skew the stats). > > I think this would be pretty subjective ... and there are also a LOT > of patches that don't go through the CF process. The ratio of commits > to commitfest entries is at least 5:1. If we only track what gets > registered in the CF app we're ignoring a huge amount of stuff. > If it was set during the submission it could provide an insight to reviewers to the level of difficulty. Maybe that could help reviewers pick the "correct" entry. Of course there would need to be a link to the Wiki with a description of each of the levels. The committer could of course always change the level upon Rejected, Returned with Feedback or Committed. Best regards, Jesper
On Tue, May 22, 2018 at 2:02 PM, Andres Freund <andres@anarazel.de> wrote: > On 2018-05-22 19:59:29 +0200, Magnus Hagander wrote: >> So the big question is, is the data we can get out of it worth it? > > I can't see it being worth it personally. I tend to agree. First, it sounds like a lot of work. If we had the opportunity to improve our process in a way that would naturally gather this data while providing various benefits to the project, I could see doing it. But if you ask any group of people to do extra work just for reporting purposes, it tends to annoy people ... and they often don't take the trouble to make it accurate, either. Even with best intentions, it's all pretty subjective. Look at the arguments we have about Bruce's count of release note items -- does a change in the number represent a change in his standards, a change in the number of items meeting his standards, an artifact of how he groups things together, or a real effect? And that's with all the classification being done by one person. With ~19 active committers, it's bound to diverge even more. I have found counting "number of lines added" with "git log -w -M --stat" to be a decent barometer of contribution strength for a lot of patches, but it greatly overstates the value of mechanical change. I'm not sure there's any automated way to take that into account, but it would be nice if there were. Anyhow, I'm inclined to view automatically generated metrics, even if imperfect, as a better option than manual classification. Also, while it's useful to know how much is getting committed, I'm not sure how much it matters in the end. It should be clear to everyone at this point that there is insufficient committer bandwidth to deal with all of the good patches that get sent in. I suggest we discuss at PGCon what we want to do about that problem. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company