GNU bug report logs - #69292
[PATCH 0/6] Prepare the database code for use in the daemon

Previous Next

Full log

View this message in rfc822 format

From: Reepca Russelstein <reepca <at> russelstein.xyz>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, Tobias Geerinckx-Rice <me <at> tobias.gr>, Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>, Christopher Baines <mail <at> cbaines.net>, Ricardo Wurmus <rekado <at> elephly.net>, 69292 <at> debbugs.gnu.org, Christopher Baines <guix <at> cbaines.net>
Subject: [bug#69292] [PATCH 2/6] store: database: Remove with-statement and associated code.
Date: Fri, 23 Feb 2024 12:32:03 -0600

[Message part 1 (text/plain, inline)]

>> I think using dynamic-wind to finalize all statements is the wrong
>> approach. Firstly it would be good to allow reseting statements rather than
>> finalizing them.

Let me once again mention https://issues.guix.gnu.org/69292#7 with this
much more magnificent Cc list.

>> Then for the problem of handling errors, the approach I've
>> settled on in the build coordinator is to close the database connection, since
>> that'll trigger guile-sqlite3 to finalize all the cached statements.

And in the event that a statement is *not* cached, it will hang around
until the gc next pumps the statement guardian, at which point it will
do... whatever happens when a statement is finalized after the database
connection it was created with has already been closed, I guess.  I
don't know if that's a problem or not.  On further investigation, it
appears that sqlite_close would return SQLITE_BUSY, but guile-sqlite3's
sqlite-close doesn't throw any exceptions, and according to
https://www.sqlite.org/c3ref/close.html it would just hold off on
actually closing the database until all statements have been finalized.
So I guess that works.

>>
>> This reverts commit 5d6e2255286e591def122ec2f4a3cbda497fea21.
>>
>> * .dir-locals.el (scheme-mode): Remove with-statement.
>> * guix/store/database.scm (call-with-statement): Remove procedure.
>> (with-statement): Remove syntax rule.
>> (call-with-transaction, last-insert-row-id, path-id, update-or-insert,
>> add-references): Don't use with-statement.
>>
>> Change-Id: I2fd976b3f12ec8105cc56350933a953cf53647e8
>
> I’m all for removing ‘dynamic-wind’, we’ll have to do it to make it
> usable in a fiberized context anyway.
>
> I’ll let reepca comment.

What is the proper fibers-friendly replacement for dynamic-wind, anyway,
that is "like dynamic-wind, except when suspending a fiber"?  It feels
like the current interaction between dynamic-wind and fibers is more of
an accident of how the implementation works; I don't suppose fibers could
export a transparent replacement, like how it already exports a
replacement for 'sleep'?  Or perhaps the underlying issue is that we
keep using 'dynamic-wind' in situations where it only makes sense to
enter or exit the dynamic extent once?  Is it time to bring
'unwind-protect' back into style?  I see that fibers now has a
dynamic-wind*, should that be preferred?

I don't have a strong opinion on these changes, I just want to make sure
we're all aware of how guile-sqlite3's sqlite-finalize acts with cached
statements.

- reepca

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.