git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Beller <sbeller@google.com>
To: mhagger@alum.mit.edu, peff@peff.net, git@vger.kernel.org,
	gitster@pobox.com, loic@dachary.org
Cc: Stefan Beller <sbeller@google.com>
Subject: [PATCHv3 0/6] Fix bug in large transactions
Date: Fri, 23 Jan 2015 12:03:56 -0800	[thread overview]
Message-ID: <1422043442-30676-1-git-send-email-sbeller@google.com> (raw)
In-Reply-To: <54C0E76D.5070104@alum.mit.edu>

version3:
  patches 1,2,3 stayed completely as is, while patches 4,5 are new, patch 6 is 
  rewritten to first write the contents of the lock files before closing them.
  
  This combines the series "Enable large transactions v2" as sent out yesterday
  with the follow up series "[RFC PATCH 0/5] So you dislike the sequence of 
  system calls?"
  
  There is no write_in_full_to_lock_file wrapper any more, but write_ref_sha1
  was reduced in functionality in patch 5.
  
  This applies on top of origin/sb/atomic-push and results in a merge conflict
  when merging it to origin/jk/lock-ref-sha1-basic-return-errors which looks like
        
        $git checkout origin/jk/lock-ref-sha1-basic-return-errors
        $git merge enable_large_transactions
        CONFLICT (content): Merge conflict in refs.c
        $git diff
++<<<<<<< HEAD
 +      lock->lock_fd = hold_lock_file_for_update(lock->lk, ref_file, lflags);
 +      if (lock->lock_fd < 0) {
++=======
+       if (hold_lock_file_for_update(lock->lk, ref_file, lflags) < 0) {
++>>>>>>> enable_large_transactions

which is best resolved as:
@@@ -2316,8 -2333,7 +2333,12 @@@ static struct ref_lock *lock_ref_sha1_b
                goto error_return;
        }
  
+        if (hold_lock_file_for_update(lock->lk, ref_file, lflags) < 0) {
                last_errno = errno;
                if (errno == ENOENT && --attempts_remaining > 0)
                        /*


version2:

* This applies on top of origin/sb/atomic-push though it will result in a one
  line merge conflict with origin/jk/lock-ref-sha1-basic-return-errors when
  merging to origin/next.

* It now uses the FILE* pointer instead of file descriptors. This
  results in a combination of the 2 former patches "refs.c: have
  a write_in_full_to_lock_file wrapper" and "refs.c: write to a
  lock file only once" as the wrapper function is more adapted to
  its consumers

* no need to dance around with char *pointers which may leak.

* another new patch sneaked into the series: Renaming ULIMIT in t7004
  to ULIMIT_STACK_SIZE

That said, only the first and third patch are updated from the first version
of the patches. The others are new in the sense that rewriting them was cheaper
than keeping notes in between.

version1:

(reported as: git update-ref --stdin : too many open files, 2014-12-20)

First a test case is introduced to demonstrate the failure,
the patches 2-6 are little refactoring and the last patch
fixes the bug and also marks the bugs as resolved in the
test suite.

Unfortunately this applies on top of origin/next.

Any feedback would be welcome!

Thanks,
Stefan

Stefan Beller (6):
  update-ref: test handling large transactions properly
  t7004: rename ULIMIT test prerequisite to ULIMIT_STACK_SIZE
  refs.c: remove lock_fd from struct ref_lock
  refs.c: move static functions to close and commit refs
  refs.c: remove unlock_ref and commit_ref from write_ref_sha1
  refs.c: enable large transactions

 refs.c                | 93 +++++++++++++++++++++++++++------------------------
 t/t1400-update-ref.sh | 28 ++++++++++++++++
 t/t7004-tag.sh        |  4 +--
 3 files changed, 79 insertions(+), 46 deletions(-)

-- 
2.2.1.62.g3f15098

  reply	other threads:[~2015-01-23 20:05 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-22  2:32 [PATCHv2 0/5] Fix bug in large transactions Stefan Beller
2015-01-22  2:32 ` [PATCHv2 1/5] update-ref: test handling large transactions properly Stefan Beller
2015-01-22 10:54   ` Michael Haggerty
2015-01-22 13:07     ` Jeff King
2015-01-22  2:32 ` [PATCHv2 2/5] t7004: rename ULIMIT test prerequisite to ULIMIT_STACK_SIZE Stefan Beller
2015-01-22  2:32 ` [PATCHv2 3/5] refs.c: remove lock_fd from struct ref_lock Stefan Beller
2015-01-22  2:32 ` [PATCHv2 4/5] refs.c: have a write_sha1_to_lock_file wrapper Stefan Beller
2015-01-22  2:32 ` [PATCHv2 5/5] refs.c: enable large transactions Stefan Beller
2015-01-22 11:24   ` Michael Haggerty
2015-01-22 13:10     ` Jeff King
2015-01-22 16:33       ` Michael Haggerty
2015-01-22 19:24         ` Stefan Beller
2015-01-22 23:11     ` [RFC PATCH 0/5] So you dislike the sequence of system calls? Stefan Beller
2015-01-22 23:11       ` [PATCH 1/5] fixup for "refs.c: enable large transactions" Stefan Beller
2015-01-22 23:11       ` [PATCH 2/5] refs.c: remove unlock_ref from write_ref_sha1 Stefan Beller
2015-01-22 23:11       ` [PATCH 3/5] refs.c: move static functions to close and commit refs Stefan Beller
2015-01-22 23:11       ` [PATCH 4/5] refs.c: remove committing the ref from write_ref_sha1 Stefan Beller
2015-01-22 23:11       ` [PATCH 5/5] refs.c: write values to lock files early for committing Stefan Beller
2015-01-22 12:59   ` [PATCHv2 5/5] refs.c: enable large transactions Ramsay Jones
2015-01-22 19:16     ` Stefan Beller
2015-01-22 19:51       ` Ramsay Jones
2015-01-22 20:13         ` Ramsay Jones
2015-01-22 20:20           ` Stefan Beller
2015-01-22 20:59             ` Ramsay Jones
2015-01-22 12:05 ` [PATCHv2 0/5] Fix bug in " Michael Haggerty
2015-01-23 20:03   ` Stefan Beller [this message]
2015-01-23 20:03     ` [PATCHv3 1/6] update-ref: test handling large transactions properly Stefan Beller
2015-01-23 20:03     ` [PATCHv3 2/6] t7004: rename ULIMIT test prerequisite to ULIMIT_STACK_SIZE Stefan Beller
2015-01-23 20:03     ` [PATCHv3 3/6] refs.c: remove lock_fd from struct ref_lock Stefan Beller
2015-01-23 20:04     ` [PATCHv3 4/6] refs.c: move static functions to close and commit refs Stefan Beller
2015-01-23 20:04     ` [PATCHv3 5/6] refs.c: remove unlock_ref and commit_ref from write_ref_sha1 Stefan Beller
2015-01-23 23:57       ` Junio C Hamano
2015-01-24  0:22         ` Stefan Beller
2015-01-24  0:39           ` Junio C Hamano
2015-01-24  1:04             ` Stefan Beller
2015-01-24  1:29               ` Junio C Hamano
2015-01-23 20:04     ` [PATCHv3 6/6] refs.c: enable large transactions Stefan Beller
2015-01-24  0:14       ` Junio C Hamano
2015-01-24  0:24         ` Stefan Beller
2015-01-24  0:38           ` Junio C Hamano
2015-01-26 19:30             ` Stefan Beller
2015-01-26 21:10               ` [PATCH] refs.c: clean up write_ref_sha1 returns Stefan Beller
2015-01-27  3:22                 ` Junio C Hamano
2015-01-28 21:35                   ` Stefan Beller
2015-01-27  3:17               ` [PATCHv3 6/6] refs.c: enable large transactions Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1422043442-30676-1-git-send-email-sbeller@google.com \
    --to=sbeller@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=loic@dachary.org \
    --cc=mhagger@alum.mit.edu \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).