Re: [PATCH] nfsdcld: use WAL journal for faster commits

[Bruce Fields <bfields@fieldses.org>]

On Fri, Dec 03, 2021 at 10:07:19PM +0000, Chuck Lever III wrote:
> 
> 
> > On Dec 3, 2021, at 4:55 PM, Bruce Fields <bfields@fieldses.org> wrote:
> > 
> > From: "J. Bruce Fields" <bfields@redhat.com>
> > 
> > Currently nfsdcld is doing three fdatasyncs for each upcall.  Based on
> > SQLite documentation, WAL mode should also be safe, and I can confirm
> > from an strace that it results in only one fdatasync each.
> > 
> > This may be a bottleneck e.g. when lots of clients are being created or
> > expired at once (e.g. on reboot).
> > 
> > Not bothering with error checking, as this is just an optimization and
> > nfsdcld will still function without.  (Might be better to log something
> > on failure, though.)
> 
> I'm in full philosophical agreement for performance improvements
> in this area. There are some caveats for WAL:
> 
>  - It requires SQLite v3.7.0 (2010). configure.ac may need to be
>    updated.

Makes sense.  But I dug around a bit, and how to do this is a total
mystery to me....

>  - All processes using the DB file have to be on the same system.
>    Not sure if this matters for some DR scenarios that nfs-utils
>    is supposed to support.

I don't think we support that.

>  - WAL mode is persistent; you could set this at database creation
>    time and it should be sti cky.

I wanted to upgrade existing databases too, and figured there's no harm
in calling it on each startup.

>  - Documentation says "synchronous = FULL is the most commonly
>    used synchronous setting when not in WAL mode." Why are both
>    PRAGMAs necessary here?

Maybe they're not; I think I did see that FULL is actually the default
but I can't find that in the documentation right now.

--b.

> 
> I agree that nfsdcld functionality is not affected if the first
> PRAGMA fails.
> 
> 
> > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > ---
> > utils/nfsdcld/sqlite.c | 3 +++
> > 1 file changed, 3 insertions(+)
> > 
> > diff --git a/utils/nfsdcld/sqlite.c b/utils/nfsdcld/sqlite.c
> > index 03016fb95823..b248eeffa204 100644
> > --- a/utils/nfsdcld/sqlite.c
> > +++ b/utils/nfsdcld/sqlite.c
> > @@ -826,6 +826,9 @@ sqlite_prepare_dbh(const char *topdir)
> > 		goto out_close;
> > 	}
> > 
> > +	sqlite3_exec(dbh, "PRAGMA journal_mode = WAL;", NULL, NULL, NULL);
> > +	sqlite3_exec(dbh, "PRAGMA synchronous = FULL;", NULL, NULL, NULL);
> > +
> > 	ret = sqlite_query_schema_version();
> > 	switch (ret) {
> > 	case CLD_SQLITE_LATEST_SCHEMA_VERSION:
> > -- 
> > 2.33.1
> > 
> 
> --
> Chuck Lever
> 
>