From: "Ludovic Courtès" <ludo@gnu.org>
To: Christopher Baines <mail@cbaines.net>
Cc: 42023@debbugs.gnu.org, Caleb Ristvedt <caleb.ristvedt@cune.org>
Subject: [bug#42023] [PATCH] database: register-items: reduce transaction scope.
Date: Wed, 24 Jun 2020 00:15:30 +0200 [thread overview]
Message-ID: <87366lxwcd.fsf@gnu.org> (raw)
In-Reply-To: <20200623163649.32444-1-mail@cbaines.net> (Christopher Baines's message of "Tue, 23 Jun 2020 17:36:49 +0100")
Hi,
(+Cc: reepca)
Christopher Baines <mail@cbaines.net> skribis:
> It was made transactional in a4678c6ba18d8dbd79d931f80426eebf61be7ebe, with
> the reasoning to prevent broken intermediate states from being visible. I
> think this means something like an entry being in ValidPaths, but the Refs not
> being inserted.
>
> Using a transaction for this makes sense, but I think using one single
> transaction for the whole register-items call is unnecessary to avoid broken
> states from being visible, and could block other writes to the store database
> while register-items is running. Because the deduplication and resetting
> timestamps happens within the transaction as well, even though these things
> don't involve the database, writes to the database will still be blocked while
> this is happening.
>
> To reduce the potential for register-items to block other writers to the
> database for extended periods, this commit moves the transaction to just wrap
> the call to sqlite-register. This is the one place where writes occur, so that
> should prevent the broken intermediate states issue above. The one difference
> this will make is some of the registered items will be visible to other
> connections while others may be still being added. I think this is OK, as it's
> equivalent to just registering different items.
>
> * guix/store/database.scm (register-items): Reduce transaction scope.
[...]
> + (call-with-retrying-transaction db
> + (lambda ()
^^
Too much indentation (maybe we miss a rule in .dir-locals.el?).
> + (sqlite-register db #:path to-register
> + #:references (store-info-references item)
> + #:deriver (store-info-deriver item)
> + #:hash (string-append
> + "sha256:"
> + (bytevector->base16-string hash))
> + #:nar-size nar-size
> + #:time registration-time)))
I think it would be good to have a 2-line summary of the rationale right
above ‘call-with-retrying-transaction’.
Two questions:
1. Can another process come and fiddle with TO-REGISTER while we’re
still in ‘reset-timestamps’? Or can GC happen while we’re in
‘reset-timestamps’ and delete TO-REGISTER and remove it from the
database?
I think none of these scenarios can happen, as long as we’ve taken the
.lock file for TO-REGISTER before, like ‘finalize-store-file’ does.
2. After the transaction, TO-REGISTER is considered valid. But are
the effects of the on-going deduplication observable, due to
non-atomicity of some operation?
I think the ‘replace-with-link’ dance is atomic, so we should be fine.
Thoughts?
Ludo’.
next prev parent reply other threads:[~2020-06-23 22:16 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-23 16:36 [bug#42023] [PATCH] database: register-items: reduce transaction scope Christopher Baines
2020-06-23 22:15 ` Ludovic Courtès [this message]
2020-06-24 17:51 ` Caleb Ristvedt
2020-06-25 7:45 ` Ludovic Courtès
2020-07-26 15:31 ` Ludovic Courtès
2020-08-09 4:13 ` Caleb Ristvedt
2020-09-14 8:58 ` Ludovic Courtès
2020-09-15 20:29 ` [bug#42023] [PATCH] Retry deduplication on ENOENT Caleb Ristvedt
2020-09-16 20:37 ` Ludovic Courtès
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
List information: https://guix.gnu.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87366lxwcd.fsf@gnu.org \
--to=ludo@gnu.org \
--cc=42023@debbugs.gnu.org \
--cc=caleb.ristvedt@cune.org \
--cc=mail@cbaines.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.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).