[PATCH 0/4] fix ENOSPC on writes

[PATCH 0/4] fix ENOSPC on writes

[Eric Wong]

2/4 is the actual fix to the bug reported by Robin, but the
others update tests and increase coverage for integration with
the v2 write path.

Eric Wong (4):
  t/msgmap: updates for Perl 5.12+ and copyrights
  msgmap: mid_insert: reraise on unexpected errors
  t/v2writable: test ENOSPC from fast-import
  t/v2writable: use 5.10.1 and autodie more

 lib/PublicInbox/Msgmap.pm |  6 +++-
 t/msgmap.t                | 25 ++++++++++---
 t/v2writable.t            | 76 +++++++++++++++++++++++++++++++--------
 3 files changed, 86 insertions(+), 21 deletions(-)

[PATCH 1/4] t/msgmap: updates for Perl 5.12+ and copyrights

[Eric Wong]

Test::More is redundant with TestCommon, and nothing here
conflicts with the unicode_strings feature in Perl v5.12.
---
 t/msgmap.t | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/t/msgmap.t b/t/msgmap.t
index a3b7200f..124d3b10 100644
--- a/t/msgmap.t
+++ b/t/msgmap.t
@@ -1,8 +1,7 @@
-# Copyright (C) 2015-2021 all contributors <meta@public-inbox.org>
+#!perl -w
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
-use strict;
-use warnings;
-use Test::More;
+use v5.12;
 use PublicInbox::TestCommon;
 require_mods('DBD::SQLite');
 use_ok 'PublicInbox::Msgmap';

[PATCH 2/4] msgmap: mid_insert: reraise on unexpected errors

[Eric Wong]

SQLITE_CONSTRAINT is the only SQLite error we really expect under
normal circumstances.  This avoids infinite loops when writing
to inboxes after hitting ENOSPC.

Reported-by: Robin H. Johnson <robbat2@orbis-terrarum.net>
Link: https://public-inbox.org/git/robbat2-20240722T060013-765939809Z@orbis-terrarum.net/
---
 lib/PublicInbox/Msgmap.pm |  6 +++++-
 t/msgmap.t                | 18 +++++++++++++++++-
 t/v2writable.t            | 18 +++++++++++++++++-
 3 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/lib/PublicInbox/Msgmap.pm b/lib/PublicInbox/Msgmap.pm
index cb4bb295..c4bc766d 100644
--- a/lib/PublicInbox/Msgmap.pm
+++ b/lib/PublicInbox/Msgmap.pm
@@ -12,6 +12,7 @@ use strict;
 use v5.10.1;
 use DBI;
 use DBD::SQLite;
+use DBD::SQLite::Constants qw(SQLITE_CONSTRAINT);
 use PublicInbox::Over;
 use Scalar::Util qw(blessed);
 
@@ -113,7 +114,10 @@ sub mid_insert {
 	my $sth = $self->{dbh}->prepare_cached(<<'');
 INSERT INTO msgmap (mid) VALUES (?)
 
-	return unless eval { $sth->execute($mid) };
+	unless (eval { $sth->execute($mid) }) {
+		return if $self->{dbh}->err == SQLITE_CONSTRAINT;
+		die $@;
+	}
 	my $num = $self->{dbh}->last_insert_id(undef, undef, 'msgmap', 'num');
 	$self->num_highwater($num) if defined($num);
 	$num;
diff --git a/t/msgmap.t b/t/msgmap.t
index 124d3b10..031854e0 100644
--- a/t/msgmap.t
+++ b/t/msgmap.t
@@ -3,6 +3,9 @@
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 use v5.12;
 use PublicInbox::TestCommon;
+use autodie;
+use Config;
+use PublicInbox::Spawn qw(popen_rd);
 require_mods('DBD::SQLite');
 use_ok 'PublicInbox::Msgmap';
 my ($tmpdir, $for_destroy) = tmpdir();
@@ -70,4 +73,17 @@ is(eval {
 	'ok'
 }, 'ok', 'atfork_* work on tmp_clone');
 
-done_testing();
+SKIP: {
+	my $strace = strace_inject;
+	open my $fh, '>', my $trace = "$tmpdir/trace.out";
+	my $rd = popen_rd([ $strace, '-p', $$, '-o', $trace,
+		'-e', 'inject=pwrite64:error=ENOSPC'], undef, { 2 => 1 });
+	$rd->poll_in(10) or die 'strace not ready';
+	is eval { $d->mid_insert('this-better-trigger-ENOSPC@error') },
+		undef, 'insert fails w/ ENOSPC';
+	like $@, qr/ disk is full/, 'set $@ for ENOSPC';
+	kill 'TERM', $rd->attached_pid;
+	$rd->close;
+}
+
+done_testing;
diff --git a/t/v2writable.t b/t/v2writable.t
index 1b7e9e7d..a062d1b3 100644
--- a/t/v2writable.t
+++ b/t/v2writable.t
@@ -6,6 +6,8 @@ use Test::More;
 use PublicInbox::Eml;
 use PublicInbox::ContentHash qw(content_digest content_hash);
 use PublicInbox::TestCommon;
+use PublicInbox::Spawn qw(popen_rd);
+use Config;
 use Cwd qw(abs_path);
 require_git(2.6);
 require_mods(qw(DBD::SQLite Xapian));
@@ -335,4 +337,18 @@ ok($@, 'V2Writable fails on non-existent dir');
 	is($mode, 0664, sprintf('0%03o', $mode).' is 0664');
 }
 
-done_testing();
+SKIP: {
+	my $strace = strace_inject;
+	my $eml = eml_load 't/plack-qp.eml';
+	open my $fh, '>', my $trace = "$inboxdir/trace.out";
+	my $rd = popen_rd([ $strace, '-p', $$, '-o', $trace,
+		'-e', 'inject=pwrite64:error=ENOSPC'], undef, { 2 => 1 });
+	$rd->poll_in(10) or die 'strace not ready';
+	ok ! eval { $im->add($eml) }, 'v2w->add fails on ENOSPC';
+	like $@, qr/ disk is full/, 'set $@ for ENOSPC';
+	$im->done;
+	kill 'TERM', $rd->attached_pid;
+	$rd->close;
+}
+
+done_testing;

[PATCH 3/4] t/v2writable: test ENOSPC from fast-import

[Eric Wong]

This also ensures we can recover after the ENOSPC condition
is resolved.
---
 t/v2writable.t | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/t/v2writable.t b/t/v2writable.t
index a062d1b3..93385456 100644
--- a/t/v2writable.t
+++ b/t/v2writable.t
@@ -9,6 +9,7 @@ use PublicInbox::TestCommon;
 use PublicInbox::Spawn qw(popen_rd);
 use Config;
 use Cwd qw(abs_path);
+use autodie qw(kill open read);
 require_git(2.6);
 require_mods(qw(DBD::SQLite Xapian));
 local $ENV{HOME} = abs_path('t');
@@ -340,15 +341,46 @@ ok($@, 'V2Writable fails on non-existent dir');
 SKIP: {
 	my $strace = strace_inject;
 	my $eml = eml_load 't/plack-qp.eml';
+	my $gfi_err = "$inboxdir/gfi.err";
 	open my $fh, '>', my $trace = "$inboxdir/trace.out";
 	my $rd = popen_rd([ $strace, '-p', $$, '-o', $trace,
 		'-e', 'inject=pwrite64:error=ENOSPC'], undef, { 2 => 1 });
 	$rd->poll_in(10) or die 'strace not ready';
-	ok ! eval { $im->add($eml) }, 'v2w->add fails on ENOSPC';
+	ok ! eval {
+		open my $olderr, '>&', \*STDERR;
+		open STDERR, '>>', $gfi_err;
+		$im->add($eml);
+		open STDERR, '>&', $olderr;
+	}, 'v2w->add fails on ENOSPC';
 	like $@, qr/ disk is full/, 'set $@ for ENOSPC';
 	$im->done;
 	kill 'TERM', $rd->attached_pid;
 	$rd->close;
+
+	$im->add($eml) or xbail 'cannot add message to start fast-import';
+	my $pid = $im->{im}->{io}->attached_pid or xbail 'no import pid';
+	open $fh, '>', $trace;
+
+	$rd = popen_rd([$strace, '-p', $pid, '-o', $trace,
+		'-e', 'inject=write:error=ENOSPC:when=1'],
+		undef, { 2 => 1 });
+	$rd->poll_in(10) or die 'strace not ready';
+	ok !eval { $im->done }, 'done fails with ENOSPC';
+	ok $@, '$@ set on ENOSPC';
+	kill 'TERM', $rd->attached_pid;
+
+	open $fh, '<', $gfi_err;
+	read $fh, my $errbuf, -s $fh;
+	like $errbuf, qr/fatal:/, 'fatal git error noted';
+	open $fh, '>', $gfi_err;
+
+	$rd->close;
+
+	$im->add($eml) or xbail '->add fail after fixing ENOSPC';
+	$im->done;
+	ok !$im->add($eml), '->add detects existing message';
+	$im->done;
+	is -s $fh, 0, 'nothing new fast-import stderr';
 }
 
 done_testing;

[PATCH 4/4] t/v2writable: use 5.10.1 and autodie more

[Eric Wong]

Switching to Perl v5.12 will require more review due to
unicode_strings, but 5.10.1 is an easy change and we can rely
more on autodie to simplify error checking.
---
 t/v2writable.t | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/t/v2writable.t b/t/v2writable.t
index 93385456..c016b049 100644
--- a/t/v2writable.t
+++ b/t/v2writable.t
@@ -1,15 +1,14 @@
-# Copyright (C) 2018-2021 all contributors <meta@public-inbox.org>
+#!perl -w
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
-use strict;
-use warnings;
-use Test::More;
+use v5.10.1; # FIXME: check 5.12 unicode_strings compat
 use PublicInbox::Eml;
 use PublicInbox::ContentHash qw(content_digest content_hash);
 use PublicInbox::TestCommon;
 use PublicInbox::Spawn qw(popen_rd);
 use Config;
 use Cwd qw(abs_path);
-use autodie qw(kill open read);
+use autodie qw(chmod close kill open read);
 require_git(2.6);
 require_mods(qw(DBD::SQLite Xapian));
 local $ENV{HOME} = abs_path('t');
@@ -157,16 +156,15 @@ SELECT COUNT(*) FROM over WHERE num > 0
 	my $out = "$inboxdir/stdout.log";
 	my $group = 'inbox.comp.test.v2writable';
 	my $pi_config = "$inboxdir/pi_config";
-	open my $fh, '>', $pi_config or die "open: $!\n";
-	print $fh <<EOF
+	open my $fh, '>', $pi_config;
+	print $fh <<EOF;
 [publicinbox "test-v2writable"]
 	inboxdir = $inboxdir
 	version = 2
 	address = test\@example.com
 	newsgroup = $group
 EOF
-	;
-	close $fh or die "close: $!\n";
+	close $fh;
 	my $sock = tcp_server();
 	my $len;
 	my $cmd = [ '-nntpd', '-W0', "--stdout=$out", "--stderr=$err" ];
@@ -320,15 +318,15 @@ ok($@, 'V2Writable fails on non-existent dir');
 	$v2w->{parallel} = 0;
 	$v2w->init_inbox(0);
 	my $alt = "$tmp->{inboxdir}/all.git/objects/info/alternates";
-	open my $fh, '>>', $alt or die $!;
-	print $fh "$inboxdir/all.git/objects\n" or die $!;
-	chmod(0664, $fh) or die "fchmod: $!";
-	close $fh or die $!;
-	open $fh, '<', $alt or die $!;
+	open my $fh, '>>', $alt;
+	print $fh "$inboxdir/all.git/objects\n";
+	chmod 0664, $fh;
+	close $fh;
+	open $fh, '<', $alt;
 	my $before = do { local $/; <$fh> };
 
 	ok($v2w->{mg}->add_epoch(3), 'init a new epoch');
-	open $fh, '<', $alt or die $!;
+	open $fh, '<', $alt;
 	my $after = do { local $/; <$fh> };
 	ok(index($after, $before) > 0,
 		'old contents preserved after adding epoch');

Re: [PATCH 2/4] msgmap: mid_insert: reraise on unexpected errors, [PATCH 3/4] t/v2writable: test ENOSPC from fast-import

[Štěpán Němec]

Just a few nits in the new test messages, if I may:

[2/4]:
On Tue, 23 Jul 2024 21:28:35 +0000
Eric Wong wrote:

> SQLITE_CONSTRAINT is the only SQLite error we really expect under
> normal circumstances.  This avoids infinite loops when writing
> to inboxes after hitting ENOSPC.
[...]
> diff --git a/t/msgmap.t b/t/msgmap.t
[...]
> @@ -70,4 +73,17 @@ is(eval {
>  	'ok'
>  }, 'ok', 'atfork_* work on tmp_clone');
>  
> -done_testing();
> +SKIP: {
> +	my $strace = strace_inject;
> +	open my $fh, '>', my $trace = "$tmpdir/trace.out";
> +	my $rd = popen_rd([ $strace, '-p', $$, '-o', $trace,
> +		'-e', 'inject=pwrite64:error=ENOSPC'], undef, { 2 => 1 });
> +	$rd->poll_in(10) or die 'strace not ready';
> +	is eval { $d->mid_insert('this-better-trigger-ENOSPC@error') },
> +		undef, 'insert fails w/ ENOSPC';
> +	like $@, qr/ disk is full/, 'set $@ for ENOSPC';
                                     ^^^^^^^^^^^^^^^^^
> +	kill 'TERM', $rd->attached_pid;
> +	$rd->close;
> +}
> +
> +done_testing;
> diff --git a/t/v2writable.t b/t/v2writable.t
[...]
> @@ -335,4 +337,18 @@ ok($@, 'V2Writable fails on non-existent dir');
>  	is($mode, 0664, sprintf('0%03o', $mode).' is 0664');
>  }
>  
> -done_testing();
> +SKIP: {
> +	my $strace = strace_inject;
> +	my $eml = eml_load 't/plack-qp.eml';
> +	open my $fh, '>', my $trace = "$inboxdir/trace.out";
> +	my $rd = popen_rd([ $strace, '-p', $$, '-o', $trace,
> +		'-e', 'inject=pwrite64:error=ENOSPC'], undef, { 2 => 1 });
> +	$rd->poll_in(10) or die 'strace not ready';
> +	ok ! eval { $im->add($eml) }, 'v2w->add fails on ENOSPC';
> +	like $@, qr/ disk is full/, 'set $@ for ENOSPC';
                                     ^^^^^^^^^^^^^^^^^

[3/4]:
On Tue, 23 Jul 2024 21:28:36 +0000
Eric Wong wrote:

> This also ensures we can recover after the ENOSPC condition
> is resolved.
> ---
>  t/v2writable.t | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/t/v2writable.t b/t/v2writable.t
> index a062d1b3..93385456 100644
> --- a/t/v2writable.t
> +++ b/t/v2writable.t
[...]
> @@ -340,15 +341,46 @@ ok($@, 'V2Writable fails on non-existent dir');
[...]
> +	$rd->poll_in(10) or die 'strace not ready';
> +	ok !eval { $im->done }, 'done fails with ENOSPC';
> +	ok $@, '$@ set on ENOSPC';
                ^^^^^^^^^^^^^^^^
If I'm correct in assuming that no semantic nuance is
intended between the 'set $@ for ENOSPC' and '$@ set on
ENOSPC' variants, I suggest unifying on the latter (the
unequivocal passive reads clearer than the
imperative-looking 'set $@').

[...]

> +	$im->add($eml) or xbail '->add fail after fixing ENOSPC';

s/fail/fails/ ?

> +	$im->done;
> +	ok !$im->add($eml), '->add detects existing message';
> +	$im->done;
> +	is -s $fh, 0, 'nothing new fast-import stderr';

s/new/new on/ ?

Thanks.

-- 
Štěpán

Re: [PATCH 2/4] msgmap: mid_insert: reraise on unexpected errors, [PATCH 3/4] t/v2writable: test ENOSPC from fast-import

[Eric Wong]

Štěpán Němec <stepnem@smrk.net> wrote:
> Just a few nits in the new test messages, if I may:
> 
> [2/4]:
> > +	like $@, qr/ disk is full/, 'set $@ for ENOSPC';
>                                      ^^^^^^^^^^^^^^^^^
> 
> [3/4]:
> > +	$rd->poll_in(10) or die 'strace not ready';
> > +	ok !eval { $im->done }, 'done fails with ENOSPC';
> > +	ok $@, '$@ set on ENOSPC';
>                 ^^^^^^^^^^^^^^^^
> If I'm correct in assuming that no semantic nuance is
> intended between the 'set $@ for ENOSPC' and '$@ set on
> ENOSPC' variants, I suggest unifying on the latter (the
> unequivocal passive reads clearer than the
> imperative-looking 'set $@').

The former ensures $@ matches qr/ disk is full/,
while the latter merely ensures $@ is set to a true value.

Arguably, the former could be '$@ reports ENOSPC'  *shrug*

> > +	$im->add($eml) or xbail '->add fail after fixing ENOSPC';
> 
> s/fail/fails/ ?
> 
> > +	$im->done;
> > +	ok !$im->add($eml), '->add detects existing message';
> > +	$im->done;
> > +	is -s $fh, 0, 'nothing new fast-import stderr';
> 
> s/new/new on/ ?

Will fix for v2.  Thanks.

Re: [PATCH 2/4] msgmap: mid_insert: reraise on unexpected errors, [PATCH 3/4] t/v2writable: test ENOSPC from fast-import

[Štěpán Němec]

On Thu, 25 Jul 2024 00:38:24 +0000
Eric Wong wrote:

> Štěpán Němec <stepnem@smrk.net> wrote:
>> Just a few nits in the new test messages, if I may:
>> 
>> [2/4]:
>> > +	like $@, qr/ disk is full/, 'set $@ for ENOSPC';
>>                                      ^^^^^^^^^^^^^^^^^
>> 
>> [3/4]:
>> > +	$rd->poll_in(10) or die 'strace not ready';
>> > +	ok !eval { $im->done }, 'done fails with ENOSPC';
>> > +	ok $@, '$@ set on ENOSPC';
>>                 ^^^^^^^^^^^^^^^^
>> If I'm correct in assuming that no semantic nuance is
>> intended between the 'set $@ for ENOSPC' and '$@ set on
>> ENOSPC' variants, I suggest unifying on the latter (the
>> unequivocal passive reads clearer than the
>> imperative-looking 'set $@').
>
> The former ensures $@ matches qr/ disk is full/,
> while the latter merely ensures $@ is set to a true value.
>
> Arguably, the former could be '$@ reports ENOSPC'  *shrug*

Indeed; 'set for' vs. 'set on' seems too subtle.

Thanks!

-- 
Štěpán

[PATCH v2 0/4] fix ENOSPC on writes

[Eric Wong]

v2 addresses grammar nits by Štěpán.

2/4 is the actual fix to the bug reported by Robin, but the
others update tests and increase coverage for integration with
the v2writable code path.

Eric Wong (4):
  t/msgmap: updates for Perl 5.12+ and copyrights
  msgmap: mid_insert: reraise on unexpected errors
  t/v2writable: test ENOSPC from fast-import
  t/v2writable: use 5.10.1 and autodie more

 lib/PublicInbox/Msgmap.pm |  6 +++-
 t/msgmap.t                | 25 ++++++++++---
 t/v2writable.t            | 76 +++++++++++++++++++++++++++++++--------
 3 files changed, 86 insertions(+), 21 deletions(-)

Range-diff against v1:
-:  -------- > 1:  c72c8a39 t/msgmap: updates for Perl 5.12+ and copyrights
1:  f1a53f30 ! 2:  c4ca4d41 msgmap: mid_insert: reraise on unexpected errors
    @@ t/msgmap.t: is(eval {
     +	$rd->poll_in(10) or die 'strace not ready';
     +	is eval { $d->mid_insert('this-better-trigger-ENOSPC@error') },
     +		undef, 'insert fails w/ ENOSPC';
    -+	like $@, qr/ disk is full/, 'set $@ for ENOSPC';
    ++	like $@, qr/ disk is full/, '$@ reports ENOSPC';
     +	kill 'TERM', $rd->attached_pid;
     +	$rd->close;
     +}
2:  6de9c07c ! 3:  4f59948b t/v2writable: test ENOSPC from fast-import
    @@ t/v2writable.t: ok($@, 'V2Writable fails on non-existent dir');
      		'-e', 'inject=pwrite64:error=ENOSPC'], undef, { 2 => 1 });
      	$rd->poll_in(10) or die 'strace not ready';
     -	ok ! eval { $im->add($eml) }, 'v2w->add fails on ENOSPC';
    +-	like $@, qr/ disk is full/, 'set $@ for ENOSPC';
     +	ok ! eval {
     +		open my $olderr, '>&', \*STDERR;
     +		open STDERR, '>>', $gfi_err;
     +		$im->add($eml);
     +		open STDERR, '>&', $olderr;
     +	}, 'v2w->add fails on ENOSPC';
    - 	like $@, qr/ disk is full/, 'set $@ for ENOSPC';
    ++	like $@, qr/ disk is full/, '$@ reports ENOSPC';
      	$im->done;
      	kill 'TERM', $rd->attached_pid;
      	$rd->close;
    @@ t/v2writable.t: ok($@, 'V2Writable fails on non-existent dir');
     +
     +	$rd->close;
     +
    -+	$im->add($eml) or xbail '->add fail after fixing ENOSPC';
    ++	$im->add($eml) or xbail '->add fails after fixing ENOSPC';
     +	$im->done;
     +	ok !$im->add($eml), '->add detects existing message';
     +	$im->done;
    -+	is -s $fh, 0, 'nothing new fast-import stderr';
    ++	is -s $fh, 0, 'nothing new in fast-import stderr';
      }
      
      done_testing;
3:  f7d286d4 = 4:  fba5535f t/v2writable: use 5.10.1 and autodie more

[PATCH v2 1/4] t/msgmap: updates for Perl 5.12+ and copyrights

[Eric Wong]

Test::More is redundant with TestCommon, and nothing here
conflicts with the unicode_strings feature in Perl v5.12.
---
 t/msgmap.t | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/t/msgmap.t b/t/msgmap.t
index a3b7200f..124d3b10 100644
--- a/t/msgmap.t
+++ b/t/msgmap.t
@@ -1,8 +1,7 @@
-# Copyright (C) 2015-2021 all contributors <meta@public-inbox.org>
+#!perl -w
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
-use strict;
-use warnings;
-use Test::More;
+use v5.12;
 use PublicInbox::TestCommon;
 require_mods('DBD::SQLite');
 use_ok 'PublicInbox::Msgmap';

[PATCH v2 2/4] msgmap: mid_insert: reraise on unexpected errors

[Eric Wong]

SQLITE_CONSTRAINT is the only SQLite error we really expect under
normal circumstances.  This avoids infinite loops when writing
to inboxes after hitting ENOSPC.

Reported-by: Robin H. Johnson <robbat2@orbis-terrarum.net>
Link: https://public-inbox.org/git/robbat2-20240722T060013-765939809Z@orbis-terrarum.net/
---
 lib/PublicInbox/Msgmap.pm |  6 +++++-
 t/msgmap.t                | 18 +++++++++++++++++-
 t/v2writable.t            | 18 +++++++++++++++++-
 3 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/lib/PublicInbox/Msgmap.pm b/lib/PublicInbox/Msgmap.pm
index cb4bb295..c4bc766d 100644
--- a/lib/PublicInbox/Msgmap.pm
+++ b/lib/PublicInbox/Msgmap.pm
@@ -12,6 +12,7 @@ use strict;
 use v5.10.1;
 use DBI;
 use DBD::SQLite;
+use DBD::SQLite::Constants qw(SQLITE_CONSTRAINT);
 use PublicInbox::Over;
 use Scalar::Util qw(blessed);
 
@@ -113,7 +114,10 @@ sub mid_insert {
 	my $sth = $self->{dbh}->prepare_cached(<<'');
 INSERT INTO msgmap (mid) VALUES (?)
 
-	return unless eval { $sth->execute($mid) };
+	unless (eval { $sth->execute($mid) }) {
+		return if $self->{dbh}->err == SQLITE_CONSTRAINT;
+		die $@;
+	}
 	my $num = $self->{dbh}->last_insert_id(undef, undef, 'msgmap', 'num');
 	$self->num_highwater($num) if defined($num);
 	$num;
diff --git a/t/msgmap.t b/t/msgmap.t
index 124d3b10..fb9c2d93 100644
--- a/t/msgmap.t
+++ b/t/msgmap.t
@@ -3,6 +3,9 @@
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 use v5.12;
 use PublicInbox::TestCommon;
+use autodie;
+use Config;
+use PublicInbox::Spawn qw(popen_rd);
 require_mods('DBD::SQLite');
 use_ok 'PublicInbox::Msgmap';
 my ($tmpdir, $for_destroy) = tmpdir();
@@ -70,4 +73,17 @@ is(eval {
 	'ok'
 }, 'ok', 'atfork_* work on tmp_clone');
 
-done_testing();
+SKIP: {
+	my $strace = strace_inject;
+	open my $fh, '>', my $trace = "$tmpdir/trace.out";
+	my $rd = popen_rd([ $strace, '-p', $$, '-o', $trace,
+		'-e', 'inject=pwrite64:error=ENOSPC'], undef, { 2 => 1 });
+	$rd->poll_in(10) or die 'strace not ready';
+	is eval { $d->mid_insert('this-better-trigger-ENOSPC@error') },
+		undef, 'insert fails w/ ENOSPC';
+	like $@, qr/ disk is full/, '$@ reports ENOSPC';
+	kill 'TERM', $rd->attached_pid;
+	$rd->close;
+}
+
+done_testing;
diff --git a/t/v2writable.t b/t/v2writable.t
index 1b7e9e7d..a062d1b3 100644
--- a/t/v2writable.t
+++ b/t/v2writable.t
@@ -6,6 +6,8 @@ use Test::More;
 use PublicInbox::Eml;
 use PublicInbox::ContentHash qw(content_digest content_hash);
 use PublicInbox::TestCommon;
+use PublicInbox::Spawn qw(popen_rd);
+use Config;
 use Cwd qw(abs_path);
 require_git(2.6);
 require_mods(qw(DBD::SQLite Xapian));
@@ -335,4 +337,18 @@ ok($@, 'V2Writable fails on non-existent dir');
 	is($mode, 0664, sprintf('0%03o', $mode).' is 0664');
 }
 
-done_testing();
+SKIP: {
+	my $strace = strace_inject;
+	my $eml = eml_load 't/plack-qp.eml';
+	open my $fh, '>', my $trace = "$inboxdir/trace.out";
+	my $rd = popen_rd([ $strace, '-p', $$, '-o', $trace,
+		'-e', 'inject=pwrite64:error=ENOSPC'], undef, { 2 => 1 });
+	$rd->poll_in(10) or die 'strace not ready';
+	ok ! eval { $im->add($eml) }, 'v2w->add fails on ENOSPC';
+	like $@, qr/ disk is full/, 'set $@ for ENOSPC';
+	$im->done;
+	kill 'TERM', $rd->attached_pid;
+	$rd->close;
+}
+
+done_testing;

[PATCH v2 3/4] t/v2writable: test ENOSPC from fast-import

[Eric Wong]

This also ensures we can recover after the ENOSPC condition
is resolved.
---
 t/v2writable.t | 36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/t/v2writable.t b/t/v2writable.t
index a062d1b3..0493f90e 100644
--- a/t/v2writable.t
+++ b/t/v2writable.t
@@ -9,6 +9,7 @@ use PublicInbox::TestCommon;
 use PublicInbox::Spawn qw(popen_rd);
 use Config;
 use Cwd qw(abs_path);
+use autodie qw(kill open read);
 require_git(2.6);
 require_mods(qw(DBD::SQLite Xapian));
 local $ENV{HOME} = abs_path('t');
@@ -340,15 +341,46 @@ ok($@, 'V2Writable fails on non-existent dir');
 SKIP: {
 	my $strace = strace_inject;
 	my $eml = eml_load 't/plack-qp.eml';
+	my $gfi_err = "$inboxdir/gfi.err";
 	open my $fh, '>', my $trace = "$inboxdir/trace.out";
 	my $rd = popen_rd([ $strace, '-p', $$, '-o', $trace,
 		'-e', 'inject=pwrite64:error=ENOSPC'], undef, { 2 => 1 });
 	$rd->poll_in(10) or die 'strace not ready';
-	ok ! eval { $im->add($eml) }, 'v2w->add fails on ENOSPC';
-	like $@, qr/ disk is full/, 'set $@ for ENOSPC';
+	ok ! eval {
+		open my $olderr, '>&', \*STDERR;
+		open STDERR, '>>', $gfi_err;
+		$im->add($eml);
+		open STDERR, '>&', $olderr;
+	}, 'v2w->add fails on ENOSPC';
+	like $@, qr/ disk is full/, '$@ reports ENOSPC';
 	$im->done;
 	kill 'TERM', $rd->attached_pid;
 	$rd->close;
+
+	$im->add($eml) or xbail 'cannot add message to start fast-import';
+	my $pid = $im->{im}->{io}->attached_pid or xbail 'no import pid';
+	open $fh, '>', $trace;
+
+	$rd = popen_rd([$strace, '-p', $pid, '-o', $trace,
+		'-e', 'inject=write:error=ENOSPC:when=1'],
+		undef, { 2 => 1 });
+	$rd->poll_in(10) or die 'strace not ready';
+	ok !eval { $im->done }, 'done fails with ENOSPC';
+	ok $@, '$@ set on ENOSPC';
+	kill 'TERM', $rd->attached_pid;
+
+	open $fh, '<', $gfi_err;
+	read $fh, my $errbuf, -s $fh;
+	like $errbuf, qr/fatal:/, 'fatal git error noted';
+	open $fh, '>', $gfi_err;
+
+	$rd->close;
+
+	$im->add($eml) or xbail '->add fails after fixing ENOSPC';
+	$im->done;
+	ok !$im->add($eml), '->add detects existing message';
+	$im->done;
+	is -s $fh, 0, 'nothing new in fast-import stderr';
 }
 
 done_testing;

[PATCH v2 4/4] t/v2writable: use 5.10.1 and autodie more

[Eric Wong]

Switching to Perl v5.12 will require more review due to
unicode_strings, but 5.10.1 is an easy change and we can rely
more on autodie to simplify error checking.
---
 t/v2writable.t | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/t/v2writable.t b/t/v2writable.t
index 0493f90e..496eba2c 100644
--- a/t/v2writable.t
+++ b/t/v2writable.t
@@ -1,15 +1,14 @@
-# Copyright (C) 2018-2021 all contributors <meta@public-inbox.org>
+#!perl -w
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
-use strict;
-use warnings;
-use Test::More;
+use v5.10.1; # FIXME: check 5.12 unicode_strings compat
 use PublicInbox::Eml;
 use PublicInbox::ContentHash qw(content_digest content_hash);
 use PublicInbox::TestCommon;
 use PublicInbox::Spawn qw(popen_rd);
 use Config;
 use Cwd qw(abs_path);
-use autodie qw(kill open read);
+use autodie qw(chmod close kill open read);
 require_git(2.6);
 require_mods(qw(DBD::SQLite Xapian));
 local $ENV{HOME} = abs_path('t');
@@ -157,16 +156,15 @@ SELECT COUNT(*) FROM over WHERE num > 0
 	my $out = "$inboxdir/stdout.log";
 	my $group = 'inbox.comp.test.v2writable';
 	my $pi_config = "$inboxdir/pi_config";
-	open my $fh, '>', $pi_config or die "open: $!\n";
-	print $fh <<EOF
+	open my $fh, '>', $pi_config;
+	print $fh <<EOF;
 [publicinbox "test-v2writable"]
 	inboxdir = $inboxdir
 	version = 2
 	address = test\@example.com
 	newsgroup = $group
 EOF
-	;
-	close $fh or die "close: $!\n";
+	close $fh;
 	my $sock = tcp_server();
 	my $len;
 	my $cmd = [ '-nntpd', '-W0', "--stdout=$out", "--stderr=$err" ];
@@ -320,15 +318,15 @@ ok($@, 'V2Writable fails on non-existent dir');
 	$v2w->{parallel} = 0;
 	$v2w->init_inbox(0);
 	my $alt = "$tmp->{inboxdir}/all.git/objects/info/alternates";
-	open my $fh, '>>', $alt or die $!;
-	print $fh "$inboxdir/all.git/objects\n" or die $!;
-	chmod(0664, $fh) or die "fchmod: $!";
-	close $fh or die $!;
-	open $fh, '<', $alt or die $!;
+	open my $fh, '>>', $alt;
+	print $fh "$inboxdir/all.git/objects\n";
+	chmod 0664, $fh;
+	close $fh;
+	open $fh, '<', $alt;
 	my $before = do { local $/; <$fh> };
 
 	ok($v2w->{mg}->add_epoch(3), 'init a new epoch');
-	open $fh, '<', $alt or die $!;
+	open $fh, '<', $alt;
 	my $after = do { local $/; <$fh> };
 	ok(index($after, $before) > 0,
 		'old contents preserved after adding epoch');