Re: [PATCH 3/3] fetch.c: use a single ref transaction for all ref updates

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Apr 22, 2014 at 2:45 PM, Ronnie Sahlberg <sahlberg@xxxxxxxxxx> wrote:
> Change store_updated_refs to use a single ref transaction for all refs that
> are updated during the fetch. This makes the fetch more atomic when update
> failures occur.
>
> Since ref update failures will now no longer occur in the code path for
> updating a single ref in s_update_ref, we no longer have as detailed error
> message logging the exact reference and the ref log action as in the old code.
> Instead since we fail the entire transaction we log a much more generic
> message. But since we commit the transaction using MSG_ON_ERR we will log
> an error containing the ref name if either locking of writing the ref would
> so the regression in the log message is minor.
>
> This will also change the order in which errors are checked for and logged
> which may alter which error will be logged if there are multiple errors
> occuring during a fetch.
>
> For example, assuming we have a fetch for two refs that both would fail.

s/assuming/assume/ perhaps?

> Where the first ref would fail with ENOTDIR due to a directory in the ref
> path not existing, and the second ref in the fetch would fail due to
> the check in update_logical_ref():
>         if (current_branch &&
>             !strcmp(ref->name, current_branch->name) &&
>             !(update_head_ok || is_bare_repository()) &&
>             !is_null_sha1(ref->old_sha1)) {
>                 /*
>                  * If this is the head, and it's not okay to update
>                  * the head, and the old value of the head isn't empty...
>                  */
>
> In the old code sicne we would update the refs one ref at a time we would

s/sicne/since/

> first fail the ENOTDIR and then fail the second update of HEAD as well.
> But since the first ref failed with ENOTDIR we would eventually fail the whole
> fetch with STORE_REF_ERROR_DF_CONFLICT
>
> In the new code, since we defer committing the transaction until all refs
> has been processed, we would now detect that the second ref was bad and

s/has/have/

> rollback the transaction before we would even try start writing the update to
> disk and thus we would not return STORE_REF_ERROR_DF_CONFLICT for this case.
>
> I think this new behaviour is more correct, since if there was a problem
> we would not even try to commit the transaction but need to highlight this
> change in how/what errors are reported.
> This change in what error is returned only occurs if there are multiple
> refs that fail to update and only some, but not all, of them fail due to
> ENOTDIR.
>
> Signed-off-by: Ronnie Sahlberg <sahlberg@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]