[PATCH 00/10] index: support progress output

[PATCH 00/10] index: support progress output

[Eric Wong]

While progress support was implemented for -xcpdb, there was no
way to enable it for the -index command; and v2 progress was
lacking compared to v1.

This series fills in those functionality gaps and cleans up some
of the v2 indexing code at the same time.

Eric Wong (10):
  v2writable: introduce $sync state and put mm_tmp in it
  v2writable: sync: move delete markers into $sync state
  v2writable: move {reindex} field to $sync state
  v2writable: move {regen} into $sync state
  v2writable: move {ranges} into $sync state
  v2writable: localize unindex-range.$EPOCH to $sync state
  v2writable: use prototypes for internal subs
  v2writable: move index_sync options to sync state
  index: support --verbose option
  v2writable: show progress updates for index_sync

 lib/PublicInbox/Admin.pm      |  18 ++++
 lib/PublicInbox/V2Writable.pm | 178 +++++++++++++++++++---------------
 lib/PublicInbox/Xapcmd.pm     |  14 +--
 script/public-inbox-index     |   5 +-
 4 files changed, 124 insertions(+), 91 deletions(-)

-- 
EW

[PATCH 01/10] v2writable: introduce $sync state and put mm_tmp in it

[Eric Wong]

A first step towards making the v2 index_sync code
easier-to-follow.  More fields to follow...
---
 lib/PublicInbox/V2Writable.pm | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm
index 331c4f4..584767a 100644
--- a/lib/PublicInbox/V2Writable.pm
+++ b/lib/PublicInbox/V2Writable.pm
@@ -706,7 +706,7 @@ sub mark_deleted {
 }
 
 sub reindex_oid {
-	my ($self, $mm_tmp, $D, $git, $oid, $regen, $reindex) = @_;
+	my ($self, $sync, $D, $git, $oid, $regen, $reindex) = @_;
 	my $len;
 	my $msgref = $git->cat_file($oid, \$len);
 	my $mime = PublicInbox::MIME->new($$msgref);
@@ -714,13 +714,13 @@ sub reindex_oid {
 	my $cid = content_id($mime);
 
 	# get the NNTP article number we used before, highest number wins
-	# and gets deleted from mm_tmp;
+	# and gets deleted from sync->{mm_tmp};
 	my $mid0;
 	my $num = -1;
 	my $del = 0;
 	foreach my $mid (@$mids) {
 		$del += delete($D->{"$mid\0$cid"}) ? 1 : 0;
-		my $n = $mm_tmp->num_for($mid);
+		my $n = $sync->{mm_tmp}->num_for($mid);
 		if (defined $n && $n > $num) {
 			$mid0 = $mid;
 			$num = $n;
@@ -764,7 +764,7 @@ sub reindex_oid {
 		return;
 
 	}
-	$mm_tmp->mid_delete($mid0) or
+	$sync->{mm_tmp}->mid_delete($mid0) or
 		die "failed to delete <$mid0> for article #$num\n";
 
 	$self->{over}->add_overview($mime, $len, $num, $oid, $mid0);
@@ -775,14 +775,14 @@ sub reindex_oid {
 	my $n = $self->{transact_bytes} += $len;
 	if ($n > (PublicInbox::SearchIdx::BATCH_BYTES * $nparts)) {
 		$git->cleanup;
-		$mm_tmp->atfork_prepare;
+		$sync->{mm_tmp}->atfork_prepare;
 		$self->done; # release lock
 
 		# TODO: print progress info, here
 
 		# allow -watch or -mda to write...
 		$self->idx_init; # reacquire lock
-		$mm_tmp->atfork_parent;
+		$sync->{mm_tmp}->atfork_parent;
 	}
 }
 
@@ -961,7 +961,9 @@ sub index_sync {
 	my $latest = git_dir_latest($self, \$epoch_max);
 	return unless defined $latest;
 	$self->idx_init($opts); # acquire lock
-	my $mm_tmp = $self->{mm}->tmp_clone;
+	my $sync = {
+		mm_tmp => $self->{mm}->tmp_clone,
+	};
 	my $reindex = $opts->{reindex};
 	my $ranges = index_ranges($self, $reindex, $epoch_max);
 
@@ -997,7 +999,7 @@ sub index_sync {
 			if (/\A$x40$/o && !defined($cmt)) {
 				$cmt = $_;
 			} elsif (/\A:\d{6} 100644 $x40 ($x40) [AM]\tm$/o) {
-				$self->reindex_oid($mm_tmp, $D, $git, $1,
+				$self->reindex_oid($sync, $D, $git, $1,
 						$regen, $reindex);
 			} elsif (/\A:\d{6} 100644 $x40 ($x40) [AM]\td$/o) {
 				$self->mark_deleted($D, $git, $1);
@@ -1020,7 +1022,7 @@ sub index_sync {
 	# reindex does not pick up new changes, so we rerun w/o it:
 	if ($opts->{reindex}) {
 		my %again = %$opts;
-		$mm_tmp = undef;
+		$sync = undef;
 		delete @again{qw(reindex -skip_lock)};
 		index_sync($self, \%again);
 	}
-- 
EW

[PATCH 02/10] v2writable: sync: move delete markers into $sync state

[Eric Wong]

Another small step to reduce parameters passed to reindex_oid.
---
 lib/PublicInbox/V2Writable.pm | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm
index 584767a..efb9f42 100644
--- a/lib/PublicInbox/V2Writable.pm
+++ b/lib/PublicInbox/V2Writable.pm
@@ -695,18 +695,18 @@ sub atfork_child {
 }
 
 sub mark_deleted {
-	my ($self, $D, $git, $oid) = @_;
+	my ($self, $sync, $git, $oid) = @_;
 	my $msgref = $git->cat_file($oid);
 	my $mime = PublicInbox::MIME->new($$msgref);
 	my $mids = mids($mime->header_obj);
 	my $cid = content_id($mime);
 	foreach my $mid (@$mids) {
-		$D->{"$mid\0$cid"} = $oid;
+		$sync->{D}->{"$mid\0$cid"} = $oid;
 	}
 }
 
 sub reindex_oid {
-	my ($self, $sync, $D, $git, $oid, $regen, $reindex) = @_;
+	my ($self, $sync, $git, $oid, $regen, $reindex) = @_;
 	my $len;
 	my $msgref = $git->cat_file($oid, \$len);
 	my $mime = PublicInbox::MIME->new($$msgref);
@@ -719,7 +719,7 @@ sub reindex_oid {
 	my $num = -1;
 	my $del = 0;
 	foreach my $mid (@$mids) {
-		$del += delete($D->{"$mid\0$cid"}) ? 1 : 0;
+		$del += delete($sync->{D}->{"$mid\0$cid"}) ? 1 : 0;
 		my $n = $sync->{mm_tmp}->num_for($mid);
 		if (defined $n && $n > $num) {
 			$mid0 = $mid;
@@ -963,6 +963,7 @@ sub index_sync {
 	$self->idx_init($opts); # acquire lock
 	my $sync = {
 		mm_tmp => $self->{mm}->tmp_clone,
+		D => {}, # "$mid\0$cid" => $oid
 	};
 	my $reindex = $opts->{reindex};
 	my $ranges = index_ranges($self, $reindex, $epoch_max);
@@ -977,7 +978,6 @@ sub index_sync {
 		$$regen += $high;
 	}
 
-	my $D = {}; # "$mid\0$cid" => $oid
 	my @cmd = qw(log --raw -r --pretty=tformat:%H
 			--no-notes --no-color --no-abbrev --no-renames);
 
@@ -999,10 +999,10 @@ sub index_sync {
 			if (/\A$x40$/o && !defined($cmt)) {
 				$cmt = $_;
 			} elsif (/\A:\d{6} 100644 $x40 ($x40) [AM]\tm$/o) {
-				$self->reindex_oid($sync, $D, $git, $1,
+				$self->reindex_oid($sync, $git, $1,
 						$regen, $reindex);
 			} elsif (/\A:\d{6} 100644 $x40 ($x40) [AM]\td$/o) {
-				$self->mark_deleted($D, $git, $1);
+				$self->mark_deleted($sync, $git, $1);
 			}
 		}
 		$fh = undef;
@@ -1012,9 +1012,9 @@ sub index_sync {
 
 	# unindex is required for leftovers if "deletes" affect messages
 	# in a previous fetch+index window:
-	if (scalar keys %$D) {
+	if (my @leftovers = values %{delete $sync->{D}}) {
 		my $git = $self->{-inbox}->git;
-		$self->unindex_oid($git, $_) for values %$D;
+		$self->unindex_oid($git, $_) for @leftovers;
 		$git->cleanup;
 	}
 	$self->done;
-- 
EW

[PATCH 03/10] v2writable: move {reindex} field to $sync state

[Eric Wong]

reindexing info is not used outside of the index_sync code path.
---
 lib/PublicInbox/V2Writable.pm | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm
index efb9f42..e05dadc 100644
--- a/lib/PublicInbox/V2Writable.pm
+++ b/lib/PublicInbox/V2Writable.pm
@@ -706,7 +706,7 @@ sub mark_deleted {
 }
 
 sub reindex_oid {
-	my ($self, $sync, $git, $oid, $regen, $reindex) = @_;
+	my ($self, $sync, $git, $oid, $regen) = @_;
 	my $len;
 	my $msgref = $git->cat_file($oid, \$len);
 	my $mime = PublicInbox::MIME->new($$msgref);
@@ -750,7 +750,7 @@ sub reindex_oid {
 	if (!defined($mid0) || $del) {
 		if (!defined($mid0) && $del) { # expected for deletes
 			$num = $$regen--;
-			$self->{mm}->num_highwater($num) unless $reindex;
+			$self->{mm}->num_highwater($num) if !$sync->{reindex};
 			return
 		}
 
@@ -940,10 +940,11 @@ sub unindex {
 		qw(-c gc.reflogExpire=now gc --prune=all)]);
 }
 
-sub index_ranges ($$$) {
-	my ($self, $reindex, $epoch_max) = @_;
-	return last_commits($self, $epoch_max) unless $reindex;
+sub sync_ranges ($$$) {
+	my ($self, $sync, $epoch_max) = @_;
+	my $reindex = $sync->{reindex};
 
+	return last_commits($self, $epoch_max) unless $reindex;
 	return [] if ref($reindex) ne 'HASH';
 
 	my $ranges = $reindex->{from}; # arrayref;
@@ -964,9 +965,9 @@ sub index_sync {
 	my $sync = {
 		mm_tmp => $self->{mm}->tmp_clone,
 		D => {}, # "$mid\0$cid" => $oid
+		reindex => $opts->{reindex},
 	};
-	my $reindex = $opts->{reindex};
-	my $ranges = index_ranges($self, $reindex, $epoch_max);
+	my $ranges = sync_ranges($self, $sync, $epoch_max);
 
 	my $high = $self->{mm}->num_highwater();
 	my $regen = $self->index_prepare($opts, $epoch_max, $ranges);
@@ -999,8 +1000,7 @@ sub index_sync {
 			if (/\A$x40$/o && !defined($cmt)) {
 				$cmt = $_;
 			} elsif (/\A:\d{6} 100644 $x40 ($x40) [AM]\tm$/o) {
-				$self->reindex_oid($sync, $git, $1,
-						$regen, $reindex);
+				$self->reindex_oid($sync, $git, $1, $regen);
 			} elsif (/\A:\d{6} 100644 $x40 ($x40) [AM]\td$/o) {
 				$self->mark_deleted($sync, $git, $1);
 			}
-- 
EW

[PATCH 04/10] v2writable: move {regen} into $sync state

[Eric Wong]

regen is always enabled for index_sync nowadays (and has
been for a while).

Rename `index_prepare' to `sync_prepare' to show it's for
->index_sync; and not the online indexing we do for ->add.
---
 lib/PublicInbox/V2Writable.pm | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm
index e05dadc..e341d93 100644
--- a/lib/PublicInbox/V2Writable.pm
+++ b/lib/PublicInbox/V2Writable.pm
@@ -706,7 +706,7 @@ sub mark_deleted {
 }
 
 sub reindex_oid {
-	my ($self, $sync, $git, $oid, $regen) = @_;
+	my ($self, $sync, $git, $oid) = @_;
 	my $len;
 	my $msgref = $git->cat_file($oid, \$len);
 	my $mime = PublicInbox::MIME->new($$msgref);
@@ -727,8 +727,8 @@ sub reindex_oid {
 			$self->{mm}->mid_set($num, $mid0);
 		}
 	}
-	if (!defined($mid0) && $regen && !$del) {
-		$num = $$regen--;
+	if (!defined($mid0) && !$del) {
+		$num = $sync->{regen}--;
 		die "BUG: ran out of article numbers\n" if $num <= 0;
 		my $mm = $self->{mm};
 		foreach my $mid (reverse @$mids) {
@@ -749,7 +749,7 @@ sub reindex_oid {
 
 	if (!defined($mid0) || $del) {
 		if (!defined($mid0) && $del) { # expected for deletes
-			$num = $$regen--;
+			$num = $sync->{regen}--;
 			$self->{mm}->num_highwater($num) if !$sync->{reindex};
 			return
 		}
@@ -845,7 +845,7 @@ $range
 	$range;
 }
 
-sub index_prepare {
+sub sync_prepare {
 	my ($self, $opts, $epoch_max, $ranges) = @_;
 	my $pr = $opts->{-progress};
 	my $regen_max = 0;
@@ -879,7 +879,10 @@ sub index_prepare {
 		$pr->("$n\n") if $pr;
 		$regen_max += $n;
 	}
-	\$regen_max;
+	# reindex should NOT see new commits anymore, if we do,
+	# it's a problem and we need to notice it via die()
+	return -1 if $opts->{reindex};
+	$regen_max + $self->{mm}->num_highwater() || 0;
 }
 
 sub unindex_oid_remote {
@@ -968,16 +971,7 @@ sub index_sync {
 		reindex => $opts->{reindex},
 	};
 	my $ranges = sync_ranges($self, $sync, $epoch_max);
-
-	my $high = $self->{mm}->num_highwater();
-	my $regen = $self->index_prepare($opts, $epoch_max, $ranges);
-	if ($opts->{reindex}) {
-		# reindex should NOT see new commits anymore, if we do,
-		# it's a problem and we need to notice it via die()
-		$$regen = -1;
-	} else {
-		$$regen += $high;
-	}
+	$sync->{regen} = sync_prepare($self, $opts, $epoch_max, $ranges);
 
 	my @cmd = qw(log --raw -r --pretty=tformat:%H
 			--no-notes --no-color --no-abbrev --no-renames);
@@ -1000,7 +994,7 @@ sub index_sync {
 			if (/\A$x40$/o && !defined($cmt)) {
 				$cmt = $_;
 			} elsif (/\A:\d{6} 100644 $x40 ($x40) [AM]\tm$/o) {
-				$self->reindex_oid($sync, $git, $1, $regen);
+				$self->reindex_oid($sync, $git, $1);
 			} elsif (/\A:\d{6} 100644 $x40 ($x40) [AM]\td$/o) {
 				$self->mark_deleted($sync, $git, $1);
 			}
-- 
EW

[PATCH 05/10] v2writable: move {ranges} into $sync state

[Eric Wong]

Yet another temporary variable with no use outside of index_sync.
---
 lib/PublicInbox/V2Writable.pm | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm
index e341d93..eef5576 100644
--- a/lib/PublicInbox/V2Writable.pm
+++ b/lib/PublicInbox/V2Writable.pm
@@ -813,14 +813,14 @@ sub last_commits {
 
 # returns a revision range for git-log(1)
 sub log_range ($$$$$) {
-	my ($self, $git, $ranges, $i, $tip) = @_;
-	my $cur = $ranges->[$i] or return $tip; # all of it
+	my ($self, $sync, $git, $i, $tip) = @_;
+	my $cur = $sync->{ranges}->[$i] or return $tip; # all of it
 	my $range = "$cur..$tip";
 	if (is_ancestor($git, $cur, $tip)) { # common case
 		my $n = $git->qx(qw(rev-list --count), $range);
 		chomp($n);
 		if ($n == 0) {
-			$ranges->[$i] = undef;
+			$sync->{ranges}->[$i] = undef;
 			return; # nothing to do
 		}
 	} else {
@@ -846,7 +846,7 @@ $range
 }
 
 sub sync_prepare {
-	my ($self, $opts, $epoch_max, $ranges) = @_;
+	my ($self, $sync, $opts, $epoch_max) = @_;
 	my $pr = $opts->{-progress};
 	my $regen_max = 0;
 	my $head = $self->{-inbox}->{ref_head} || 'refs/heads/master';
@@ -866,8 +866,8 @@ sub sync_prepare {
 		chomp(my $tip = $git->qx(qw(rev-parse -q --verify), $head));
 
 		next if $?; # new repo
-		my $range = log_range($self, $git, $ranges, $i, $tip) or next;
-		$ranges->[$i] = $range;
+		my $range = log_range($self, $sync, $git, $i, $tip) or next;
+		$sync->{ranges}->[$i] = $range;
 
 		# can't use 'rev-list --count' if we use --diff-filter
 		$pr->("$i.git counting changes\n\t$range ... ") if $pr;
@@ -970,8 +970,8 @@ sub index_sync {
 		D => {}, # "$mid\0$cid" => $oid
 		reindex => $opts->{reindex},
 	};
-	my $ranges = sync_ranges($self, $sync, $epoch_max);
-	$sync->{regen} = sync_prepare($self, $opts, $epoch_max, $ranges);
+	$sync->{ranges} = sync_ranges($self, $sync, $epoch_max);
+	$sync->{regen} = sync_prepare($self, $sync, $opts, $epoch_max);
 
 	my @cmd = qw(log --raw -r --pretty=tformat:%H
 			--no-notes --no-color --no-abbrev --no-renames);
@@ -985,7 +985,7 @@ sub index_sync {
 		my $git = PublicInbox::Git->new($git_dir);
 		my $unindex = delete $self->{"unindex-range.$i"};
 		$self->unindex($opts, $git, $unindex) if $unindex;
-		defined(my $range = $ranges->[$i]) or next;
+		defined(my $range = $sync->{ranges}->[$i]) or next;
 		my $fh = $self->{reindex_pipe} = $git->popen(@cmd, $range);
 		my $cmt;
 		while (<$fh>) {
-- 
EW

[PATCH 06/10] v2writable: localize unindex-range.$EPOCH to $sync state

[Eric Wong]

We don't need to stuff that into $self (V2Writable) which can be
longer-lived than a ->index_sync invocation.
---
 lib/PublicInbox/V2Writable.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm
index eef5576..315404c 100644
--- a/lib/PublicInbox/V2Writable.pm
+++ b/lib/PublicInbox/V2Writable.pm
@@ -840,7 +840,7 @@ Rewritten history? (in $git->{git_dir})
 reindexing $git->{git_dir} starting at
 $range
 
-		$self->{"unindex-range.$i"} = "$base..$cur";
+		$sync->{"unindex-range.$i"} = "$base..$cur";
 	}
 	$range;
 }
@@ -983,7 +983,7 @@ sub index_sync {
 		-d $git_dir or next; # missing parts are fine
 		fill_alternates($self, $i);
 		my $git = PublicInbox::Git->new($git_dir);
-		my $unindex = delete $self->{"unindex-range.$i"};
+		my $unindex = delete $sync->{"unindex-range.$i"};
 		$self->unindex($opts, $git, $unindex) if $unindex;
 		defined(my $range = $sync->{ranges}->[$i]) or next;
 		my $fh = $self->{reindex_pipe} = $git->popen(@cmd, $range);
-- 
EW

[PATCH 07/10] v2writable: use prototypes for internal subs

[Eric Wong]

Hopefully this improves maintainability by allowing Perl
to do some arg checking for us.
---
 lib/PublicInbox/V2Writable.pm | 52 +++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 23 deletions(-)

diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm
index 315404c..045a9cc 100644
--- a/lib/PublicInbox/V2Writable.pm
+++ b/lib/PublicInbox/V2Writable.pm
@@ -93,6 +93,7 @@ sub new {
 	bless $self, $class;
 }
 
+# public (for now?)
 sub init_inbox {
 	my ($self, $parallel, $skip_epoch) = @_;
 	$self->{parallel} = $parallel;
@@ -167,7 +168,7 @@ sub num_for {
 		# crap, Message-ID is already known, hope somebody just resent:
 		foreach my $m (@$mids) {
 			# read-only lookup now safe to do after above barrier
-			my $existing = $self->lookup_content($mime, $m);
+			my $existing = lookup_content($self, $mime, $m);
 			# easy, don't store duplicates
 			# note: do not add more diagnostic info here since
 			# it gets noisy on public-inbox-watch restarts
@@ -290,7 +291,7 @@ sub idx_init {
 	});
 }
 
-sub purge_oids {
+sub purge_oids ($$) {
 	my ($self, $purge) = @_; # $purge = { $object_id => 1, ... }
 	$self->done;
 	my $pfx = "$self->{-inbox}->{mainrepo}/git";
@@ -332,7 +333,7 @@ sub content_matches ($$) {
 	0
 }
 
-sub remove_internal {
+sub remove_internal ($$$$) {
 	my ($self, $mime, $cmt_msg, $purge) = @_;
 	$self->idx_init;
 	my $im = $self->importer unless $purge;
@@ -384,7 +385,7 @@ sub remove_internal {
 				($mark, undef) = $im->remove($orig, $cmt_msg);
 			}
 			$orig = undef;
-			$self->unindex_oid_remote($oid, $mid);
+			unindex_oid_remote($self, $oid, $mid);
 		}
 	}
 
@@ -398,13 +399,15 @@ sub remove_internal {
 	$removed;
 }
 
+# public
 sub remove {
 	my ($self, $mime, $cmt_msg) = @_;
 	$self->{-inbox}->with_umask(sub {
-		remove_internal($self, $mime, $cmt_msg);
+		remove_internal($self, $mime, $cmt_msg, undef);
 	});
 }
 
+# public
 sub purge {
 	my ($self, $mime) = @_;
 	my $purges = $self->{-inbox}->with_umask(sub {
@@ -453,6 +456,7 @@ sub barrier_wait {
 	}
 }
 
+# public
 sub checkpoint ($;$) {
 	my ($self, $wait) = @_;
 
@@ -499,8 +503,10 @@ sub checkpoint ($;$) {
 
 # issue a write barrier to ensure all data is visible to other processes
 # and read-only ops.  Order of data importance is: git > SQLite > Xapian
+# public
 sub barrier { checkpoint($_[0], 1) };
 
+# public
 sub done {
 	my ($self) = @_;
 	my $im = delete $self->{im};
@@ -655,7 +661,7 @@ sub get_blob ($$) {
 	$ibx->msg_by_smsg($smsg);
 }
 
-sub lookup_content {
+sub lookup_content ($$$) {
 	my ($self, $mime, $mid) = @_;
 	my $over = $self->{over};
 	my $cids = content_ids($mime);
@@ -694,7 +700,7 @@ sub atfork_child {
 	$self->{bnote}->[1];
 }
 
-sub mark_deleted {
+sub mark_deleted ($$$$) {
 	my ($self, $sync, $git, $oid) = @_;
 	my $msgref = $git->cat_file($oid);
 	my $mime = PublicInbox::MIME->new($$msgref);
@@ -705,7 +711,7 @@ sub mark_deleted {
 	}
 }
 
-sub reindex_oid {
+sub reindex_oid ($$$$) {
 	my ($self, $sync, $git, $oid) = @_;
 	my $len;
 	my $msgref = $git->cat_file($oid, \$len);
@@ -787,7 +793,7 @@ sub reindex_oid {
 }
 
 # only update last_commit for $i on reindex iff newer than current
-sub update_last_commit {
+sub update_last_commit ($$$$) {
 	my ($self, $git, $i, $cmt) = @_;
 	my $last = last_commit_part($self, $i);
 	if (defined $last && is_ancestor($git, $last, $cmt)) {
@@ -800,7 +806,7 @@ sub update_last_commit {
 
 sub git_dir_n ($$) { "$_[0]->{-inbox}->{mainrepo}/git/$_[1].git" }
 
-sub last_commits {
+sub last_commits ($$) {
 	my ($self, $epoch_max) = @_;
 	my $heads = [];
 	for (my $i = $epoch_max; $i >= 0; $i--) {
@@ -845,7 +851,7 @@ $range
 	$range;
 }
 
-sub sync_prepare {
+sub sync_prepare ($$$$) {
 	my ($self, $sync, $opts, $epoch_max) = @_;
 	my $pr = $opts->{-progress};
 	my $regen_max = 0;
@@ -885,13 +891,13 @@ sub sync_prepare {
 	$regen_max + $self->{mm}->num_highwater() || 0;
 }
 
-sub unindex_oid_remote {
+sub unindex_oid_remote ($$$) {
 	my ($self, $oid, $mid) = @_;
 	$_->remote_remove($oid, $mid) foreach @{$self->{idx_parts}};
 	$self->{over}->remove_oid($oid, $mid);
 }
 
-sub unindex_oid {
+sub unindex_oid ($$$) {
 	my ($self, $git, $oid) = @_;
 	my $msgref = $git->cat_file($oid);
 	my $mime = PublicInbox::MIME->new($msgref);
@@ -915,12 +921,12 @@ sub unindex_oid {
 			$self->{unindexed}->{$_}++;
 			$self->{mm}->num_delete($num);
 		}
-		$self->unindex_oid_remote($oid, $mid);
+		unindex_oid_remote($self, $oid, $mid);
 	}
 }
 
 my $x40 = qr/[a-f0-9]{40}/;
-sub unindex {
+sub unindex ($$$$) {
 	my ($self, $opts, $git, $unindex_range) = @_;
 	my $un = $self->{unindexed} ||= {}; # num => removal count
 	my $before = scalar keys %$un;
@@ -929,7 +935,7 @@ sub unindex {
 	my $fh = $self->{reindex_pipe} = $git->popen(@cmd, $unindex_range);
 	while (<$fh>) {
 		/\A:\d{6} 100644 $x40 ($x40) [AM]\tm$/o or next;
-		$self->unindex_oid($git, $1);
+		unindex_oid($self, $git, $1);
 	}
 	delete $self->{reindex_pipe};
 	$fh = undef;
@@ -957,7 +963,7 @@ sub sync_ranges ($$$) {
 	$ranges;
 }
 
-# called for public-inbox-index
+# public, called by public-inbox-index
 sub index_sync {
 	my ($self, $opts) = @_;
 	$opts ||= {};
@@ -983,8 +989,8 @@ sub index_sync {
 		-d $git_dir or next; # missing parts are fine
 		fill_alternates($self, $i);
 		my $git = PublicInbox::Git->new($git_dir);
-		my $unindex = delete $sync->{"unindex-range.$i"};
-		$self->unindex($opts, $git, $unindex) if $unindex;
+		my $unindex_range = delete $sync->{"unindex-range.$i"};
+		unindex($self, $opts, $git, $unindex_range) if $unindex_range;
 		defined(my $range = $sync->{ranges}->[$i]) or next;
 		my $fh = $self->{reindex_pipe} = $git->popen(@cmd, $range);
 		my $cmt;
@@ -994,21 +1000,21 @@ sub index_sync {
 			if (/\A$x40$/o && !defined($cmt)) {
 				$cmt = $_;
 			} elsif (/\A:\d{6} 100644 $x40 ($x40) [AM]\tm$/o) {
-				$self->reindex_oid($sync, $git, $1);
+				reindex_oid($self, $sync, $git, $1);
 			} elsif (/\A:\d{6} 100644 $x40 ($x40) [AM]\td$/o) {
-				$self->mark_deleted($sync, $git, $1);
+				mark_deleted($self, $sync, $git, $1);
 			}
 		}
 		$fh = undef;
 		delete $self->{reindex_pipe};
-		$self->update_last_commit($git, $i, $cmt) if defined $cmt;
+		update_last_commit($self, $git, $i, $cmt) if defined $cmt;
 	}
 
 	# unindex is required for leftovers if "deletes" affect messages
 	# in a previous fetch+index window:
 	if (my @leftovers = values %{delete $sync->{D}}) {
 		my $git = $self->{-inbox}->git;
-		$self->unindex_oid($git, $_) for @leftovers;
+		unindex_oid($self, $git, $_) for @leftovers;
 		$git->cleanup;
 	}
 	$self->done;
-- 
EW

[PATCH 08/10] v2writable: move index_sync options to sync state

[Eric Wong]

And use singular `opt' to be consistent with the common name
of 'getopt'.
---
 lib/PublicInbox/V2Writable.pm | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm
index 045a9cc..aa180a2 100644
--- a/lib/PublicInbox/V2Writable.pm
+++ b/lib/PublicInbox/V2Writable.pm
@@ -851,15 +851,15 @@ $range
 	$range;
 }
 
-sub sync_prepare ($$$$) {
-	my ($self, $sync, $opts, $epoch_max) = @_;
-	my $pr = $opts->{-progress};
+sub sync_prepare ($$$) {
+	my ($self, $sync, $epoch_max) = @_;
+	my $pr = $sync->{-opt}->{-progress};
 	my $regen_max = 0;
 	my $head = $self->{-inbox}->{ref_head} || 'refs/heads/master';
 
 	# reindex stops at the current heads and we later rerun index_sync
 	# without {reindex}
-	my $reindex_heads = last_commits($self, $epoch_max) if $opts->{reindex};
+	my $reindex_heads = last_commits($self, $epoch_max) if $sync->{reindex};
 
 	for (my $i = $epoch_max; $i >= 0; $i--) {
 		die 'BUG: already indexing!' if $self->{reindex_pipe};
@@ -887,7 +887,7 @@ sub sync_prepare ($$$$) {
 	}
 	# reindex should NOT see new commits anymore, if we do,
 	# it's a problem and we need to notice it via die()
-	return -1 if $opts->{reindex};
+	return -1 if $sync->{reindex};
 	$regen_max + $self->{mm}->num_highwater() || 0;
 }
 
@@ -927,7 +927,7 @@ sub unindex_oid ($$$) {
 
 my $x40 = qr/[a-f0-9]{40}/;
 sub unindex ($$$$) {
-	my ($self, $opts, $git, $unindex_range) = @_;
+	my ($self, $sync, $git, $unindex_range) = @_;
 	my $un = $self->{unindexed} ||= {}; # num => removal count
 	my $before = scalar keys %$un;
 	my @cmd = qw(log --raw -r
@@ -940,7 +940,7 @@ sub unindex ($$$$) {
 	delete $self->{reindex_pipe};
 	$fh = undef;
 
-	return unless $opts->{prune};
+	return unless $sync->{-opt}->{prune};
 	my $after = scalar keys %$un;
 	return if $before == $after;
 
@@ -965,19 +965,20 @@ sub sync_ranges ($$$) {
 
 # public, called by public-inbox-index
 sub index_sync {
-	my ($self, $opts) = @_;
-	$opts ||= {};
+	my ($self, $opt) = @_;
+	$opt ||= {};
 	my $epoch_max;
 	my $latest = git_dir_latest($self, \$epoch_max);
 	return unless defined $latest;
-	$self->idx_init($opts); # acquire lock
+	$self->idx_init($opt); # acquire lock
 	my $sync = {
 		mm_tmp => $self->{mm}->tmp_clone,
 		D => {}, # "$mid\0$cid" => $oid
-		reindex => $opts->{reindex},
+		reindex => $opt->{reindex},
+		-opt => $opt
 	};
 	$sync->{ranges} = sync_ranges($self, $sync, $epoch_max);
-	$sync->{regen} = sync_prepare($self, $sync, $opts, $epoch_max);
+	$sync->{regen} = sync_prepare($self, $sync, $epoch_max);
 
 	my @cmd = qw(log --raw -r --pretty=tformat:%H
 			--no-notes --no-color --no-abbrev --no-renames);
@@ -990,7 +991,7 @@ sub index_sync {
 		fill_alternates($self, $i);
 		my $git = PublicInbox::Git->new($git_dir);
 		my $unindex_range = delete $sync->{"unindex-range.$i"};
-		unindex($self, $opts, $git, $unindex_range) if $unindex_range;
+		unindex($self, $sync, $git, $unindex_range) if $unindex_range;
 		defined(my $range = $sync->{ranges}->[$i]) or next;
 		my $fh = $self->{reindex_pipe} = $git->popen(@cmd, $range);
 		my $cmt;
@@ -1020,8 +1021,8 @@ sub index_sync {
 	$self->done;
 
 	# reindex does not pick up new changes, so we rerun w/o it:
-	if ($opts->{reindex}) {
-		my %again = %$opts;
+	if ($opt->{reindex}) {
+		my %again = %$opt;
 		$sync = undef;
 		delete @again{qw(reindex -skip_lock)};
 		index_sync($self, \%again);
-- 
EW

[PATCH 09/10] index: support --verbose option

[Eric Wong]

It doesn't implement progress of batches, yet, but it wires
up the parsing of the command-line while preserving output
compatibility.

This output is NOT meant to be stable.
---
 lib/PublicInbox/Admin.pm  | 18 ++++++++++++++++++
 lib/PublicInbox/Xapcmd.pm | 14 +-------------
 script/public-inbox-index |  5 +++--
 3 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/lib/PublicInbox/Admin.pm b/lib/PublicInbox/Admin.pm
index 34aa312..07d8b57 100644
--- a/lib/PublicInbox/Admin.pm
+++ b/lib/PublicInbox/Admin.pm
@@ -167,4 +167,22 @@ sub index_inbox {
 	}
 }
 
+sub progress_prepare ($) {
+	my ($opt) = @_;
+
+	# public-inbox-index defaults to quiet, -xcpdb and -compact do not
+	if (defined($opt->{quiet}) && $opt->{quiet} < 0) {
+		$opt->{quiet} = !$opt->{verbose};
+	}
+	if ($opt->{quiet}) {
+		open my $null, '>', '/dev/null' or
+			die "failed to open /dev/null: $!\n";
+		$opt->{1} = fileno($null); # suitable for spawn() redirect
+		$opt->{-dev_null} = $null;
+	} else {
+		$opt->{verbose} ||= 1;
+		$opt->{-progress} = sub { print STDERR @_ };
+	}
+}
+
 1;
diff --git a/lib/PublicInbox/Xapcmd.pm b/lib/PublicInbox/Xapcmd.pm
index 999819c..7e3d47f 100644
--- a/lib/PublicInbox/Xapcmd.pm
+++ b/lib/PublicInbox/Xapcmd.pm
@@ -86,18 +86,6 @@ sub prepare_reindex ($$) {
 	}
 }
 
-sub progress_prepare ($) {
-	my ($opt) = @_;
-	if ($opt->{quiet}) {
-		open my $null, '>', '/dev/null' or
-			die "failed to open /dev/null: $!\n";
-		$opt->{1} = fileno($null);
-		$opt->{-dev_null} = $null;
-	} else {
-		$opt->{-progress} = sub { print STDERR @_ };
-	}
-}
-
 sub same_fs_or_die ($$) {
 	my ($x, $y) = @_;
 	return if ((stat($x))[0] == (stat($y))[0]); # 0 - st_dev
@@ -132,7 +120,7 @@ sub process_queue {
 sub run {
 	my ($ibx, $task, $opt) = @_; # task = 'cpdb' or 'compact'
 	my $cb = \&${\"PublicInbox::Xapcmd::$task"};
-	progress_prepare($opt ||= {});
+	PublicInbox::Admin::progress_prepare($opt ||= {});
 	my $dir = $ibx->{mainrepo} or die "no mainrepo in inbox\n";
 	runnable_or_die($XAPIAN_COMPACT) if $opt->{compact};
 	my $reindex; # v1:{ from => $x40 }, v2:{ from => [ $x40, $x40, .. ] } }
diff --git a/script/public-inbox-index b/script/public-inbox-index
index 40187b3..c0d637b 100755
--- a/script/public-inbox-index
+++ b/script/public-inbox-index
@@ -13,8 +13,8 @@ my $usage = "public-inbox-index INBOX_DIR";
 use PublicInbox::Admin;
 PublicInbox::Admin::require_or_die('-index');
 
-my $opt = {};
-GetOptions($opt, qw(reindex jobs|j=i prune indexlevel|L=s))
+my $opt = { quiet => -1 };
+GetOptions($opt, qw(verbose|v+ reindex jobs|j=i prune indexlevel|L=s))
 	or die "bad command-line args\n$usage";
 die "--jobs must be positive\n" if defined $opt->{jobs} && $opt->{jobs} <= 0;
 
@@ -35,4 +35,5 @@ foreach my $ibx (@ibxs) {
 }
 
 PublicInbox::Admin::require_or_die(keys %$mods);
+PublicInbox::Admin::progress_prepare($opt);
 PublicInbox::Admin::index_inbox($_, $opt) for @ibxs;
-- 
EW

[PATCH 10/10] v2writable: show progress updates for index_sync

[Eric Wong]

We can show progress whenever we commit changes to the FS.
---
 lib/PublicInbox/V2Writable.pm | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm
index aa180a2..6b01171 100644
--- a/lib/PublicInbox/V2Writable.pm
+++ b/lib/PublicInbox/V2Writable.pm
@@ -779,12 +779,16 @@ sub reindex_oid ($$$$) {
 	my $idx = $self->idx_part($part);
 	$idx->index_raw($len, $msgref, $num, $oid, $mid0, $mime);
 	my $n = $self->{transact_bytes} += $len;
+	$sync->{nr}++;
 	if ($n > (PublicInbox::SearchIdx::BATCH_BYTES * $nparts)) {
 		$git->cleanup;
 		$sync->{mm_tmp}->atfork_prepare;
 		$self->done; # release lock
 
-		# TODO: print progress info, here
+		if (my $pr = $sync->{-opt}->{-progress}) {
+			my ($bn) = (split('/', $git->{git_dir}))[-1];
+			$pr->("$bn ".sprintf($sync->{-regen_fmt}, $sync->{nr}));
+		}
 
 		# allow -watch or -mda to write...
 		$self->idx_init; # reacquire lock
@@ -820,16 +824,27 @@ sub last_commits ($$) {
 # returns a revision range for git-log(1)
 sub log_range ($$$$$) {
 	my ($self, $sync, $git, $i, $tip) = @_;
-	my $cur = $sync->{ranges}->[$i] or return $tip; # all of it
+	my $opt = $sync->{-opt};
+	my $pr = $opt->{-progress} if (($opt->{verbose} || 0) > 1);
+	my $cur = $sync->{ranges}->[$i] or do {
+		$pr->("$i.git indexing all of $tip") if $pr;
+		return $tip; # all of it
+	};
+
 	my $range = "$cur..$tip";
+	$pr->("$i.git checking contiguity... ") if $pr;
 	if (is_ancestor($git, $cur, $tip)) { # common case
+		$pr->("OK\n") if $pr;
 		my $n = $git->qx(qw(rev-list --count), $range);
 		chomp($n);
 		if ($n == 0) {
 			$sync->{ranges}->[$i] = undef;
+			$pr->("$i.git has nothing new\n") if $pr;
 			return; # nothing to do
 		}
+		$pr->("$i.git has $n changes since $cur\n") if $pr;
 	} else {
+		$pr->("FAIL\n") if $pr;
 		warn <<"";
 discontiguous range: $range
 Rewritten history? (in $git->{git_dir})
@@ -876,7 +891,7 @@ sub sync_prepare ($$$) {
 		$sync->{ranges}->[$i] = $range;
 
 		# can't use 'rev-list --count' if we use --diff-filter
-		$pr->("$i.git counting changes\n\t$range ... ") if $pr;
+		$pr->("$i.git counting $range ... ") if $pr;
 		my $n = 0;
 		my $fh = $git->popen(qw(log --pretty=tformat:%H
 				--no-notes --no-color --no-renames
@@ -887,6 +902,9 @@ sub sync_prepare ($$$) {
 	}
 	# reindex should NOT see new commits anymore, if we do,
 	# it's a problem and we need to notice it via die()
+	my $pad = length($regen_max) + 1;
+	$sync->{-regen_fmt} = "% ${pad}u/$regen_max\n";
+	$sync->{nr} = 0;
 	return -1 if $sync->{reindex};
 	$regen_max + $self->{mm}->num_highwater() || 0;
 }
@@ -967,6 +985,7 @@ sub sync_ranges ($$$) {
 sub index_sync {
 	my ($self, $opt) = @_;
 	$opt ||= {};
+	my $pr = $opt->{-progress};
 	my $epoch_max;
 	my $latest = git_dir_latest($self, \$epoch_max);
 	return unless defined $latest;
@@ -993,6 +1012,7 @@ sub index_sync {
 		my $unindex_range = delete $sync->{"unindex-range.$i"};
 		unindex($self, $sync, $git, $unindex_range) if $unindex_range;
 		defined(my $range = $sync->{ranges}->[$i]) or next;
+		$pr->("$i.git indexing $range\n") if $pr;
 		my $fh = $self->{reindex_pipe} = $git->popen(@cmd, $range);
 		my $cmt;
 		while (<$fh>) {
@@ -1019,6 +1039,9 @@ sub index_sync {
 		$git->cleanup;
 	}
 	$self->done;
+	if (my $pr = $sync->{-opt}->{-progress}) {
+		$pr->('all.git '.sprintf($sync->{-regen_fmt}, $sync->{nr}));
+	}
 
 	# reindex does not pick up new changes, so we rerun w/o it:
 	if ($opt->{reindex}) {
-- 
EW