unofficial mirror of meta@public-inbox.org
 help / color / mirror / Atom feed
* [PATCH 0/3] xcpdb-related fixes
@ 2021-09-23  5:53 Eric Wong
  2021-09-23  5:53 ` [PATCH 1/3] test_common: reset umask on non-forking run_script Eric Wong
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
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	[flat|nested] 4+ messages in thread

* [PATCH 1/3] test_common: reset umask on non-forking run_script
  2021-09-23  5:53 [PATCH 0/3] xcpdb-related fixes Eric Wong
@ 2021-09-23  5:53 ` Eric Wong
  2021-09-23  5:53 ` [PATCH 2/3] xcpdb: -R$SHARDS creates new shards with correct perms Eric Wong
  2021-09-23  5:53 ` [PATCH 3/3] xcpdb: avoid race when shards are added Eric Wong
  2 siblings, 0 replies; 4+ messages in thread
From: Eric Wong @ 2021-09-23  5:53 UTC (permalink / raw)
  To: meta

public-inbox-init sets umask for git <2.1.0, so our fork+exec
replacement needs to restore the original umask of the "parent".
---
 lib/PublicInbox/TestCommon.pm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index 92a7db36..aff34853 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -316,6 +316,7 @@ sub run_script ($;$$) {
 	} else { # localize and run everything in the same process:
 		# note: "local *STDIN = *STDIN;" and so forth did not work in
 		# old versions of perl
+		my $umask = umask;
 		local %ENV = $env ? (%ENV, %$env) : %ENV;
 		local @SIG{keys %SIG} = map { undef } values %SIG;
 		local $SIG{FPE} = 'IGNORE'; # Perl default
@@ -333,6 +334,7 @@ sub run_script ($;$$) {
 		die "fchdir(restore): $!" if $cwdfh && !chdir($cwdfh);
 		_undo_redirects($orig_io);
 		select STDOUT;
+		umask($umask);
 	}
 
 	# slurp the redirects back into user-supplied strings

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/3] xcpdb: -R$SHARDS creates new shards with correct perms
  2021-09-23  5:53 [PATCH 0/3] xcpdb-related fixes Eric Wong
  2021-09-23  5:53 ` [PATCH 1/3] test_common: reset umask on non-forking run_script Eric Wong
@ 2021-09-23  5:53 ` Eric Wong
  2021-09-23  5:53 ` [PATCH 3/3] xcpdb: avoid race when shards are added Eric Wong
  2 siblings, 0 replies; 4+ messages in thread
From: Eric Wong @ 2021-09-23  5:53 UTC (permalink / raw)
  To: meta

"Correct" meaning the permissions match that of the parent
xap15 or ei15 directory.
---
 lib/PublicInbox/Xapcmd.pm | 15 ++++++++++-----
 t/extsearch.t             |  7 +++++++
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/lib/PublicInbox/Xapcmd.pm b/lib/PublicInbox/Xapcmd.pm
index b962fa84..daef896c 100644
--- a/lib/PublicInbox/Xapcmd.pm
+++ b/lib/PublicInbox/Xapcmd.pm
@@ -22,11 +22,16 @@ sub commit_changes ($$$$) {
 	$SIG{INT} or die 'BUG: $SIG{INT} not handled';
 	my @old_shard;
 	my $over_chg;
+	my $mode;
 
 	while (my ($old, $newdir) = each %$tmp) {
 		next if $old eq ''; # no invalid paths
-		my @st = stat($old);
-		if (!@st && !defined($opt->{reshard})) {
+		$mode //= do {
+			my ($dname) = ($old =~ m!(.*/)[^/]+/*\z!);
+			(stat($dname))[2];
+		};
+		my $have_old = -e $old;
+		if (!$have_old && !defined($opt->{reshard})) {
 			die "failed to stat($old): $!";
 		}
 
@@ -46,13 +51,13 @@ sub commit_changes ($$$$) {
 			next;
 		}
 
-		if (@st) {
-			chmod($st[2] & 07777, $new) or die "chmod $old: $!\n";
+		chmod($mode & 07777, $new) or die "chmod($new): $!\n";
+		if ($have_old) {
 			rename($old, "$new/old") or
 					die "rename $old => $new/old: $!\n";
 		}
 		rename($new, $old) or die "rename $new => $old: $!\n";
-		if (@st) {
+		if ($have_old) {
 			my $prev = "$old/old";
 			remove_tree($prev) or
 				die "failed to remove $prev: $!\n";
diff --git a/t/extsearch.t b/t/extsearch.t
index ad4f2c6d..b2b994f6 100644
--- a/t/extsearch.t
+++ b/t/extsearch.t
@@ -423,6 +423,7 @@ if ('dedupe + dry-run') {
 		'--dry-run alone fails');
 }
 
+# chmod 0755, $home or xbail "chmod: $!";
 for my $j (1, 3, 6) {
 	my $o = { 2 => \(my $err = '') };
 	my $d = "$home/extindex-j$j";
@@ -436,11 +437,17 @@ for my $j (1, 3, 6) {
 
 SKIP: {
 	my $d = "$home/extindex-j1";
+	my @ei_dir = glob("$d/ei*/");
+	chmod 0755, $ei_dir[0] or xbail "chmod: $!";
+	my $mode = sprintf('%04o', 07777 & (stat($ei_dir[0]))[2]);
+	is($mode, '0755', 'mode set on ei*/ dir');
 	my $o = { 2 => \(my $err = '') };
 	ok(run_script([qw(-xcpdb -R4), $d]), 'xcpdb R4');
 	my @dirs = glob("$d/ei*/?");
 	for my $i (0..3) {
 		is(grep(m!/ei[0-9]+/$i\z!, @dirs), 1, "shard [$i] created");
+		my $m = sprintf('%04o', 07777 & (stat($dirs[$i]))[2]);
+		is($m, $mode, "shard [$i] mode");
 	}
 	for my $i (4..5) {
 		is(grep(m!/ei[0-9]+/$i\z!, @dirs), 0, "no shard [$i]");

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 3/3] xcpdb: avoid race when shards are added
  2021-09-23  5:53 [PATCH 0/3] xcpdb-related fixes Eric Wong
  2021-09-23  5:53 ` [PATCH 1/3] test_common: reset umask on non-forking run_script Eric Wong
  2021-09-23  5:53 ` [PATCH 2/3] xcpdb: -R$SHARDS creates new shards with correct perms Eric Wong
@ 2021-09-23  5:53 ` Eric Wong
  2 siblings, 0 replies; 4+ messages in thread
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	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-09-23  5:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-23  5:53 [PATCH 0/3] xcpdb-related fixes Eric Wong
2021-09-23  5:53 ` [PATCH 1/3] test_common: reset umask on non-forking run_script Eric Wong
2021-09-23  5:53 ` [PATCH 2/3] xcpdb: -R$SHARDS creates new shards with correct perms Eric Wong
2021-09-23  5:53 ` [PATCH 3/3] xcpdb: avoid race when shards are added Eric Wong

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).