user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
From: "Eric Wong (Contractor, The Linux Foundation)" <e@80x24.org>
To: meta@public-inbox.org
Subject: [PATCH 2/5] v2writable: simplify barrier vs checkpoints
Date: Mon,  2 Apr 2018 00:04:53 +0000	[thread overview]
Message-ID: <20180402000456.13446-3-e@80x24.org> (raw)
In-Reply-To: <20180402000456.13446-1-e@80x24.org>

searchidx_checkpoint was too convoluted and confusing.
Since barrier is mostly the same thing; use that instead
and add an fsync option for the overview DB.
---
 lib/PublicInbox/OverIdxFork.pm | 15 +++++------
 lib/PublicInbox/V2Writable.pm  | 58 ++++++++++++++++--------------------------
 2 files changed, 28 insertions(+), 45 deletions(-)

diff --git a/lib/PublicInbox/OverIdxFork.pm b/lib/PublicInbox/OverIdxFork.pm
index f4f7cdd..ec96528 100644
--- a/lib/PublicInbox/OverIdxFork.pm
+++ b/lib/PublicInbox/OverIdxFork.pm
@@ -135,9 +135,12 @@ sub barrier_init {
 
 sub barrier_wait {
 	my ($self) = @_;
-	my $bw = $self->{barrier_wait} or return;
-	my $l = $bw->getline;
-	$l eq "barrier_done\n" or die "bad response from barrier_wait: $l\n";
+	if (my $bw = $self->{barrier_wait}) {
+		my $l = $bw->getline;
+		$l eq "barrier_done\n" or die "bad response from barrier_wait: $l\n";
+	} else {
+		$self->commit_lazy;
+	}
 }
 
 sub remote_commit {
@@ -174,10 +177,4 @@ sub remote_close {
 	}
 }
 
-sub commit_fsync {
-	my ($self) = @_;
-	return if $self->{w}; # don't bother; main parent can also call this
-	$self->SUPER::commit_fsync;
-}
-
 1;
diff --git a/lib/PublicInbox/V2Writable.pm b/lib/PublicInbox/V2Writable.pm
index 8e3122a..479e2b5 100644
--- a/lib/PublicInbox/V2Writable.pm
+++ b/lib/PublicInbox/V2Writable.pm
@@ -303,24 +303,39 @@ sub purge {
 
 sub done {
 	my ($self) = @_;
-	my $locked = defined $self->{idx_parts};
 	my $im = delete $self->{im};
 	$im->done if $im; # PublicInbox::Import::done
-	$self->searchidx_checkpoint(0);
-	$self->lock_release if $locked;
+
+	if (my $mm = delete $self->{mm}) {
+		$mm->{dbh}->commit;
+	}
+
+	# order matters, we can only close {over} after all partitions
+	# are done because the partitions also write to {over}
+	my $parts = delete $self->{idx_parts};
+	if ($parts) {
+		$_->remote_commit for @$parts;
+		$_->remote_close for @$parts;
+	}
+
+	my $over = $self->{over};
+	$over->remote_commit;
+	$over->remote_close;
+	$self->{transact_bytes} = 0;
+	$self->lock_release if $parts;
 }
 
 sub checkpoint {
 	my ($self) = @_;
 	my $im = $self->{im};
 	$im->checkpoint if $im; # PublicInbox::Import::checkpoint
-	$self->searchidx_checkpoint(1);
+	$self->barrier(1);
 }
 
 # 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
 sub barrier {
-	my ($self) = @_;
+	my ($self, $fsync) = @_;
 
 	if (my $im = $self->{im}) {
 		$im->barrier;
@@ -339,42 +354,13 @@ sub barrier {
 		$_->remote_barrier foreach @$parts;
 
 		$over->barrier_wait; # wait for each Xapian partition
+		$over->commit_fsync if $fsync;
 
 		$dbh->begin_work;
 	}
 	$self->{transact_bytes} = 0;
 }
 
-sub searchidx_checkpoint {
-	my ($self, $more) = @_;
-
-	# order matters, we can only close {over} after all partitions
-	# are done because the partitions also write to {over}
-	if (my $parts = $self->{idx_parts}) {
-		foreach my $idx (@$parts) {
-			$idx->remote_commit; # propagates commit to over
-			$idx->remote_close unless $more;
-		}
-		delete $self->{idx_parts} unless $more;
-	}
-
-	if (my $mm = $self->{mm}) {
-		my $dbh = $mm->{dbh};
-		$dbh->commit;
-		if ($more) {
-			$dbh->begin_work;
-		} else {
-			delete $self->{mm};
-		}
-	}
-	my $over = $self->{over};
-	$over->remote_commit;
-	if (!$more) {
-		$over->remote_close;
-	}
-	$self->{transact_bytes} = 0;
-}
-
 sub git_init {
 	my ($self, $new) = @_;
 	my $pfx = "$self->{-inbox}->{mainrepo}/git";
@@ -435,7 +421,7 @@ sub importer {
 		} else {
 			$self->{im} = undef;
 			$im->done;
-			$self->searchidx_checkpoint(1);
+			$self->barrier(1);
 			$im = undef;
 			my $git_dir = $self->git_init(++$self->{max_git});
 			my $git = PublicInbox::Git->new($git_dir);
-- 
EW


  parent reply	other threads:[~2018-04-02  0:04 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-02  0:04 [PATCH 0/5] v2: drop Xapian skeleton for SQLite overview DB Eric Wong (Contractor, The Linux Foundation)
2018-04-02  0:04 ` [PATCH 1/5] replace Xapian skeleton with " Eric Wong (Contractor, The Linux Foundation)
2018-04-05  8:59   ` Eric Wong
2018-04-02  0:04 ` Eric Wong (Contractor, The Linux Foundation) [this message]
2018-04-02  0:04 ` [PATCH 3/5] t/over: test empty Subject: line matching Eric Wong (Contractor, The Linux Foundation)
2018-04-02  0:04 ` [PATCH 4/5] www: rework query responses to avoid COUNT in SQLite Eric Wong (Contractor, The Linux Foundation)
2018-04-02  0:04 ` [PATCH 5/5] over: speedup get_thread by avoiding JOIN Eric Wong (Contractor, The Linux Foundation)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://public-inbox.org/README

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180402000456.13446-3-e@80x24.org \
    --to=e@80x24.org \
    --cc=meta@public-inbox.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).