Commit fest 2017-11

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 marking it as "waiting on author", but the patch doesn't move to the next CF when I then change the status as "Move to next CF." 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 then change 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 then change 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 Anastasia that 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 Anastasia that 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

On Fri, Dec 1, 2017 at 12:44 AM, Andreas Karlsson <andreas@proxel.se> wrote:

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.

As a patch author, you have the right to decide what you are yourself
planning to do with your own baby :)

When the commit fest comes close to the end, I always try to deal with
my own patches first. If the discussion has stalled or that based on
the feedback a given thing is going to require more efforts than I
initially planned so it is not possible to spend more time on
something in the close future, I mark my own entries as returned with
feedback. If I am still planning to work on something in the close
future, then I move them to the next CF. This makes less work for the
CFM which has less patches to deal with at the end. Mentioning what
you do on the patch thread with the so-said CF entry is also something
worth doing in my opinion to keep a track of what you did. When
multiple authors and/or reviewers are involved, it is sometimes good
to post your intention before doing it some time ahead.
--
Michael

On Thu, Nov 30, 2017 at 1:51 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

Remains 22 patches as of now, exactly *one* for each committer.

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.

(I know that I am a couple of hours ahead, doing things now fits
better with my schedule.)
--
Michael

Hello Michaël,

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.

There is a significant "moved to next CF" set of patches, which includes:

- patches that were ready but did not get a committer,
(about one per committer...)

- patches that were still waiting for a review (a lot...)

- possibly a few that were in "waiting on author" state, although
they tend to be switched to "returned with feedback".

There is a rolling chunck of 50% of patches which moves from CF to CF.

Not enough review(er)s, obviously.

Also, not enough committers: as a reviewer, if the patches I review do not
get committed once ready, spending time on reviews becomes quite
unattractive.

--
Fabien.

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...

------
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 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).

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;

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 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).

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

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

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 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).

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..

Yes, there is (stalled, but still) work in progress on that one. I think
that's a separate thing though.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;