From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 934DF1FA10 for ; Tue, 25 Aug 2020 03:02:47 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 3/3] over+msgmap: respect WAL journal_mode if set Date: Tue, 25 Aug 2020 03:02:47 +0000 Message-Id: <20200825030247.12307-4-e@yhbt.net> In-Reply-To: <20200825030247.12307-1-e@yhbt.net> References: <20200825030247.12307-1-e@yhbt.net> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: WAL actually seems to have ideal locking characteristics given concurrency problems I'm experiencing with --reindex running in parallel with expensive read-only SQLite queries: Unfortunately, we cannot blindly use WAL while preserving compatibility with existing setups nor our guarantees that read-only daemons are indeed "read-only". However, respect an user's the choice to set WAL on their own if they're comfortable with giving -nntpd/-httpd/-imapd processes write permission to the directory storing SQLite DBs. --- lib/PublicInbox/Msgmap.pm | 3 --- lib/PublicInbox/Over.pm | 23 ++++++++++++++++++++++- lib/PublicInbox/OverIdx.pm | 5 ----- t/over.t | 10 ++++++++++ 4 files changed, 32 insertions(+), 9 deletions(-) diff --git a/lib/PublicInbox/Msgmap.pm b/lib/PublicInbox/Msgmap.pm index 5b4cebc1..d28e96c8 100644 --- a/lib/PublicInbox/Msgmap.pm +++ b/lib/PublicInbox/Msgmap.pm @@ -31,9 +31,6 @@ sub new_file { my $self = bless { filename => $f }, $class; my $dbh = $self->{dbh} = PublicInbox::Over::dbh_new($self, $rw); if ($rw) { - # TRUNCATE reduces I/O compared to the default (DELETE) - $dbh->do('PRAGMA journal_mode = TRUNCATE'); - $dbh->begin_work; create_tables($dbh); $self->created_at(time) unless $self->created_at; diff --git a/lib/PublicInbox/Over.pm b/lib/PublicInbox/Over.pm index a2f04117..3e74b7a6 100644 --- a/lib/PublicInbox/Over.pm +++ b/lib/PublicInbox/Over.pm @@ -42,7 +42,28 @@ sub dbh_new { $st = pack('dd', $st[0], $st[1]); } while ($st ne $self->{st} && $tries++ < 3); warn "W: $f: .st_dev, .st_ino unstable\n" if $st ne $self->{st}; - $dbh->do('PRAGMA synchronous = OFF') if ($rw // 0) > 1; + + if ($rw) { + # TRUNCATE reduces I/O compared to the default (DELETE). + # + # Do not use WAL by default since we expect the case + # where any users may read via read-only daemons + # (-httpd/-imapd/-nntpd); but only a single user has + # write permissions for -watch/-mda. + # + # Read-only WAL support in SQLite 3.22.0 (2018-01-22) + # doesn't do what we need: it is only intended for + # immutable read-only media (e.g. CD-ROM) and not + # usable for our use case described above. + # + # If an admin is willing to give read-only daemons R/W + # permissions; they can enable WAL manually and we will + # respect that by not clobbering it. + my $jm = $dbh->selectrow_array('PRAGMA journal_mode'); + $dbh->do('PRAGMA journal_mode = TRUNCATE') if $jm ne 'wal'; + + $dbh->do('PRAGMA synchronous = OFF') if $rw > 1; + } $dbh; } diff --git a/lib/PublicInbox/OverIdx.pm b/lib/PublicInbox/OverIdx.pm index d42d6fe7..9f4a56fb 100644 --- a/lib/PublicInbox/OverIdx.pm +++ b/lib/PublicInbox/OverIdx.pm @@ -23,11 +23,6 @@ sub dbh_new { my ($self) = @_; my $dbh = $self->SUPER::dbh_new($self->{-no_fsync} ? 2 : 1); - # TRUNCATE reduces I/O compared to the default (DELETE) - # We do not use WAL since we're optimized for read-only ops, - # (and read-only requires SQLite 3.22.0 (2018-01-22)). - $dbh->do('PRAGMA journal_mode = TRUNCATE'); - # 80000 pages (80MiB on SQLite <3.12.0, 320MiB on 3.12.0+) # was found to be good in 2018 during the large LKML import # at the time. This ought to be configurable based on HW diff --git a/t/over.t b/t/over.t index 07672aa7..8bf64ecb 100644 --- a/t/over.t +++ b/t/over.t @@ -65,4 +65,14 @@ isnt($over->max, 0, 'max is non-zero'); $over->rollback_lazy; +# L +my $v = eval 'v'.$over->{dbh}->{sqlite_version}; +SKIP: { + skip("no WAL in SQLite version $v < 3.7.0", 1) if $v lt v3.7.0; + $over->{dbh}->do('PRAGMA journal_mode = WAL'); + $over = PublicInbox::OverIdx->new("$tmpdir/over.sqlite3"); + is($over->connect->selectrow_array('PRAGMA journal_mode'), 'wal', + 'WAL journal_mode not clobbered if manually set'); +} + done_testing();