From: Eric Wong Date: Tue, 26 Aug 2025 19:50:49 +0000 (+0000) Subject: use Getopt::Long hashref for --no-fsync and --dangerous X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=057f6575c8181645005196c127ef557f61e8fe85;p=thirdparty%2Fpublic-inbox.git use Getopt::Long hashref for --no-fsync and --dangerous We can eliminate the {-no_fsync} and {-dangerous} fields by using the Getopt::Long hashref directly. This will make it easier to support additional CLI options (e.g. --cow). Our internal APIs now defaults to disabling fsync, however the CLI tools still override that internal default to enable fsync. Having our internals default to disabling fsync can slightly improve test performance, since they're the main users of our unstable internal API. --- diff --git a/lib/PublicInbox/CodeSearchIdx.pm b/lib/PublicInbox/CodeSearchIdx.pm index ddb598030..dbb0fe3a1 100644 --- a/lib/PublicInbox/CodeSearchIdx.pm +++ b/lib/PublicInbox/CodeSearchIdx.pm @@ -152,8 +152,6 @@ sub new { }, __PACKAGE__; $self->{nshard} = count_shards($self) || nproc_shards({nproc => $opt->{jobs}}); - $self->{-no_fsync} = 1 if !$opt->{fsync}; - $self->{-dangerous} = 1 if $opt->{dangerous}; $self; } diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm index cb8a992ae..911a900f5 100644 --- a/lib/PublicInbox/ExtSearchIdx.pm +++ b/lib/PublicInbox/ExtSearchIdx.pm @@ -94,8 +94,6 @@ sub new { die "invalid indexlevel=$l\n"; $self->{indexlevel} = $l; $self->{oidx} = PublicInbox::OverIdx->new($over_file, $opt); - $self->{-no_fsync} = 1 if !$opt->{fsync}; - $self->{-dangerous} = 1 if $opt->{dangerous}; $self } diff --git a/lib/PublicInbox/MiscIdx.pm b/lib/PublicInbox/MiscIdx.pm index e2ed9d111..257bd7a83 100644 --- a/lib/PublicInbox/MiscIdx.pm +++ b/lib/PublicInbox/MiscIdx.pm @@ -30,8 +30,9 @@ sub new { File::Path::mkpath($mi_dir); PublicInbox::Syscall::nodatacow_dir($mi_dir); my $flags = $PublicInbox::SearchIdx::DB_CREATE_OR_OPEN; - $flags |= $PublicInbox::SearchIdx::DB_NO_SYNC if $eidx->{-no_fsync}; - $flags |= $PublicInbox::SearchIdx::DB_DANGEROUS if $eidx->{-dangerous}; + my $opt = $eidx->{-opt}; + $flags |= $PublicInbox::SearchIdx::DB_NO_SYNC if !$opt->{fsync}; + $flags |= $PublicInbox::SearchIdx::DB_DANGEROUS if $opt->{dangerous}; $json //= PublicInbox::Config::json(); bless { mi_dir => $mi_dir, diff --git a/lib/PublicInbox/Msgmap.pm b/lib/PublicInbox/Msgmap.pm index 9ef87d789..fffaa7b57 100644 --- a/lib/PublicInbox/Msgmap.pm +++ b/lib/PublicInbox/Msgmap.pm @@ -17,18 +17,12 @@ use Scalar::Util qw(blessed); sub new_file { my ($class, $ibx, $opt) = @_; - my $f; my $rw = !!$opt; - if (blessed($ibx)) { - $f = $ibx->mm_file; - $rw = 2 if $rw && $ibx->{-no_fsync}; - } else { - $f = $ibx; - } + my $f = blessed($ibx) ? $ibx->mm_file : $ibx; return if !$rw && !-r $f; my $self = bless { filename => $f }, $class; - $self->{-opt} = $opt if $opt; # for WAL + $self->{-opt} = $opt if $opt; # for WAL and fsync my $dbh = $self->{dbh} = PublicInbox::Over::dbh_new($self, $rw); if ($rw) { $dbh->begin_work; diff --git a/lib/PublicInbox/SearchIdx.pm b/lib/PublicInbox/SearchIdx.pm index 6e05f9567..c0056ee5f 100644 --- a/lib/PublicInbox/SearchIdx.pm +++ b/lib/PublicInbox/SearchIdx.pm @@ -153,11 +153,11 @@ sub idx_acquire { PublicInbox::Syscall::nodatacow_dir($dir); # owner == self for CodeSearchIdx $self->{-set_has_threadid_once} = 1 if $owner != $self; - $flag |= $DB_DANGEROUS if $owner->{-dangerous}; + $flag |= $DB_DANGEROUS if $self->{-opt}->{dangerous}; } } return unless defined $flag; - $flag |= $DB_NO_SYNC if $owner->{-no_fsync}; + $flag |= $DB_NO_SYNC if !$self->{-opt}->{fsync}; my $xdb = eval { ($X->{WritableDatabase})->new($dir, $flag) }; croak "Failed opening $dir: $@" if $@; $self->{xdb} = $xdb; @@ -1196,6 +1196,7 @@ sub eidx_shard_new { my ($class, $eidx, $shard) = @_; my $self = bless { eidx => $eidx, + -opt => $eidx->{-opt}, # hmm... xpfx => $eidx->{xpfx}, indexlevel => $eidx->{indexlevel}, -skip_docdata => 1, diff --git a/lib/PublicInbox/SearchIdxShard.pm b/lib/PublicInbox/SearchIdxShard.pm index 108aaaeb7..8f340071f 100644 --- a/lib/PublicInbox/SearchIdxShard.pm +++ b/lib/PublicInbox/SearchIdxShard.pm @@ -12,7 +12,7 @@ use PublicInbox::Syscall qw($F_SETPIPE_SZ); sub new { my ($class, $v2w, $shard) = @_; # v2w may be ExtSearchIdx my $ibx = $v2w->{ibx}; - my $self = $ibx ? $class->SUPER::new($ibx, 1, $shard) + my $self = $ibx ? $class->SUPER::new($ibx, $v2w->{-opt}, $shard) : $class->eidx_shard_new($v2w, $shard); # create the DB before forking: $self->idx_acquire; diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm index 33bd9faba..7b173b611 100644 --- a/lib/PublicInbox/TestCommon.pm +++ b/lib/PublicInbox/TestCommon.pm @@ -857,7 +857,6 @@ sub setup_public_inboxes () { my $seen = 0; $cfg->each_inbox(sub { my ($ibx) = @_; - $ibx->{-no_fsync} = 1; my $im = PublicInbox::InboxWritable->new($ibx)->importer(0); my $V = $ibx->version; my @eml = (glob('t/*.eml'), 't/data/0001.patch'); @@ -944,7 +943,6 @@ sub create_inbox ($;@) { my $scope = $lk->lock_for_scope; my $pre_cb = delete $opt{pre_cb}; $pre_cb->($dir) if $pre_cb && $new; - $opt{-no_fsync} = 1; my $no_gc = delete $opt{-no_gc}; my $addr = $opt{address} // []; $opt{-primary_address} //= $addr->[0] // "$ident\@example.com"; diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm index 09e2777ce..bd5d9d320 100644 --- a/lib/PublicInbox/V2Writable.pm +++ b/lib/PublicInbox/V2Writable.pm @@ -70,8 +70,8 @@ sub new { # limit each git repo (epoch) to 1GB or so rotate_bytes => int((1024 * 1024 * 1024) / $PACKING_FACTOR), last_commit => [], # git epoch -> commit + -opt => $creat_opt // {}, }; - $self->{oidx}->{-no_fsync} = 1 if $v2ibx->{-no_fsync}; $self->{shards} = count_shards($self) || nproc_shards($creat_opt); bless $self, $class; } diff --git a/script/public-inbox-convert b/script/public-inbox-convert index cf87b4ebb..4ca8ecf28 100755 --- a/script/public-inbox-convert +++ b/script/public-inbox-convert @@ -84,8 +84,8 @@ $new->{inboxdir} = PublicInbox::Config::rel2abs_collapsed($new_dir); $new->{version} = 2; my $creat_opt = { nproc => $opt->{jobs} }; $creat_opt->{wal} = 1 if $opt->{wal}; +$creat_opt->{fsync} = $opt->{fsync}; $new = PublicInbox::InboxWritable->new($new, $creat_opt); -$new->{-no_fsync} = 1 if !$opt->{fsync}; my $v2w; sub link_or_copy ($$) { diff --git a/script/public-inbox-index b/script/public-inbox-index index b4965ec95..acdec3a9c 100755 --- a/script/public-inbox-index +++ b/script/public-inbox-index @@ -122,8 +122,6 @@ for my $ibx (@ibxs) { if ($opt->{compact} >= 2) { PublicInbox::Xapcmd::run($ibx, 'compact', $opt->{compact_opt}); } - $ibx->{-no_fsync} = 1 if !$opt->{fsync}; - $ibx->{-dangerous} = 1 if $opt->{dangerous}; $ibx->{-skip_docdata} //= $opt->{'skip-docdata'}; my $ibx_opt = $opt; diff --git a/script/public-inbox-purge b/script/public-inbox-purge index 0100cf48a..7649950ea 100755 --- a/script/public-inbox-purge +++ b/script/public-inbox-purge @@ -25,7 +25,7 @@ options: See public-inbox-purge(1) man page for full documentation. EOF -my $opt = { verbose => 1, all => 0, -min_inbox_version => 2 }; +my $opt = { verbose => 1, all => 0, -min_inbox_version => 2, fsync => 1 }; GetOptions($opt, @PublicInbox::AdminEdit::OPT, 'C=s@') or die $help; if ($opt->{help}) { print $help; exit 0 }; @@ -39,7 +39,7 @@ my $n_purged = 0; foreach my $ibx (@ibxs) { my $eml = PublicInbox::Eml->new($data); - my $v2w = PublicInbox::V2Writable->new($ibx); + my $v2w = PublicInbox::V2Writable->new($ibx, $opt); my $commits = $v2w->purge($eml) || []; diff --git a/scripts/import_slrnspool b/scripts/import_slrnspool index 81df6c2ef..7b0dbb365 100755 --- a/scripts/import_slrnspool +++ b/scripts/import_slrnspool @@ -38,7 +38,7 @@ my $git = $ibx->git; my $im; if ($ibx->version == 2) { require PublicInbox::V2Writable; - $im = PublicInbox::V2Writable->new($ibx); + $im = PublicInbox::V2Writable->new($ibx, { fsync => 1 }); $im->{parallel} = 0; # pointless to be parallel for a single message } else { $im = PublicInbox::Import->new($git, $ibx->{name}, diff --git a/t/extsearch.t b/t/extsearch.t index e97a735f6..5c9b6dbe0 100644 --- a/t/extsearch.t +++ b/t/extsearch.t @@ -266,7 +266,6 @@ if ('inject w/o indexing') { if ('reindex catches missed messages') { my $v2ibx = $cfg->lookup_name('v2test'); - $v2ibx->{-no_fsync} = 1; my $im = PublicInbox::InboxWritable->new($v2ibx)->importer(0); my $cmt_a = $v2ibx->mm->last_commit_xap($schema_version, 0); my $eml = eml_load('t/data/0001.patch'); @@ -337,7 +336,6 @@ if ('reindex catches missed messages') { if ('reindex catches content bifurcation') { use PublicInbox::MID qw(mids); my $v2ibx = $cfg->lookup_name('v2test'); - $v2ibx->{-no_fsync} = 1; my $im = PublicInbox::InboxWritable->new($v2ibx)->importer(0); my $eml = eml_load('t/data/message_embed.eml'); my $cmt_a = $v2ibx->mm->last_commit_xap($schema_version, 0); diff --git a/t/miscsearch.t b/t/miscsearch.t index ec837153d..a95cfe919 100644 --- a/t/miscsearch.t +++ b/t/miscsearch.t @@ -10,7 +10,7 @@ use_ok 'PublicInbox::MiscSearch'; use_ok 'PublicInbox::MiscIdx'; my ($tmp, $for_destroy) = tmpdir(); -my $eidx = { xpfx => "$tmp/eidx", -no_fsync => 1 }; # mock ExtSearchIdx +my $eidx = { xpfx => "$tmp/eidx" }; # mock ExtSearchIdx my $v1 = create_inbox 'hope', address => [ 'nope@example.com' ], indexlevel => 'basic', -no_gc => 1, sub { my ($im, $ibx) = @_; diff --git a/t/purge.t b/t/purge.t index a33cd3290..f4281c139 100644 --- a/t/purge.t +++ b/t/purge.t @@ -16,7 +16,6 @@ my $ibx = PublicInbox::Inbox->new({ inboxdir => $inboxdir, name => 'test-v2purge', version => 2, - -no_fsync => 1, -primary_address => 'test@example.com', indexlevel => 'basic', }); diff --git a/t/replace.t b/t/replace.t index a61c3ca06..b733c5562 100644 --- a/t/replace.t +++ b/t/replace.t @@ -20,7 +20,6 @@ sub test_replace ($$$) { inboxdir => "$tmpdir/testbox", name => $this, version => $v, - -no_fsync => 1, -primary_address => 'test@example.com', indexlevel => $level, }); diff --git a/t/v1reindex.t b/t/v1reindex.t index 5c00ca919..364331940 100644 --- a/t/v1reindex.t +++ b/t/v1reindex.t @@ -18,7 +18,6 @@ my $ibx_config = { name => 'test-v1reindex', -primary_address => 'test@example.com', indexlevel => 'full', - -no_fsync => 1, }; my $mime = PublicInbox::Eml->new(<<'EOF'); From: a@example.com diff --git a/t/v2-add-remove-add.t b/t/v2-add-remove-add.t index 070b1ac1e..f8ca2c210 100644 --- a/t/v2-add-remove-add.t +++ b/t/v2-add-remove-add.t @@ -13,7 +13,6 @@ my $ibx = { inboxdir => "$inboxdir/v2", name => 'test-v2writable', version => 2, - -no_fsync => 1, -primary_address => 'test@example.com', }; $ibx = PublicInbox::Inbox->new($ibx); diff --git a/t/v2mirror.t b/t/v2mirror.t index 9d8ba6277..8cda0a058 100644 --- a/t/v2mirror.t +++ b/t/v2mirror.t @@ -37,7 +37,6 @@ my $cfg = PublicInbox::Config->new($pi_config); my $ibx = $cfg->lookup('test@example.com'); ok($ibx, 'inbox found'); $ibx->{version} = 2; -$ibx->{-no_fsync} = 1; my $v2w = PublicInbox::V2Writable->new($ibx, { nproc => 1 }); ok $v2w, 'v2w loaded'; $v2w->{parallel} = 0; diff --git a/t/v2reindex.t b/t/v2reindex.t index 0e018481e..f830ae9b3 100644 --- a/t/v2reindex.t +++ b/t/v2reindex.t @@ -15,7 +15,6 @@ my $ibx_config = { version => 2, -primary_address => 'test@example.com', indexlevel => 'full', - -no_fsync => 1, }; my $agpl = do { open my $fh, '<', 'COPYING' or die "can't open COPYING: $!"; diff --git a/t/v2writable.t b/t/v2writable.t index 242088f96..a1593d95e 100644 --- a/t/v2writable.t +++ b/t/v2writable.t @@ -19,7 +19,6 @@ my $ibx = { inboxdir => $inboxdir, name => 'test-v2writable', version => 2, - -no_fsync => 1, -primary_address => 'test@example.com', }; $ibx = PublicInbox::Inbox->new($ibx); diff --git a/t/watch_filter_rubylang.t b/t/watch_filter_rubylang.t index 81f6b00e1..dee208849 100644 --- a/t/watch_filter_rubylang.t +++ b/t/watch_filter_rubylang.t @@ -67,7 +67,6 @@ EOF watchspam = maildir:$spamdir EOM my $ibx = $cfg->lookup_name($v); - $ibx->{-no_fsync} = 1; ok($ibx, 'found inbox by name'); my $w = PublicInbox::Watch->new($cfg); @@ -100,7 +99,6 @@ EOM $cfg = PublicInbox::Config->new($cfg->{-f}); $ibx = $cfg->lookup_name($v); - $ibx->{-no_fsync} = 1; is($ibx->search->reopen->mset('b:spam')->size, 0, 'spam removed'); is_deeply [], [ grep !/^#/, @warn ], 'no warnings'; diff --git a/t/watch_maildir_v2.t b/t/watch_maildir_v2.t index fa86f7bf8..bdc73bf38 100644 --- a/t/watch_maildir_v2.t +++ b/t/watch_maildir_v2.t @@ -47,7 +47,6 @@ EOF my $cfg = cfg_new $tmpdir, $orig; my $ibx = $cfg->lookup_name('test'); ok($ibx, 'found inbox by name'); -$ibx->{-no_fsync} = 1; PublicInbox::Watch->new($cfg)->scan('full'); my $total = scalar @{$ibx->over->recent}; diff --git a/xt/create-many-inboxes.t b/xt/create-many-inboxes.t index 3d8932b7e..5cd9f448b 100644 --- a/xt/create-many-inboxes.t +++ b/xt/create-many-inboxes.t @@ -40,7 +40,6 @@ my $v2_init_add = sub { address => [ "test-$i\@example.com" ], url => [ "//example.com/test-$i" ], version => 2, - -no_fsync => 1, }); $ibx->{indexlevel} = $indexlevel if $level_cfg ne ''; my $entry = <