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

Re: [PATCH 1/4] lockfile: report when rollback fails

From
Patrick Steinhardt <ps@pks.im>
Date
Mar 6, 2024, 12:00 UTC
Message-ID
<Zeha21vz6EJD8cnk@tanuki>
In-Reply-To
<lyarkcrkcb7ww542lc6y7t534yxnpahvrwephottvcbhr5dkn2@wwd5wk5bzlw7>
On Tue, Mar 05, 2024 at 04:09:07PM -0600, Justin Tobler wrote:
Show 36 quoted lines
> On 24/03/04 12:10PM, Patrick Steinhardt wrote:
> > We do not report to the caller when rolling back a lockfile fails, which
> > will be needed by the reftable compaction logic in a subsequent commit.
> > It also cannot really report on all errors because the function calls
> > `delete_tempfile()`, which doesn't return an error either.
> > 
> > Refactor the code so that both `delete_tempfile()` and
> > `rollback_lock_file()` return an error code.
> > 
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> >  lockfile.h |  4 ++--
> >  tempfile.c | 21 +++++++++++++--------
> >  tempfile.h |  2 +-
> >  3 files changed, 16 insertions(+), 11 deletions(-)
> > 
> > diff --git a/lockfile.h b/lockfile.h
> > index 90af4e66b2..4ed570d3f7 100644
> > --- a/lockfile.h
> > +++ b/lockfile.h
> > @@ -323,9 +323,9 @@ static inline int commit_lock_file_to(struct lock_file *lk, const char *path)
> >   * for a `lock_file` object that has already been committed or rolled
> >   * back.
> >   */
> > -static inline void rollback_lock_file(struct lock_file *lk)
> > +static inline int rollback_lock_file(struct lock_file *lk)
> >  {
> > -	delete_tempfile(&lk->tempfile);
> > +	return delete_tempfile(&lk->tempfile);
> >  }
> 
> question: For a lockfile that is already committed or rolled back, is
> the underlying tempfile still active? I'm trying to figure out if it
> possible for an error to be returned in this scenerio. If not, it might
> be nice to clarify in the comment above that, in addition to being a
> NOOP, no error is returned.

No, it's not. I thought that it being a no-op should be clear enough, but on the other hand it doesn't hurt either to clarify even further.

I'll refrain from sending a v2 only to add this comment as there haven't been any other things that need to be addressed yet.

Patrick
Previous: Justin ToblerNext: Patrick Steinhardt
Message 4 of 26 in “reftable/stack: register temporary files”
  1. 0/4 reftable/stack: register temporary filesPatrick Steinhardt, Mar 4, 2024
  2. 1/4 lockfile: report when rollback failsPatrick Steinhardt, Mar 4, 2024
  3. Justin ToblerMar 5, 2024
  4. Patrick SteinhardtMar 6, 2024
  5. 2/4 reftable/stack: register new tables as tempfilesPatrick Steinhardt, Mar 4, 2024
  6. Justin ToblerMar 5, 2024
  7. Patrick SteinhardtMar 6, 2024
  8. Justin ToblerMar 6, 2024
  9. Junio C HamanoMar 6, 2024
  10. Patrick SteinhardtMar 7, 2024
  11. Junio C HamanoMar 7, 2024
  12. Patrick SteinhardtMar 7, 2024
  13. Junio C HamanoMar 7, 2024
  14. 3/4 reftable/stack: register lockfiles during compactionPatrick Steinhardt, Mar 4, 2024
  15. Justin ToblerMar 5, 2024
  16. Patrick SteinhardtMar 6, 2024
  17. Junio C HamanoMar 6, 2024
  18. Justin ToblerMar 6, 2024
  19. 4/4 reftable/stack: register compacted tables as tempfilesPatrick Steinhardt, Mar 4, 2024
  20. Toon claesMar 7, 2024
  21. Patrick SteinhardtMar 7, 2024
  22. 0/4 reftable/stack: register temporary filesPatrick Steinhardt, Mar 7, 2024
  23. 1/4 lockfile: report when rollback failsPatrick Steinhardt, Mar 7, 2024
  24. 2/4 reftable/stack: register new tables as tempfilesPatrick Steinhardt, Mar 7, 2024
  25. 3/4 reftable/stack: register lockfiles during compactionPatrick Steinhardt, Mar 7, 2024
  26. 4/4 reftable/stack: register compacted tables as tempfilesPatrick Steinhardt, Mar 7, 2024

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.