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 5CA331F94E for ; Sat, 27 Jun 2020 10:04:06 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 33/34] imaptracker: use flock(2) around writes Date: Sat, 27 Jun 2020 10:03:59 +0000 Message-Id: <20200627100400.9871-34-e@yhbt.net> In-Reply-To: <20200627100400.9871-1-e@yhbt.net> References: <20200627100400.9871-1-e@yhbt.net> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: SQLite only issues non-blocking F_SETLK ops (not F_SETLKW) and retries failures using a configurable busy_timeout. SQLite's busy loop sleeps for a millisecond and retries the lock until the configured busy_timeout is hit. Trying to set ->sqlite_busy_timeout to larger values (e.g. 30000 milliseconds) still leads to failure when running the new stress test with 8 processes with TMPDIR on a 7200 RPM HDD. Inspection of SQLite source reveals there's no built-in way to use F_SETLKW, so tack on the existing flock(2) support we use to synchronize git + SQLite + Xapian for inbox writing. We use flock(2) instead of POSIX fcntl(2) locks since Perl doesn't provide a way to manipulate "struct flock" portably. --- lib/PublicInbox/IMAPTracker.pm | 13 ++++++++++--- t/imap_tracker.t | 30 +++++++++++++++++++++++++++++- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/lib/PublicInbox/IMAPTracker.pm b/lib/PublicInbox/IMAPTracker.pm index 0bbabe07fae..102a74ce66b 100644 --- a/lib/PublicInbox/IMAPTracker.pm +++ b/lib/PublicInbox/IMAPTracker.pm @@ -2,6 +2,7 @@ # License: AGPL-3.0+ package PublicInbox::IMAPTracker; use strict; +use parent qw(PublicInbox::Lock); use DBI; use DBD::SQLite; use PublicInbox::Config; @@ -48,7 +49,10 @@ sub update_last ($$$) { INSERT OR REPLACE INTO imap_last (url, uid_validity, uid) VALUES (?, ?, ?) - $sth->execute($self->{url}, $validity, $last); + $self->lock_acquire; + my $rv = $sth->execute($self->{url}, $validity, $last); + $self->lock_release; + $rv; } sub new { @@ -68,8 +72,11 @@ sub new { require File::Basename; File::Path::mkpath(File::Basename::dirname($dbname)); } - - bless { url => $url, dbh => dbh_new($dbname) }, $class; + my $self = bless { lock_path => "$dbname.lock", url => $url }, $class; + $self->lock_acquire; + $self->{dbh} = dbh_new($dbname); + $self->lock_release; + $self; } 1; diff --git a/t/imap_tracker.t b/t/imap_tracker.t index 8dc04ed77a3..01e1d0b1549 100644 --- a/t/imap_tracker.t +++ b/t/imap_tracker.t @@ -9,8 +9,8 @@ my ($tmpdir, $for_destroy) = tmpdir(); mkdir "$tmpdir/old" or die "mkdir $tmpdir/old: $!"; my $old = "$tmpdir/old/imap.sqlite3"; my $cur = "$tmpdir/data/public-inbox/imap.sqlite3"; +local $ENV{XDG_DATA_HOME} = "$tmpdir/data"; { - local $ENV{XDG_DATA_HOME} = "$tmpdir/data"; local $ENV{PI_DIR} = "$tmpdir/old"; my $tracker = PublicInbox::IMAPTracker->new; @@ -22,5 +22,33 @@ my $cur = "$tmpdir/data/public-inbox/imap.sqlite3"; $tracker = PublicInbox::IMAPTracker->new; ok(!-f $cur, '->new does not create new file if old is present'); } +SKIP: { + my $nproc = $ENV{TEST_STRESS_NPROC}; + skip 'TEST_STRESS_NPROC= not set', 1 unless $nproc; + my $nr = $ENV{TEST_STRESS_NR} // 10000; + diag "TEST_STRESS_NPROC=$nproc TEST_STRESS_NR=$nr"; + require POSIX; + for my $n (1..$nproc) { + defined(my $pid = fork) or BAIL_OUT "fork: $!"; + if ($pid == 0) { + my $url = "imap://example.com/INBOX.$$"; + my $uidval = time; + eval { + my $itrk = PublicInbox::IMAPTracker->new($url); + for my $uid (1..$nr) { + $itrk->update_last($uidval, $uid); + my ($uv, $u) = $itrk->get_last; + } + }; + warn "E: $n $$ - $@\n" if $@; + POSIX::_exit($@ ? 1 : 0); + } + } + while (1) { + my $pid = waitpid(-1, 0); + last if $pid < 0; + is($?, 0, "$pid exited"); + } +} done_testing;