* [PATCH 1/4] t/msgmap: updates for Perl 5.12+ and copyrights
2024-07-23 21:28 [PATCH 0/4] fix ENOSPC on writes Eric Wong
@ 2024-07-23 21:28 ` Eric Wong
2024-07-23 21:28 ` [PATCH 2/4] msgmap: mid_insert: reraise on unexpected errors Eric Wong
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Eric Wong @ 2024-07-23 21:28 UTC (permalink / raw)
To: meta
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';
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 2/4] msgmap: mid_insert: reraise on unexpected errors
2024-07-23 21:28 [PATCH 0/4] fix ENOSPC on writes Eric Wong
2024-07-23 21:28 ` [PATCH 1/4] t/msgmap: updates for Perl 5.12+ and copyrights Eric Wong
@ 2024-07-23 21:28 ` Eric Wong
2024-07-24 7:22 ` [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
2024-07-23 21:28 ` Eric Wong
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Eric Wong @ 2024-07-23 21:28 UTC (permalink / raw)
To: meta
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;
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/4] msgmap: mid_insert: reraise on unexpected errors, [PATCH 3/4] t/v2writable: test ENOSPC from fast-import
2024-07-23 21:28 ` [PATCH 2/4] msgmap: mid_insert: reraise on unexpected errors Eric Wong
@ 2024-07-24 7:22 ` Štěpán Němec
2024-07-25 0:38 ` Eric Wong
0 siblings, 1 reply; 13+ messages in thread
From: Štěpán Němec @ 2024-07-24 7:22 UTC (permalink / raw)
To: Eric Wong; +Cc: meta
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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 2/4] msgmap: mid_insert: reraise on unexpected errors, [PATCH 3/4] t/v2writable: test ENOSPC from fast-import
2024-07-24 7:22 ` [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
@ 2024-07-25 0:38 ` Eric Wong
2024-07-25 6:42 ` Štěpán Němec
0 siblings, 1 reply; 13+ messages in thread
From: Eric Wong @ 2024-07-25 0:38 UTC (permalink / raw)
To: Štěpán Němec; +Cc: meta
Š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.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 2/4] msgmap: mid_insert: reraise on unexpected errors, [PATCH 3/4] t/v2writable: test ENOSPC from fast-import
2024-07-25 0:38 ` Eric Wong
@ 2024-07-25 6:42 ` Štěpán Němec
0 siblings, 0 replies; 13+ messages in thread
From: Štěpán Němec @ 2024-07-25 6:42 UTC (permalink / raw)
To: Eric Wong; +Cc: meta
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] t/v2writable: test ENOSPC from fast-import
2024-07-23 21:28 [PATCH 0/4] fix ENOSPC on writes Eric Wong
2024-07-23 21:28 ` [PATCH 1/4] t/msgmap: updates for Perl 5.12+ and copyrights Eric Wong
2024-07-23 21:28 ` [PATCH 2/4] msgmap: mid_insert: reraise on unexpected errors Eric Wong
@ 2024-07-23 21:28 ` Eric Wong
2024-07-23 21:28 ` [PATCH 4/4] t/v2writable: use 5.10.1 and autodie more Eric Wong
2024-07-26 21:31 ` [PATCH v2 0/4] fix ENOSPC on writes Eric Wong
4 siblings, 0 replies; 13+ messages in thread
From: Eric Wong @ 2024-07-23 21:28 UTC (permalink / raw)
To: meta
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;
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 4/4] t/v2writable: use 5.10.1 and autodie more
2024-07-23 21:28 [PATCH 0/4] fix ENOSPC on writes Eric Wong
` (2 preceding siblings ...)
2024-07-23 21:28 ` Eric Wong
@ 2024-07-23 21:28 ` Eric Wong
2024-07-26 21:31 ` [PATCH v2 0/4] fix ENOSPC on writes Eric Wong
4 siblings, 0 replies; 13+ messages in thread
From: Eric Wong @ 2024-07-23 21:28 UTC (permalink / raw)
To: meta
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');
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 0/4] fix ENOSPC on writes
2024-07-23 21:28 [PATCH 0/4] fix ENOSPC on writes Eric Wong
` (3 preceding siblings ...)
2024-07-23 21:28 ` [PATCH 4/4] t/v2writable: use 5.10.1 and autodie more Eric Wong
@ 2024-07-26 21:31 ` Eric Wong
2024-07-26 21:31 ` [PATCH v2 1/4] t/msgmap: updates for Perl 5.12+ and copyrights Eric Wong
` (3 more replies)
4 siblings, 4 replies; 13+ messages in thread
From: Eric Wong @ 2024-07-26 21:31 UTC (permalink / raw)
To: meta
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
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v2 1/4] t/msgmap: updates for Perl 5.12+ and copyrights
2024-07-26 21:31 ` [PATCH v2 0/4] fix ENOSPC on writes Eric Wong
@ 2024-07-26 21:31 ` Eric Wong
2024-07-26 21:31 ` [PATCH v2 2/4] msgmap: mid_insert: reraise on unexpected errors Eric Wong
` (2 subsequent siblings)
3 siblings, 0 replies; 13+ messages in thread
From: Eric Wong @ 2024-07-26 21:31 UTC (permalink / raw)
To: meta
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';
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 2/4] msgmap: mid_insert: reraise on unexpected errors
2024-07-26 21:31 ` [PATCH v2 0/4] fix ENOSPC on writes Eric Wong
2024-07-26 21:31 ` [PATCH v2 1/4] t/msgmap: updates for Perl 5.12+ and copyrights Eric Wong
@ 2024-07-26 21:31 ` Eric Wong
2024-07-26 21:31 ` [PATCH v2 3/4] t/v2writable: test ENOSPC from fast-import Eric Wong
2024-07-26 21:31 ` [PATCH v2 4/4] t/v2writable: use 5.10.1 and autodie more Eric Wong
3 siblings, 0 replies; 13+ messages in thread
From: Eric Wong @ 2024-07-26 21:31 UTC (permalink / raw)
To: meta; +Cc: Robin H . Johnson
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;
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 3/4] t/v2writable: test ENOSPC from fast-import
2024-07-26 21:31 ` [PATCH v2 0/4] fix ENOSPC on writes Eric Wong
2024-07-26 21:31 ` [PATCH v2 1/4] t/msgmap: updates for Perl 5.12+ and copyrights Eric Wong
2024-07-26 21:31 ` [PATCH v2 2/4] msgmap: mid_insert: reraise on unexpected errors Eric Wong
@ 2024-07-26 21:31 ` Eric Wong
2024-07-26 21:31 ` [PATCH v2 4/4] t/v2writable: use 5.10.1 and autodie more Eric Wong
3 siblings, 0 replies; 13+ messages in thread
From: Eric Wong @ 2024-07-26 21:31 UTC (permalink / raw)
To: meta
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;
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 4/4] t/v2writable: use 5.10.1 and autodie more
2024-07-26 21:31 ` [PATCH v2 0/4] fix ENOSPC on writes Eric Wong
` (2 preceding siblings ...)
2024-07-26 21:31 ` [PATCH v2 3/4] t/v2writable: test ENOSPC from fast-import Eric Wong
@ 2024-07-26 21:31 ` Eric Wong
3 siblings, 0 replies; 13+ messages in thread
From: Eric Wong @ 2024-07-26 21:31 UTC (permalink / raw)
To: meta
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');
^ permalink raw reply related [flat|nested] 13+ messages in thread