Hi, (+Cc: reepca) Christopher Baines 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’.