Re: [PATCH 03/15] refs.c: use packed refs when deleting refs during a transaction
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 22, 2014, 19:48 UTC
- Message-ID
- <xmqqy4s7homc.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1413923820-14457-4-git-send-email-sahlberg@google.com>
Ronnie Sahlberg <sahlberg@google.com> writes:
> commit fb5fa1d338ce113b0fea3bb955b50bbb3e827805 upstream.
Huh?
Show 45 quoted lines
> Make the deletion of refs during a transaction more atomic. > Start by first copying all loose refs we will be deleting to the packed > refs file and then commit the packed refs file. Then re-lock the packed refs > file to stop anyone else from modifying these refs and keep it locked until > we are finished. > Since all refs we are about to delete are now safely held in the packed refs > file we can proceed to immediately unlink any corresponding loose refs > and still be fully rollback-able. > > The exception is for refs that can not be resolved. Those refs are never > added to the packed refs and will just be un-rollback-ably deleted during > commit. > > By deleting all the loose refs at the start of the transaction we make make > it possible to both delete one ref and then re-create a different ref in > the same transaction even if the two refs would normally conflict. > > Example: rename m->m/m > > In that example we want to delete the file 'm' so that we make room so > that we can create a directory with the same name in order to lock and > write to the ref m/m and its lock-file m/m.lock. > > If there is a failure during the commit phase we can rollback without losing > any refs since we have so far only deleted loose refs that that are > guaranteed to also have a corresponding entry in the packed refs file. > Once we have finished all updates for refs and their reflogs we can repack > the packed refs file and remove the to-be-deleted refs from the packed refs, > at which point all the deleted refs will disappear in one atomic rename > operation. > > This also means that for an outside observer, deletion of multiple refs > in a single transaction will look atomic instead of one ref being deleted > at a time. > > In order to do all this we need to change the semantics for the > repack_without_refs function so that we can lock the packed refs file, > do other stuff, and later be able to call repack_without_refs with the > lock already taken. > This means we need some additional changes in remote.c to reflect the > changes to the repack_without_refs semantics. > > Change-Id: I1e4f58050acaabc6bcfa3409fbc0c2b866bb59aa > Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> > Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Show 6 quoted lines
> @@ -3821,23 +3824,109 @@ int transaction_commit(struct transaction *transaction,
> ...
> + if (need_repack) {
> + packed = get_packed_refs(&ref_cache);;
> + sort_ref_dir(packed);
> + if (commit_packed_refs()){SP between "){".> + strbuf_addf(err, "unable to overwrite old ref-pack " > + "file");
This is bending backwards sacrificing readability; 80-col limit is not that hard a limit. Split at the comma instead, perhaps?
strbuf_addf(err, "unable to overwrite old ref-pack file");
Show 8 quoted lines
> ...
> + if (!update->lock) {
> + int df_conflict = (errno == ENOTDIR);
> +
> + strbuf_addf(err, "Cannot lock the ref '%s'.",
> + update->refname);
> + ret = df_conflict ?
> + TRANSACTION_NAME_CONFLICT : A trailing SP here...
Show 10 quoted lines
> @@ -3860,6 +3953,16 @@ int transaction_commit(struct transaction *transaction,
> update->reflog_lock = update->orig_update->reflog_lock;
> continue;
> }
> + if (log_all_ref_updates && !reflog_exists(update->refname) &&
> + create_reflog(update->refname)) {
> + ret = -1;
> + if (err)
> + strbuf_addf(err, "Failed to setup reflog for "
> + "%s", update->refname);Split after comma, perhaps?