]> git.ipfire.org Git - thirdparty/public-inbox.git/log
thirdparty/public-inbox.git
5 months agosearchidx: don't index Base-85 w/ CRLF endings
Eric Wong [Wed, 19 Feb 2025 10:10:32 +0000 (10:10 +0000)] 
searchidx: don't index Base-85 w/ CRLF endings

I encountered a false positive search result from a CRLF message
with a Base-85 patch in it.  It turns out our Base-85 filtering
code didn't account for the possibility of "\r" showing up in
patch messages, so just ignore all trailing spaces (not just
horizontal spaces) in index_diff().

While we're at it, exclude horizontal whitespace and CR
consistently from Base-85-looking quoted text in
index_body_text(), too, since I'm sure there's messages with
CRCRLF in the wild, too...

5 months agocfgwr: fix non-libgit2 case
Eric Wong [Wed, 19 Feb 2025 12:39:26 +0000 (12:39 +0000)] 
cfgwr: fix non-libgit2 case

libgit2 isn't installed on all my test machines, and the
addition of libgit2 support was careless in that it failed to
test the non-libgit2 case thoroughly :x

Fixes: a8073f6c (lg2: use cfgwr_commit to write to configs using libgit2, 2025-02-15)
5 months agolg2: use cfgwr_commit to write to configs using libgit2
Eric Wong [Sat, 15 Feb 2025 11:10:11 +0000 (11:10 +0000)] 
lg2: use cfgwr_commit to write to configs using libgit2

libgit2 allows us to avoid process spawning overhead when
writing to the config file.  While the performance difference is
unlikely to matter in real-world use, it should improve
maintainability of tests by allowing us to write tests with
`public-inbox-init' with a smaller performance penalty (as
opposed to using `PublicInbox::IO::write_file' to setup
configs).

5 months agorename Gcf2 -> Lg2 for general libgit2 use
Eric Wong [Sat, 15 Feb 2025 11:10:10 +0000 (11:10 +0000)] 
rename Gcf2 -> Lg2 for general libgit2 use

We'll be exploring libgit2 for writing (and possibly reading) config
files, so the `gcf2' (git-cat-file-2) name isn't appropriate
anymore for the package name.  We'll still keep `gcf2_' for
subroutine names related to git-cat-file-like behavior.

5 months agocfgwrite: new module to batch commit writes
Eric Wong [Sat, 15 Feb 2025 11:10:09 +0000 (11:10 +0000)] 
cfgwrite: new module to batch commit writes

Hoisting git config writing code into a separate module will
make it easier to use libgit2 to avoid process spawning overhead
in a future change.

5 months agot/watch_v1_v2_mix_no_modify: load PublicInbox::Config
Eric Wong [Sat, 15 Feb 2025 11:10:08 +0000 (11:10 +0000)] 
t/watch_v1_v2_mix_no_modify: load PublicInbox::Config

For testing with TEST_RUN_MODE=0 (which spawns new processes
instead of reusing them), we can't rely on packages being loaded
by subprocesses influencing the main Perl process.  Thus we must
load PublicInbox::Config explicitly to prevent failures with
this run mode.

5 months agotest_common: key2sub: detect syntax errors early
Eric Wong [Sat, 15 Feb 2025 11:10:07 +0000 (11:10 +0000)] 
test_common: key2sub: detect syntax errors early

Instead of attempting to use an `undef' $sub as a coderef;
detect compile errors early and die immediately if `eval'
raises an exception.

5 months agoimport: clobber $? if global init.defaultBranch fails
Eric Wong [Sat, 15 Feb 2025 11:10:06 +0000 (11:10 +0000)] 
import: clobber $? if global init.defaultBranch fails

We don't want $? influencing further error checking downstream,
such as thw _wq_done_wait call in public-inbox-clone when
git-config(1) invocations which set $?=0 are replaced with
in-process libgit2 config function calls.

5 months agoedit: use autodie and favor flush over autoflush
Eric Wong [Sat, 15 Feb 2025 11:10:05 +0000 (11:10 +0000)] 
edit: use autodie and favor flush over autoflush

Using a single ->flush reduces iops for the (default) non-raw
case and autodie for `open' makes error messages more
consistent.

5 months agofavor run_wait() over CORE::system()
Eric Wong [Sat, 15 Feb 2025 11:10:04 +0000 (11:10 +0000)] 
favor run_wait() over CORE::system()

run_wait() can use vfork(2) which saves memory in large
processes, such as public-inbox-extindex when dealing with giant
messages.  While vfork is unlikely to matter for real-world uses
of public-inbox-edit, PublicInbox::Spawn is a sunk cost treewide
our `make check-run' test target avoids spawning new Perl
processes for most things in script/*, so there can be a small
savings for testing.

5 months agoimap_searchqp: avoid occasional P::RD hint spew
Eric Wong [Fri, 14 Feb 2025 09:38:55 +0000 (09:38 +0000)] 
imap_searchqp: avoid occasional P::RD hint spew

It turns out Inline::C::Parser::RecDescent increments $::RD_HINT
when Inline::C rebuilds are necessary, thus causing verbose log
spew on bogus IMAP search queries in our own use of
Parse::RecDescent.  So, local-ize $::RD_HINT as we do with other
$::RD_* vars to avoid Inline::C influencing our IMAP query
parsing.

Triggering the increment of $::RD_HINT via
Inline::C::Parser::RecDescent may be achieved by forcing an
Inline::C rebuild, such as clearing the contents of
~/.cache/public-inbox/inline-c/* or
$ENV{PERL_INLINE_DIRECTORY}/* (but leaving one of those
top-level directories to trigger Inline::C rebuilds.  This bug
mainly manifested in t/imap_searchqp.t under the `make
check-run' test target which reuses existing processes to speed
up tests, though it could manifest in real-world -imapd (and
-netd) usage if the process spawning the daemon built Inline::C
code.

5 months agodaemon: make xap_helper socket non-blocking
Eric Wong [Tue, 11 Feb 2025 03:55:36 +0000 (03:55 +0000)] 
daemon: make xap_helper socket non-blocking

A non-blocking socket for public-facing searches is ideal on
overloaded servers which receive more searches than it can
handle in a short time span.  A non-blocking socket here
prevents a sendmsg(2) call from blocking the main event loop
and ensures non-search requests can still be processed even
if all xap_helper subprocesses are overwhelmed.

$search_xh_pid is hoisted out to TestCommon for use with the new
slow-xh-search test and renamed $find_xh_pid to avoid confusion
with Xapian search functionality.  We'll also drop a redundant
call to find the xap_helper PID in psgi_v2.t while transitioning
to $find_xh_pid.

5 months agowww: mbox: break out of loop on exceptions
Eric Wong [Tue, 11 Feb 2025 03:55:35 +0000 (03:55 +0000)] 
www: mbox: break out of loop on exceptions

In case async_mset fails due to resource limitations, ensure we
stop trying to make expensive search queries ASAP.

5 months agosearch: async_mset: always run callback on exceptions
Eric Wong [Tue, 11 Feb 2025 03:55:34 +0000 (03:55 +0000)] 
search: async_mset: always run callback on exceptions

To produce consistent error behavior, ensure we always run
the user-supplied callback on exceptions when using in-process
Xapian (as opposed to the external xap_helper process).

5 months agogit_http_backend: input_prepare: die on unrecoverable errors
Eric Wong [Mon, 10 Feb 2025 21:09:34 +0000 (21:09 +0000)] 
git_http_backend: input_prepare: die on unrecoverable errors

We shouldn't be guarding against errors at this level to return
500 errors, but rather we can rely on existing eval wrappers in
the stack (e.g. Plack::Util::run_app and Qspawn->parse_hdr_done)
to capture errors and return the HTTP 500 status.

5 months agolei_mirror: use write_file to append configs
Eric Wong [Mon, 10 Feb 2025 21:09:33 +0000 (21:09 +0000)] 
lei_mirror: use write_file to append configs

It saves us a few lines of code and reduces unnecessary
error checking since the close done by write_file aleady
does autodie for error checking.

5 months agomsgmap: use v5.12
Eric Wong [Mon, 10 Feb 2025 21:09:32 +0000 (21:09 +0000)] 
msgmap: use v5.12

No unicode_string dependencies, here, so we can use v5.12 and
perhaps to speed up loading one day if everything drops `strict'
in favor of v5.12 (or higher).

5 months agoinit: reduce git-config execve for idempotent cases
Eric Wong [Mon, 10 Feb 2025 21:09:31 +0000 (21:09 +0000)] 
init: reduce git-config execve for idempotent cases

This probably saves us a few cycles in some cases since spawning
new processes is expensive.

5 months agomulti_git: remove redundant ops
Eric Wong [Mon, 10 Feb 2025 21:09:30 +0000 (21:09 +0000)] 
multi_git: remove redundant ops

read_all already returns an array in array context so the
`split' op is unnecessary, and we don't need to use
`join("", ...)' on arrays that are destined for `print'
or `say'.

5 months agodevel/sysdefs-list: explain why we don't autodie, here
Eric Wong [Mon, 10 Feb 2025 21:09:29 +0000 (21:09 +0000)] 
devel/sysdefs-list: explain why we don't autodie, here

Some distros split out core modules like autodie, and
sysdefs-list has higher portability requirements than the rest
of our code since it's used for uncommon platforms.

5 months agoconfig: handle limiter `max' knob in ->setup_limiter
Eric Wong [Sat, 8 Feb 2025 03:26:39 +0000 (03:26 +0000)] 
config: handle limiter `max' knob in ->setup_limiter

Handling all config knobs in the same sub is more logical and
reduces the size of the frequently-loaded PublicInbox::Config
package.

5 months agoqspawn: drop redundant check against limiter->{max}
Eric Wong [Sat, 8 Feb 2025 03:26:38 +0000 (03:26 +0000)] 
qspawn: drop redundant check against limiter->{max}

We only need to guard against excessive parallelism on entry
in one place during Qspawn->start.  Redundant guards are
confusing and make bugs harder-to-spot.

5 months agoviewvcs: -codeblob limiter w/ depth for solver
Eric Wong [Sat, 8 Feb 2025 03:26:37 +0000 (03:26 +0000)] 
viewvcs: -codeblob limiter w/ depth for solver

By adding the `publicinboxlimiter.<name>.depth' parameter for
all limiters, we can have queue overflow detection and reject
requests with HTTP 503 error codes for all limiter uses, not
just the custom queue in ViewVCS as before.  Then we can just
use a normal limiter in ViewVCS and get rid of the old custom
queue.

5 months agoqspawn: use limiter->new default
Eric Wong [Sat, 8 Feb 2025 03:26:36 +0000 (03:26 +0000)] 
qspawn: use limiter->new default

No need to hardcode `32' here since PublicInbox::Limiter already
uses that value already (and includes a helpful comment about
using the same value as git-daemon).

5 months agodaemon: check connections before solving codeblobs
Eric Wong [Sat, 8 Feb 2025 03:26:35 +0000 (03:26 +0000)] 
daemon: check connections before solving codeblobs

The /$MSGID/s/ codeblob reconstruction endpoint is extremely
expensive and one of the few which do a large amount of upfront
processing before being able to write to an HTTP client and
detect if they're still alive (unlike other expensive
endpoints).

For TCP connections, use the TCP_INFO via getsockopt(2) if
supported or attempt to use poll(2) + the FIONREAD ioctl(2) to
check the connection before possibly invoking
git-(apply|ls-files|update-index) dozens or even hundreds of
times to reconstruct a blob.

5 months agogit_http_backend: fix 32 default connection limit
Eric Wong [Sat, 8 Feb 2025 03:26:34 +0000 (03:26 +0000)] 
git_http_backend: fix 32 default connection limit

Using any configurable limiter actually defaults to `1', which
isn't intended for `-httpbackend'.  So add a $max_default
parameter to ensure we can set a per-limiter default value for
default limiters such as `-httpbackend'.

Fixes: 0a083241 (git_http_backend: use default limiter from Qspawn, 2025-01-28)
5 months agosyscall: `use bytes' throughout
Eric Wong [Sat, 8 Feb 2025 03:26:33 +0000 (03:26 +0000)] 
syscall: `use bytes' throughout

All *nix syscalls operate on the byte-level with no knowledge of
encodings, so ensure all `substr' and `length' calculations use
the bytes(3perl) pragma.

5 months agoRevert "daemon: check connections WIP"
Eric Wong [Tue, 4 Feb 2025 20:33:50 +0000 (20:33 +0000)] 
Revert "daemon: check connections WIP"

Accidentally pushed out a WIP change since I couldn't access my
normal VM for pushing :x
This reverts commit 10394e508575fd7cc78d0255d39b04c833efcdbb.

5 months agogit_http_backend: use default limiter from Qspawn
Eric Wong [Tue, 28 Jan 2025 08:31:13 +0000 (08:31 +0000)] 
git_http_backend: use default limiter from Qspawn

There's no reason for us to have another default limiter object
when the one in Qspawn already exists.

5 months agowww: configurable "-httpbackend" named limiter
Eric Wong [Tue, 28 Jan 2025 08:31:12 +0000 (08:31 +0000)] 
www: configurable "-httpbackend" named limiter

The default limiter being hard-coded to 32 is excessive for
smaller systems.  With dozens or hundreds of inboxes,
configuring all inboxes to point to a user-defined limiter
is tedious, as well.  So give WWW admins the opportunity
to configure the limiter for all git-http-backend(1) processes
with one `publicinbox.-httpbackend.max' knob.

5 months agolimiter: ignore unparseable rlimit
Eric Wong [Tue, 28 Jan 2025 08:31:11 +0000 (08:31 +0000)] 
limiter: ignore unparseable rlimit

Instead of blindly accepting whatever bogus values the config
gave us, ignore it and use the `W:' prefix to be consistent
with the rest of our codebase.

6 months agodaemon: check connections WIP
Eric Wong [Tue, 28 Jan 2025 08:10:06 +0000 (08:10 +0000)] 
daemon: check connections WIP

6 months agoviewvcs: handle exceptions in on_destroy cb
Eric Wong [Fri, 24 Jan 2025 09:18:56 +0000 (09:18 +0000)] 
viewvcs: handle exceptions in on_destroy cb

We need to account for File::Temp->newdir or autodie::open
croaking on us in case the system is out of space or memory.
These resource errors are already handled in our normal code
path.  However, this on_destroy codepath can be triggered
when the solver request needs to be queued due to business.
Thus we explicitly call the {-wcb} to ensure the socket
isn't forgotten.

6 months agoemergency: remove needless $! clobber
Eric Wong [Sat, 18 Jan 2025 01:26:16 +0000 (01:26 +0000)] 
emergency: remove needless $! clobber

We don't check $! unless `sysopen' fails, and `sysopen' will set
$! on failure so there's no need to undef $! ourselves.

6 months agomda: use read_all for error handling
Eric Wong [Sat, 18 Jan 2025 01:26:15 +0000 (01:26 +0000)] 
mda: use read_all for error handling

We already imported PublicInbox::IO for this script and read_all
provides more safety in case default PerlIO semantics change for
read-in-full behavior.

6 months agov2writable: remove outdated FIXME comment and assertions
Eric Wong [Sat, 18 Jan 2025 01:26:14 +0000 (01:26 +0000)] 
v2writable: remove outdated FIXME comment and assertions

The lei/store bug was resolved and thus the comments and
assertions are no longer necessary.

Followup-to: 99fc3d76 (v2writable: done: force synchronous awaitpid, 2024-11-19)
6 months agoconfig: config_fh_parse: hardcode FS/RS
Eric Wong [Sat, 18 Jan 2025 01:26:13 +0000 (01:26 +0000)] 
config: config_fh_parse: hardcode FS/RS

We no longer need the flexibility to handle non-NUL-delimited
output from `git config -l', so simplify callers and allow
a theoretically sufficiently-advanced Perl implementation
optimize more easily.

Followup-to: 21146412 (config: drop scalar ref support from internal API, 2023-09-24)
6 months agogit: rely on autodie for sysseek/sysread/truncate
Eric Wong [Sat, 18 Jan 2025 01:26:12 +0000 (01:26 +0000)] 
git: rely on autodie for sysseek/sysread/truncate

Error messages are more consistent with autodie.  We'll also
drop autodie::read since it's no longer in use for this package.

6 months agotreewide: use autodie for seek+sysseek
Eric Wong [Sat, 18 Jan 2025 01:26:11 +0000 (01:26 +0000)] 
treewide: use autodie for seek+sysseek

The underlying lseek(2) syscall won't fail due to HW errors,
only due to usage errors (ESPIPE, EINVAL); so don't waste
code on error checking ourselves and just let autodie check
things during development.

6 months agolei_xsearch: use autodie for `pipe' ops
Eric Wong [Sat, 18 Jan 2025 01:26:10 +0000 (01:26 +0000)] 
lei_xsearch: use autodie for `pipe' ops

We already use autodie in this module and we can have more
consistent error messages this way.

6 months agotreewide: replace redundant `;;' with `;'
Eric Wong [Sat, 18 Jan 2025 01:26:09 +0000 (01:26 +0000)] 
treewide: replace redundant `;;' with `;'

Wonky USB keyboard adapter or too much coffee sometimes makes me
type too many semi-colons (and other characters) :x

6 months agolei_saved_search: drop needless comparisons and `next'
Eric Wong [Sat, 18 Jan 2025 01:26:08 +0000 (01:26 +0000)] 
lei_saved_search: drop needless comparisons and `next'

The grep(!/\A\.\.?\z/, ...) op already filters out the `.' and
`..' entries from `readdir'.

6 months agoinit: move --skip-epoch handling to {-creat_opt}
Eric Wong [Sat, 18 Jan 2025 01:26:07 +0000 (01:26 +0000)] 
init: move --skip-epoch handling to {-creat_opt}

Epoch skipping only makes sense at inbox creation, thus we'll
bundle inbox creation options together to reduce potentionally
confusing positional parameters.

6 months agoinit: move --skip-artnum handling to {-creat_opt}
Eric Wong [Sat, 18 Jan 2025 01:26:06 +0000 (01:26 +0000)] 
init: move --skip-artnum handling to {-creat_opt}

It makes more sense to have inbox creation options bundled
together and reduces the amount of potentially confusing
positional parameters we must pass around.

6 months agoinbox_writable: match v1 and v2 init semantics
Eric Wong [Sat, 18 Jan 2025 01:26:05 +0000 (01:26 +0000)] 
inbox_writable: match v1 and v2 init semantics

More consistent code between v1 and v2 will make maintenance
easier going forward.

6 months agov2writable: simplify ->new by reducing arg flexibility
Eric Wong [Sat, 18 Jan 2025 01:26:04 +0000 (01:26 +0000)] 
v2writable: simplify ->new by reducing arg flexibility

We can update callers easily enough for internal-only APIs,
so there's no need to deal with unwarranted flexibility for
V2Writable->new.

6 months agoinbox_writable: use autodie::open
Eric Wong [Sat, 18 Jan 2025 01:26:03 +0000 (01:26 +0000)] 
inbox_writable: use autodie::open

autodie should give us more consistent error reporting going
forward.

6 months agodoc: add an example of acceptable marketing
Eric Wong [Sat, 18 Jan 2025 01:50:18 +0000 (01:50 +0000)] 
doc: add an example of acceptable marketing

Presenting specific solutions to random comments is probably
acceptable since it's potentially helpful to some people;
but disclaimers must be present to lower expectations.

6 months agosearch_query: fix warnings on bogus `o=' query param
Eric Wong [Sun, 19 Jan 2025 04:40:58 +0000 (04:40 +0000)] 
search_query: fix warnings on bogus `o=' query param

Users may specify non-numeric values for the `o' parameter
which won't be captured.  Avoid uninitialized variable warnings
by assuming `0' when no numeric value is specified.

6 months agoconfig: force absolute path when `-C /' (chdir) is used
Eric Wong [Thu, 16 Jan 2025 03:28:51 +0000 (03:28 +0000)] 
config: force absolute path when `-C /' (chdir) is used

For rare cases where we rely on `-c' to override temporary
options, we also chdir(2) to `/' to avoid hitting a users'
$worktree/.git/config.  Thus, the config file must be given as
as an absolute path.  While the majority of or code assumes
absolute paths are used for config files, it's possible we end
up on some strange setups where $ENV{HOME} is a relative path.

6 months ago(ext)index: eliminate $sync entirely
Eric Wong [Fri, 10 Jan 2025 23:21:00 +0000 (23:21 +0000)] 
(ext)index: eliminate $sync entirely

Finally, the elimination of this variable simplifies subroutine
calls hopefully makes object lifetimes easier to reason about.

6 months ago(ext)index: eliminate $sync->{ibx}
Eric Wong [Fri, 10 Jan 2025 23:20:59 +0000 (23:20 +0000)] 
(ext)index: eliminate $sync->{ibx}

Perhaps the trickiest field to eliminate from $sync {ibx}
due to the way -extindex handles cross-posted messages.  We
now eliminate the per-request $req->{ibx} for non-crossposted
messages and use local-ized $self->{ibx} directly.

6 months ago(ext)index: move {ranges} to $self
Eric Wong [Fri, 10 Jan 2025 23:20:14 +0000 (23:20 +0000)] 
(ext)index: move {ranges} to $self

Another field we can trivially `local'-ize in our quest to
eliminate the clumsy $sync structure.

6 months ago(ext)index: move {unindexed} to $self
Eric Wong [Fri, 10 Jan 2025 23:20:13 +0000 (23:20 +0000)] 
(ext)index: move {unindexed} to $self

As with most things that were formerly in $sync, we can just
rely on `local' to flatten the data structure graph and work
towards eliminating $sync.

6 months agov2writable: hoist out process_todo sub for extindex
Eric Wong [Fri, 10 Jan 2025 23:20:12 +0000 (23:20 +0000)] 
v2writable: hoist out process_todo sub for extindex

We can have consistent $self->{quit} detection this way and
eliminate an early return from index_todo.

6 months agov2writable: {in_unindex} moved to self
Eric Wong [Fri, 10 Jan 2025 23:20:11 +0000 (23:20 +0000)] 
v2writable: {in_unindex} moved to self

{in_unindex} is already local-ized for $sync, so moving it to
$self is trivial since we use ->async_wait_all to ensure
cat-file requests are done.

6 months ago(ext)index: move {todo} into $self
Eric Wong [Fri, 10 Jan 2025 23:20:10 +0000 (23:20 +0000)] 
(ext)index: move {todo} into $self

Another trivial field to `local'-ize and another step towards
eliminating the $sync structure.

6 months agoextindex: simplify data structures used for dedupe
Eric Wong [Fri, 10 Jan 2025 23:20:09 +0000 (23:20 +0000)] 
extindex: simplify data structures used for dedupe

We can stuff more into $self via `local' and eliminate the $per_mid
structure and extra lookups.

6 months agoextindex: move {dedupe_cull} to self
Eric Wong [Fri, 10 Jan 2025 23:20:08 +0000 (23:20 +0000)] 
extindex: move {dedupe_cull} to self

This allows us to eliminate the $per_mid->{sync} field used for
dedupe, as well.

6 months ago(ext)index: eliminate redundant $sync->{epoch_max}
Eric Wong [Fri, 10 Jan 2025 23:20:07 +0000 (23:20 +0000)] 
(ext)index: eliminate redundant $sync->{epoch_max}

We already had $self->{epoch_max}, and it's easy enough to
`local'-ize it to ensure it doesn't outlive its usefulness when
-extindex handles multiple inboxes.

6 months ago(ext)index: $sync->{unit} => $self->{unit}
Eric Wong [Fri, 10 Jan 2025 23:20:06 +0000 (23:20 +0000)] 
(ext)index: $sync->{unit} => $self->{unit}

Since we were using `local' anyways on the $sync hashref, it's a
trivial swap to move it into $self.

6 months agov2writable: eliminate $sync->{art_end}
Eric Wong [Fri, 10 Jan 2025 23:20:05 +0000 (23:20 +0000)] 
v2writable: eliminate $sync->{art_end}

Instead of creating a long-lived {art_end} entry or even
local-izing it in $self, pass the `$art_end' value on stack to
achieve symmetry with the existing `$arg_beg' arg in calls to
index_xap_step().

6 months agoextindex: eliminate repeated ->eidx_key method call
Eric Wong [Fri, 10 Jan 2025 23:19:04 +0000 (23:19 +0000)] 
extindex: eliminate repeated ->eidx_key method call

No need to make two relatively expensive `->' method calls when
one will do.

6 months ago(ext)index: eliminate most uses of `$sync->{ibx}'
Eric Wong [Fri, 10 Jan 2025 23:19:03 +0000 (23:19 +0000)] 
(ext)index: eliminate most uses of `$sync->{ibx}'

Yet another step towards eliminating the $sync structure.
There's still a few remaining dependencies due to cross-inbox
linkage but the goal is to reduce refcount traffic for the
majority of Inbox object references.  This field in $sync will
be completely eliminated in the next few commits.

6 months agov2writable: move {mm_tmp} to $self
Eric Wong [Fri, 10 Jan 2025 23:19:02 +0000 (23:19 +0000)] 
v2writable: move {mm_tmp} to $self

Another variable we can easily local-ize to limit scope and
reduce refcount traffic from copying into the per-cat_async $arg
structure.

6 months agosearchidx: eliminate $sync from subroutines
Eric Wong [Fri, 10 Jan 2025 23:19:01 +0000 (23:19 +0000)] 
searchidx: eliminate $sync from subroutines

Eliminating this variable will hopefully reduce cognitive
overhead as well as reducing overhead from argument passing.

6 months agoindex: move {reindex} to $self
Eric Wong [Fri, 10 Jan 2025 23:19:00 +0000 (23:19 +0000)] 
index: move {reindex} to $self

Again, we can easily contain the scope of {reindex} to $self via
`local', so there's no need to rely on another temporary structure.

6 months agoindex: move {D} (delete state) to $self
Eric Wong [Fri, 10 Jan 2025 23:18:59 +0000 (23:18 +0000)] 
index: move {D} (delete state) to $self

The {D} delete state is short-lived and may have its scope
limited via `local', so take another step towards reducing
internal data structures.

6 months ago(ext)index: move {max_size} and related bits to $self
Eric Wong [Fri, 10 Jan 2025 23:18:58 +0000 (23:18 +0000)] 
(ext)index: move {max_size} and related bits to $self

`--max-size' is persistent for a process, so having it in $self
is more appropriate anyways than the shorter-lived $sync.  The
{index_oid} function pointer is tied to max-size handling, so
we'll move that over to $self as well.

6 months agosearchidx: rename {sidx} to {self}
Eric Wong [Fri, 10 Jan 2025 23:18:57 +0000 (23:18 +0000)] 
searchidx: rename {sidx} to {self}

While this is v1-only code, {self} is more consistent with the
rest of the code and the eventual goal is to eliminate $sync
entirely in favor of $self for temporary state.

6 months agosearchidx: move {ntodo} to $self
Eric Wong [Fri, 10 Jan 2025 23:18:56 +0000 (23:18 +0000)] 
searchidx: move {ntodo} to $self

Another step towards eliminating the temporary $sync structure
to flatten the internal data structures.

6 months agoextindex: move {id2pos} to $self
Eric Wong [Fri, 10 Jan 2025 23:18:55 +0000 (23:18 +0000)] 
extindex: move {id2pos} to $self

{id2pos} doesn't need a different hash table entry for every
request, instead it is tied to the inboxes associated with the
ExtSearchIdx ($self) object so we can just store it and give it
the same lifetime as the associated inbox references.

6 months agoextindex: {boost_in_use} field to $self
Eric Wong [Fri, 10 Jan 2025 23:18:16 +0000 (23:18 +0000)] 
extindex: {boost_in_use} field to $self

There's no need to repeatedly propagate this field to every
cat-file blob request since this field is tied to the attached
config and inboxes associated with an inbox.

6 months ago(ext)index: move {quit} from $sync to $self
Eric Wong [Fri, 10 Jan 2025 23:18:15 +0000 (23:18 +0000)] 
(ext)index: move {quit} from $sync to $self

No need to localize it, either, since we don't expect to
use the $self instance after {quit} is set.

6 months ago(ext)index: avoid needless {git} ref with --max-size
Eric Wong [Fri, 10 Jan 2025 23:18:14 +0000 (23:18 +0000)] 
(ext)index: avoid needless {git} ref with --max-size

We can access the PublicInbox::Git object via {ibx} directly to
avoid redundant refs and cut down the $sync struct further.

6 months agosearchidx: prefix v1 code with `v1_'
Eric Wong [Fri, 10 Jan 2025 23:18:13 +0000 (23:18 +0000)] 
searchidx: prefix v1 code with `v1_'

Since there's no or v1-specific modules such as `V1Writable',
prefixing v1-specific subs seems like a prudent way to demarcate
subs used for dealing with the old v1 storage format.

We'll also take this opportunity to rearrange the order of subs
to take advantage of prototypes and compile-time checking with
`perl -c'.

6 months agosmsg->populate: rename $sync to $cmt_info
Eric Wong [Fri, 10 Jan 2025 23:18:12 +0000 (23:18 +0000)] 
smsg->populate: rename $sync to $cmt_info

We only use this hashref to propagate author and commit time
from the commit, so give it a more descriptive name rather than
overloading the to-be-eliminated `$sync' struct.  There's no
need to explicitly vivify into a hashref, either.

6 months ago(ext)index: move {latest_cmt} to $self (from $sync)
Eric Wong [Fri, 10 Jan 2025 23:18:11 +0000 (23:18 +0000)] 
(ext)index: move {latest_cmt} to $self (from $sync)

No more sigil and brace overload from ${$sync->{latest_cmt}}
and another step towards $sync elimination.

6 months ago(ext)index: move {-regen_fmt} from $sync to $self
Eric Wong [Fri, 10 Jan 2025 23:18:10 +0000 (23:18 +0000)] 
(ext)index: move {-regen_fmt} from $sync to $self

We rely on `local' anyways in some cases.  This is yet another
step towards eliminating the $sync structure.

6 months ago(ext)index: move {-opt} from $sync to $self
Eric Wong [Fri, 10 Jan 2025 23:18:09 +0000 (23:18 +0000)] 
(ext)index: move {-opt} from $sync to $self

Another step towards eradicating the $sync structure.

This intermediate step does introduce the $self arg into
log2stack() and prepare_stack() but $self will eventually
eliminate $sync entirely.

6 months ago(ext)index: ${$sync->{nr}} to $self->{nrec}
Eric Wong [Fri, 10 Jan 2025 23:18:08 +0000 (23:18 +0000)] 
(ext)index: ${$sync->{nr}} to $self->{nrec}

{nrec} is probably less confusing as a name in a long-lived
context and we can get rid of the awkward scalar dereferencing
by using $self.

6 months agoExtMsg, NewsWWW: account for publicinbox.nameIsUrl
Eric Wong [Wed, 8 Jan 2025 10:12:05 +0000 (10:12 +0000)] 
ExtMsg, NewsWWW: account for publicinbox.nameIsUrl

As with WwwListing, these generate full URLs which go across
multiple inboxes, so support the new publicinbox.nameIsUrl
feature which allows users with many inboxes on the same domain
to avoid setting `publicinbox.*.url' for each one.

6 months agot/lei-sigpipe.t: use correct pipe size
Alyssa Ross [Tue, 7 Jan 2025 17:46:23 +0000 (18:46 +0100)] 
t/lei-sigpipe.t: use correct pipe size

According to fcntl(2), Linux will round up pipe sizes lower than a
page to a whole page.  Additionally, the kernel may use a larger size
in general, "if that is convenient for the implementation".  The
actual size of the pipe is returned from fcntl, so the test should use
that return value, rather than assuming the size requested was
accepted exactly.  This fixes the test on my system with 16K pages.

6 months agoextindex: use nproc_shards directly from IPC
Eric Wong [Thu, 2 Jan 2025 22:50:31 +0000 (22:50 +0000)] 
extindex: use nproc_shards directly from IPC

There's no reason to use it from the V2Writable namespace
since it was moved to PublicInbox::IPC in commit
9225145d (ipc: move nproc_shards from v2writable, 2023-03-21)
Import it early via `use' so we can take advantage of prototype
checking while we're at it.

6 months agowatch: don't propagate header changes to other inboxes
Eric Wong [Wed, 1 Jan 2025 23:11:53 +0000 (23:11 +0000)] 
watch: don't propagate header changes to other inboxes

Message-IDs may be appended by v2 inboxes for messages with with
conflicting Message-IDs or SpamAssassin (or another spam filter)
can change headers of both v1 and v2 messages.  So we rely on
`local' to reach into Eml internals to localize ->header_set
modifications and rely on modern Perl having copy-on-write
scalars to minimize memory traffic in the common case.

6 months agowatch: import_eml: avoid Eml dup for non-scrub case
Eric Wong [Mon, 30 Dec 2024 22:28:46 +0000 (22:28 +0000)] 
watch: import_eml: avoid Eml dup for non-scrub case

Most inboxes don't use filters, so the destructive ->scrub won't
be called and we can avoid duplicating the Eml object in the
common case.  We'll also drop the last inbox optimization for
net_cb since it's likely unnecessary given that filters are not
very common.

This may be more noticeable for users who map multiple inboxes
to a single Maildir|IMAP|NNTP source.

6 months agowatch|mda|purge: Filter::*->scrub is destructive
Eric Wong [Mon, 30 Dec 2024 22:28:45 +0000 (22:28 +0000)] 
watch|mda|purge: Filter::*->scrub is destructive

-watch was failing to properly account for inboxes having
different filters and ->scrub behavior during message removal
(but not adds).  Explicitly duplicate and simplify the -watch
code for handling message removal and update the rest of the
callers to reflect the standardized destructive behavior of
->scrub.  While most filters were destructive anyways, the Vger
filter was a notable exception and updated to be destructive.

We'll also update some variables from `$mime' uses to `$eml' to
reflect the switch from (Email|PublicInbox)::MIME to
PublicInbox::Eml years ago.

Finally, the Vger filter includes updated comments and uses
v5.10.1+ features, now.

6 months agotest_common: support `require_mods "v2"' for v2 inboxes
Eric Wong [Wed, 1 Jan 2025 22:36:59 +0000 (22:36 +0000)] 
test_common: support `require_mods "v2"' for v2 inboxes

For our purposes, `v2' should be interpreted as a public-inbox
format and not as a Perl version (e.g. `use v5.12').  This ensures
v2index-late-dupe.t correctly depends on DBD::SQLite and git
v2.6+ via `require_mods "v2"' instead of requiring Perl v2.x.

On a side note, it's unlikely public-inbox-v3 format will ever
be necessary as the v2 format appears scalable enough; thus we
won't hit v5 and risk conflicting with in-use Perl versions.
However, I'm not ruling out a v2.112 format for (nearly)
transparent v1->v2 migrations.

6 months agowatch: don't count invalid paths against batch limit
Eric Wong [Thu, 26 Dec 2024 21:48:51 +0000 (21:48 +0000)] 
watch: don't count invalid paths against batch limit

Invalid paths such as `.', `..', `.mh_sequence', and perhaps
other implementation-specific files may throw off the count and
cause premature commits.  While the premature commit isn't too
harmful in the common case, it's possible a pathological case of
having too many non-mail entries in a directory can cause
noticeable slowdowns and storage wear.

So have _try_path() and _remove_spam() return a true value if
a file was actually read.  We'll also simplify the $inboxes
check by relying simply on `eq', since the `ref' check isn't
necessary as the `eq' against a ref will never match the
"watchspam" literal.

7 months agoimport: fix space calculation when reusing epochs
Eric Wong [Tue, 17 Dec 2024 21:27:37 +0000 (21:27 +0000)] 
import: fix space calculation when reusing epochs

Dividing the result of $git->packed_bytes by $PACKING_FACTOR
_twice_ was completely wrong for v2.  Just calculate
$unpacked_bytes once and use it for the Import->{bytes_added}
field.  The calculation for lei/store was actually correct,
just redundant since repeated division is unnecessary.

7 months agov2writable: simplify epoch directory generation
Eric Wong [Tue, 17 Dec 2024 21:27:36 +0000 (21:27 +0000)] 
v2writable: simplify epoch directory generation

As noted in a now-removed comment, InboxWritable->git_dir_latest
seems redundant and an unnecessary function.  Instead, we can
use MultiGit->epoch_dir for these v2-only (non-extindex)
codepaths.

7 months agot/filter_base: relax Regexp class match with ->isa
Eric Wong [Fri, 13 Dec 2024 18:36:31 +0000 (18:36 +0000)] 
t/filter_base: relax Regexp class match with ->isa

It would be nice to support alternative Regexp engines such
as re::engine::PCRE2 in the future.  The exact ref() name
can't match, however ->isa() works with re::engine::PCRE2.
So future-proof our code for potential changes in case PCRE2
becomes usable.

7 months agolei: use PublicInbox::Eml::warn_ignore_cb
Eric Wong [Sat, 14 Dec 2024 14:58:04 +0000 (14:58 +0000)] 
lei: use PublicInbox::Eml::warn_ignore_cb

The open-coded callback was identical to the existing callback
generated by our Eml package, so just use the existing code
instead unless a user uses `lei import --noisy'

7 months agoadmin: common warn_cb
Eric Wong [Sat, 14 Dec 2024 14:58:03 +0000 (14:58 +0000)] 
admin: common warn_cb

I noticed verbose output from t/extsearch.t was not correctly
commenting out (`# ') informational messages when the
{current_info} prefix is in use.  This also appears to affect
other indexers with the exception of the new -cindex.

So consolidate all the common warning code into
PublicInbox::Admin module so we can handle {current_info}
properly in all contexts.

-cindex gets some unnecessary Eml warning filtering, but that's
probably not a real problem and worth the overall code savings
since PublicInbox::Admin is loaded by script/public-inbox-cindex
anyways.

7 months agot/extindex: add --max-size test
Eric Wong [Sat, 14 Dec 2024 14:58:02 +0000 (14:58 +0000)] 
t/extindex: add --max-size test

I noticed this missing coverage while working on another
series of changes which aren't quite ready, yet.

7 months agosearchidx: consolidate checkpoint accounting
Eric Wong [Thu, 12 Dec 2024 10:10:40 +0000 (10:10 +0000)] 
searchidx: consolidate checkpoint accounting

We can eliminate check_batch_limit() and checkpoint_due() from
extsearchidx in favor of update_checkpoint() in searchidx.  We
can also get rid of the awkward scalar deref for setting the
{need_checkpoint} field.

The only behavioral difference is the checkpoint interval is
standardized to 5s and -extindex no longer uses 10s for its
checkpoints.  In retrospect, 5s should work more nicely for
public-facing indices since they spend less time waiting on
writers, but it has the downside of potentially hurting writer
performance.

This is another step in the gradual shift away from the $sync
arg in favor of `local $self->{...}'.

7 months agoextindex: move {checkpoint_unlocks} to $self
Eric Wong [Thu, 12 Dec 2024 10:10:39 +0000 (10:10 +0000)] 
extindex: move {checkpoint_unlocks} to $self

One small step towards eliminating the $sync structure.

7 months agosearchidx: update_checkpoint: take bytes arg directly
Eric Wong [Thu, 12 Dec 2024 10:10:38 +0000 (10:10 +0000)] 
searchidx: update_checkpoint: take bytes arg directly

Passing $smsg limits flexibility in case we reuse it for deletes
and/or commit search.

7 months agoover_idx: simplify each_by_mid
Eric Wong [Wed, 11 Dec 2024 08:10:47 +0000 (08:10 +0000)] 
over_idx: simplify each_by_mid

Eliminate a useless use of `map {}' and avoid a needless `push'
op since the `join' op can just take extra arg.

7 months agocindex: adjust estimated memory cost for deletes
Eric Wong [Wed, 11 Dec 2024 08:10:46 +0000 (08:10 +0000)] 
cindex: adjust estimated memory cost for deletes

Based on my conversations with the Xapian lead, the cost of
deletes were overestimated by 7x in cindex.  Adjust the estimate
cost of a deleted document to a more reasonable number based on
calculations discussed on the xapian-discuss list.

In any case, all of our batch size memory cost estimates are
rough since since Xapian provides no way of letting us know the
memory cost of the current transaction.