user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
Search results ordered by [date|relevance]  view[summary|nested|Atom feed]
thread overview below | download mbox.gz: |
* [PATCH 3/3] xcpdb: avoid race when shards are added
  2021-09-23  5:53  6% [PATCH 0/3] xcpdb-related fixes Eric Wong
@ 2021-09-23  5:53  7% ` Eric Wong
  0 siblings, 0 replies; 2+ results
From: Eric Wong @ 2021-09-23  5:53 UTC (permalink / raw)
  To: meta

It's possible for the rename() sequence to cause read-only
daemons using ->xdb_shards_flat to load an incomplete set of
contiguous shards and get invalid docids for search results.

With this change, we favor the case where search is momentarily
unavailable rather than giving wrong results during the small
window where Xapcmd->commit_changes runs.
---
 lib/PublicInbox/Xapcmd.pm | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/lib/PublicInbox/Xapcmd.pm b/lib/PublicInbox/Xapcmd.pm
index daef896c..44e0f8e5 100644
--- a/lib/PublicInbox/Xapcmd.pm
+++ b/lib/PublicInbox/Xapcmd.pm
@@ -20,16 +20,23 @@ sub commit_changes ($$$$) {
 	my $reshard = $opt->{reshard};
 
 	$SIG{INT} or die 'BUG: $SIG{INT} not handled';
-	my @old_shard;
-	my $over_chg;
-	my $mode;
-
-	while (my ($old, $newdir) = each %$tmp) {
+	my (@old_shard, $over_chg);
+
+	# Sort shards highest-to-lowest, since ->xdb_shards_flat
+	# determines the number of shards to load based on the max;
+	# and we'd rather xdb_shards_flat to momentarily fail rather
+	# than load out-of-date shards
+	my @order = sort {
+		my ($x) = ($a =~ m!/([0-9]+)/*\z!);
+		my ($y) = ($b =~ m!/([0-9]+)/*\z!);
+		($y // -1) <=> ($x // -1) # we may have non-shards
+	} keys %$tmp;
+
+	my ($dname) = ($order[0] =~ m!(.*/)[^/]+/*\z!);
+	my $mode = (stat($dname))[2];
+	for my $old (@order) {
 		next if $old eq ''; # no invalid paths
-		$mode //= do {
-			my ($dname) = ($old =~ m!(.*/)[^/]+/*\z!);
-			(stat($dname))[2];
-		};
+		my $newdir = $tmp->{$old};
 		my $have_old = -e $old;
 		if (!$have_old && !defined($opt->{reshard})) {
 			die "failed to stat($old): $!";
@@ -57,11 +64,7 @@ sub commit_changes ($$$$) {
 					die "rename $old => $new/old: $!\n";
 		}
 		rename($new, $old) or die "rename $new => $old: $!\n";
-		if ($have_old) {
-			my $prev = "$old/old";
-			remove_tree($prev) or
-				die "failed to remove $prev: $!\n";
-		}
+		push @old_shard, "$old/old" if $have_old;
 	}
 
 	# trigger ->check_inodes in read-only daemons

^ permalink raw reply related	[relevance 7%]

* [PATCH 0/3] xcpdb-related fixes
@ 2021-09-23  5:53  6% Eric Wong
  2021-09-23  5:53  7% ` [PATCH 3/3] xcpdb: avoid race when shards are added Eric Wong
  0 siblings, 1 reply; 2+ results
From: Eric Wong @ 2021-09-23  5:53 UTC (permalink / raw)
  To: meta

Some bugs I discovered while getting cleanup stuff working
better for read-only daemons.

Eric Wong (3):
  test_common: reset umask on non-forking run_script
  xcpdb: -R$SHARDS creates new shards with correct perms
  xcpdb: avoid race when shards are added

 lib/PublicInbox/TestCommon.pm |  2 ++
 lib/PublicInbox/Xapcmd.pm     | 34 +++++++++++++++++++++-------------
 t/extsearch.t                 |  7 +++++++
 3 files changed, 30 insertions(+), 13 deletions(-)

^ permalink raw reply	[relevance 6%]

Results 1-2 of 2 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2021-09-23  5:53  6% [PATCH 0/3] xcpdb-related fixes Eric Wong
2021-09-23  5:53  7% ` [PATCH 3/3] xcpdb: avoid race when shards are added Eric Wong

Code repositories for project(s) associated with this public inbox

	https://80x24.org/public-inbox.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).