git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v6 00/42] Use ref transactions for all ref updates

From
Ronnie Sahlberg <sahlberg@google.com>
Date
May 1, 2014, 20:37 UTC
Message-ID
<1398976662-6962-1-git-send-email-sahlberg@google.com>

This patch series is based on next and expands on the transaction API. It converts all ref updates, inside refs.c as well as external, to use the transaction API for updates. This makes most of the ref updates to become atomic when there are failures locking or writing to a ref.

This version completes the work to convert all ref updates to use transactions. Now that all updates are through transactions I will start working on cleaning up the reading of refs and to create an api for managing reflogs but all that will go in a different patch series.

Version 6:
 - Convert all updates in refs.c to use transactions too.
Version 5:
 - Reword commit messages for having _create/_delete/_update returning
   success/failure. There are no conditions yet that return an error from
   these failures but there will be in the future. So we still check the
   return from these functions in the callers in preparation for this.
 - Don't leak memory by just passing a strbuf_detach() pointer to functions.
   Use <obj>.buf and explicitely strbuf_release the data afterwards.
 - Remove the function update_ref_lock.
 - Remove the function update_ref_write.
 - Track transaction status and die(BUG:) if we call _create/_delete/_update/
   _commit for a transaction that is not OPEN.
Version 4:
 - Rename patch series from "Use ref transactions from most callers" to
   "Use ref transactions for all ref updates".
 - Convert all external ref writes to use transactions and make write_ref_sha1
   and lock_ref_sha1 static functions.
 - Change the ref commit and free handling so we no longer pass pointer to
   pointer to _commit. _commit no longer frees the transaction. The caller
   MUST call _free itself.
 - Change _commit to take a strbuf pointer instead of a char* for error
   reporting back to the caller.
 - Re-add the walker patch after fixing it.
Version 3:
 - Remove the walker patch for now. Walker needs more complex solution
   so defer it until the basics are done.
 - Remove the onerr argument to ref_transaction_commit(). All callers
   that need to die() on error now have to do this explicitely.
 - Pass an error string from ref_transaction_commit() back to the callers
   so that they can craft a nice error message upon failures.
 - Make ref_transaction_rollback() accept NULL as argument.
 - Change ref_transaction_commit() to take a pointer to pointer argument for
   the transaction and have it clear the callers pointer to NULL when
   invoked. This allows for much nicer handling of transaction rollback on
   failure.
Version 2:
 - Add a patch to ref_transaction_commit to make it honor onerr even if the
   error triggered in ref_Transaction_commit itself rather than in a call
   to other functions (that already honor onerr).
 - Add a patch to make the update_ref() helper function use transactions
   internally.
 - Change ref_transaction_update to die() instead of error() if we pass
   if a NULL old_sha1 but have have_old == true.
 - Change ref_transaction_create to die() instead of error() if new_sha1
   is false but we pass it a null_sha1.
 - Change ref_transaction_delete die() instead of error() if we pass
   if a NULL old_sha1 but have have_old == true.
 - Change several places to do  if(!transaction || ref_transaction_update()
   || ref_Transaction_commit()) die(generic-message) instead of checking each
   step separately and having a different message for each failure.
   Most users are likely not interested in what step of the transaction
   failed and only whether it failed or not.
 - Change commit.c to only pass a pointer to ref_transaction_update
   iff current_head is non-NULL.
   The previous patch used to compute a garbage pointer for
   current_head->object.sha1 and relied on the fact that ref_transaction_update
   would not try to dereference this pointer if !!current_head was 0.
 - Updated commit message for the walker_fetch change to try to justify why
   the change in locking semantics should not be harmful.
Ronnie Sahlberg (42):
  refs.c: constify the sha arguments for
    ref_transaction_create|delete|update
  refs.c: allow passing NULL to ref_transaction_free
  refs.c: add a strbuf argument to ref_transaction_commit for error
    logging
  refs.c: make ref_update_reject_duplicates take a strbuf argument for
    errors
  update-ref.c: log transaction error from the update_ref
  refs.c: make update_ref_write update a strbuf on failure
  refs.c: remove the onerr argument to ref_transaction_commit
  refs.c: change ref_transaction_update() to do error checking and
    return status
  refs.c: change ref_transaction_create to do error checking and return
    status
  refs.c: ref_transaction_delete to check for error and return status
  tag.c: use ref transactions when doing updates
  replace.c: use the ref transaction functions for updates
  commit.c: use ref transactions for updates
  sequencer.c: use ref transactions for all ref updates
  fast-import.c: change update_branch to use ref transactions
  branch.c: use ref transaction for all ref updates
  refs.c: change update_ref to use a transaction
  refs.c: free the transaction before returning when number of updates
    is 0
  refs.c: ref_transaction_commit should not free the transaction
  fetch.c: clear errno before calling functions that might set it
  fetch.c: change s_update_ref to use a ref transaction
  fetch.c: use a single ref transaction for all ref updates
  receive-pack.c: use a reference transaction for updating the refs
  fast-import.c: use a ref transaction when dumping tags
  walker.c: use ref transaction for ref updates
  refs.c: make write_ref_sha1 static
  refs.c: make lock_ref_sha1 static
  refs.c: add transaction.status and track OPEN/CLOSED/ERROR
  refs.c: remove the update_ref_lock function
  refs.c: remove the update_ref_write function
  refs.c: remove lock_ref_sha1
  refs.c: make prune_ref use a transaction to delete the ref
  refs.c: make delete_ref use a transaction
  refs.c: pass the ref log message to _create/delete/update instead of
    _commit
  refs.c: pass NULL as *flags to read_ref_full
  refs.c: pack all refs before we start to rename a ref
  refs.c: move the check for valid refname to lock_ref_sha1_basic
  refs.c: call lock_ref_sha1_basic directly from commit
  refs.c: add a new flag for transaction delete for refs we know are
    packed only
  refs.c: pass a skip list to name_conflict_fn
  refs.c: make rename_ref use a transaction
  refs.c: remove forward declaraion of write_ref_sha1
 branch.c               |  31 ++--
 builtin/commit.c       |  24 ++-
 builtin/fetch.c        |  29 ++--
 builtin/receive-pack.c |  20 +--
 builtin/replace.c      |  15 +-
 builtin/tag.c          |  15 +-
 builtin/update-ref.c   |  32 ++--
 fast-import.c          |  39 +++--
 refs.c                 | 404 +++++++++++++++++++++++++++----------------------
 refs.h                 |  49 +++---
 sequencer.c            |  24 ++-
 t/t3200-branch.sh      |   2 +-
 walker.c               |  51 ++++---
 13 files changed, 414 insertions(+), 321 deletions(-)
-- 
2.0.0.rc1.351.g4d2c8e4
Next: Ronnie Sahlberg
Message 1 of 78 in “Use ref transactions for all ref updates”
  1. 00/42 Use ref transactions for all ref updatesRonnie Sahlberg, May 1, 2014
  2. 01/42 refs.c: constify the sha arguments for ref_transaction_create|delete|updateRonnie Sahlberg, May 1, 2014
  3. 02/42 refs.c: allow passing NULL to ref_transaction_freeRonnie Sahlberg, May 1, 2014
  4. Jonathan NiederMay 13, 2014
  5. Ronnie SahlbergMay 13, 2014
  6. Ronnie SahlbergMay 14, 2014
  7. 03/42 refs.c: add a strbuf argument to ref_transaction_commit for error loggingRonnie Sahlberg, May 1, 2014
  8. Jonathan NiederMay 13, 2014
  9. Ronnie SahlbergMay 14, 2014
  10. 04/42 refs.c: make ref_update_reject_duplicates take a strbuf argument for errorsRonnie Sahlberg, May 1, 2014
  11. Jonathan NiederMay 14, 2014
  12. Ronnie SahlbergMay 14, 2014
  13. 05/42 update-ref.c: log transaction error from the update_refRonnie Sahlberg, May 1, 2014
  14. Jonathan NiederMay 14, 2014
  15. Ronnie SahlbergMay 15, 2014
  16. 06/42 refs.c: make update_ref_write update a strbuf on failureRonnie Sahlberg, May 1, 2014
  17. Jonathan NiederMay 14, 2014
  18. 07/42 refs.c: remove the onerr argument to ref_transaction_commitRonnie Sahlberg, May 1, 2014
  19. Jonathan NiederMay 14, 2014
  20. 08/42 refs.c: change ref_transaction_update() to do error checking and return statusRonnie Sahlberg, May 1, 2014
  21. Michael HaggertyMay 5, 2014
  22. Ronnie SahlbergMay 5, 2014
  23. Jonathan NiederMay 14, 2014
  24. Ronnie SahlbergMay 15, 2014
  25. 09/42 refs.c: change ref_transaction_create to do error checking and return statusRonnie Sahlberg, May 1, 2014
  26. Jonathan NiederMay 15, 2014
  27. Ronnie SahlbergMay 15, 2014
  28. Jonathan NiederMay 15, 2014
  29. 10/42 refs.c: ref_transaction_delete to check for error and return statusRonnie Sahlberg, May 1, 2014
  30. Jonathan NiederMay 15, 2014
  31. Ronnie SahlbergMay 15, 2014
  32. 11/42 tag.c: use ref transactions when doing updatesRonnie Sahlberg, May 1, 2014
  33. Jonathan NiederMay 15, 2014
  34. Ronnie SahlbergMay 15, 2014
  35. Jonathan NiederMay 15, 2014
  36. 12/42 replace.c: use the ref transaction functions for updatesRonnie Sahlberg, May 1, 2014
  37. Jonathan NiederMay 15, 2014
  38. Ronnie SahlbergMay 15, 2014
  39. 13/42 commit.c: use ref transactions for updatesRonnie Sahlberg, May 1, 2014
  40. Jonathan NiederMay 15, 2014
  41. Ronnie SahlbergMay 15, 2014
  42. 14/42 sequencer.c: use ref transactions for all ref updatesRonnie Sahlberg, May 1, 2014
  43. Jonathan NiederMay 15, 2014
  44. 15/42 fast-import.c: change update_branch to use ref transactionsRonnie Sahlberg, May 1, 2014
  45. 16/42 branch.c: use ref transaction for all ref updatesRonnie Sahlberg, May 1, 2014
  46. 17/42 refs.c: change update_ref to use a transactionRonnie Sahlberg, May 1, 2014
  47. 18/42 refs.c: free the transaction before returning when number of updates is 0Ronnie Sahlberg, May 1, 2014
  48. 19/42 refs.c: ref_transaction_commit should not free the transactionRonnie Sahlberg, May 1, 2014
  49. 20/42 fetch.c: clear errno before calling functions that might set itRonnie Sahlberg, May 1, 2014
  50. Eric SunshineMay 2, 2014
  51. Ronnie SahlbergMay 2, 2014
  52. 21/42 fetch.c: change s_update_ref to use a ref transactionRonnie Sahlberg, May 1, 2014
  53. 22/42 fetch.c: use a single ref transaction for all ref updatesRonnie Sahlberg, May 1, 2014
  54. 23/42 receive-pack.c: use a reference transaction for updating the refsRonnie Sahlberg, May 1, 2014
  55. 24/42 fast-import.c: use a ref transaction when dumping tagsRonnie Sahlberg, May 1, 2014
  56. 25/42 walker.c: use ref transaction for ref updatesRonnie Sahlberg, May 1, 2014
  57. 26/42 refs.c: make write_ref_sha1 staticRonnie Sahlberg, May 1, 2014
  58. 27/42 refs.c: make lock_ref_sha1 staticRonnie Sahlberg, May 1, 2014
  59. 28/42 refs.c: add transaction.status and track OPEN/CLOSED/ERRORRonnie Sahlberg, May 1, 2014
  60. 29/42 refs.c: remove the update_ref_lock functionRonnie Sahlberg, May 1, 2014
  61. 30/42 refs.c: remove the update_ref_write functionRonnie Sahlberg, May 1, 2014
  62. 31/42 refs.c: remove lock_ref_sha1Ronnie Sahlberg, May 1, 2014
  63. 32/42 refs.c: make prune_ref use a transaction to delete the refRonnie Sahlberg, May 1, 2014
  64. 33/42 refs.c: make delete_ref use a transactionRonnie Sahlberg, May 1, 2014
  65. 34/42 refs.c: pass the ref log message to _create/delete/update instead of _commitRonnie Sahlberg, May 1, 2014
  66. 35/42 refs.c: pass NULL as *flags to read_ref_fullRonnie Sahlberg, May 1, 2014
  67. 36/42 refs.c: pack all refs before we start to rename a refRonnie Sahlberg, May 1, 2014
  68. 37/42 refs.c: move the check for valid refname to lock_ref_sha1_basicRonnie Sahlberg, May 1, 2014
  69. 38/42 refs.c: call lock_ref_sha1_basic directly from commitRonnie Sahlberg, May 1, 2014
  70. 39/42 refs.c: add a new flag for transaction delete for refs we know are packed onlyRonnie Sahlberg, May 1, 2014
  71. 40/42 refs.c: pass a skip list to name_conflict_fnRonnie Sahlberg, May 1, 2014
  72. Eric SunshineMay 2, 2014
  73. Ronnie SahlbergMay 2, 2014
  74. 41/42 refs.c: make rename_ref use a transactionRonnie Sahlberg, May 1, 2014
  75. 42/42 refs.c: remove forward declaraion of write_ref_sha1Ronnie Sahlberg, May 1, 2014
  76. Michael HaggertyMay 5, 2014
  77. Ronnie SahlbergMay 5, 2014
  78. Jonathan NiederMay 13, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.