Re: [PATCH 5/5] ref_transaction_commit(): only keep one lockfile open at a time

2015-04-27 Thread Jeff King
On Sat, Apr 25, 2015 at 12:21:07PM -0700, Junio C Hamano wrote: > Junio C Hamano writes: > > > I am not too worried about "push --atomic", as we can just add a few > > words to Release Notes and documentation saying "this is still an > > experimental broken code that is unusable; don't use the f

Re: [PATCH 5/5] ref_transaction_commit(): only keep one lockfile open at a time

2015-04-25 Thread Junio C Hamano
Junio C Hamano writes: > I am not too worried about "push --atomic", as we can just add a few > words to Release Notes and documentation saying "this is still an > experimental broken code that is unusable; don't use the feature in > production". > > I however am more worried about the other one

Re: [PATCH 5/5] ref_transaction_commit(): only keep one lockfile open at a time

2015-04-24 Thread Junio C Hamano
Michael Haggerty writes: > Given that "git push --atomic" is one of the main new features of 2.4.0, > it would be unfortunate for the release to contain this bug, plus the > bug that atomic pushes can fail due to file descriptor exhaustion. > > Is this problem important enough to justify delaying

Re: [PATCH 5/5] ref_transaction_commit(): only keep one lockfile open at a time

2015-04-24 Thread Michael Haggerty
On 04/24/2015 01:35 PM, Michael Haggerty wrote: > The old code locked all of the references in the first loop, leaving > all of the lockfiles open until later loops. Since we might be > updating a lot of references, this could cause file descriptor > exhaustion. > > But there is no reason to keep

[PATCH 5/5] ref_transaction_commit(): only keep one lockfile open at a time

2015-04-24 Thread Michael Haggerty
The old code locked all of the references in the first loop, leaving all of the lockfiles open until later loops. Since we might be updating a lot of references, this could cause file descriptor exhaustion. But there is no reason to keep the lockfiles open after the first loop: * For references b