Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladimir Davydov <vdavydov.dev@gmail.com>
To: kostja@tarantool.org
Cc: tarantool-patches@freelists.org
Subject: [PATCH v2 05/10] box: move checkpointing to gc module
Date: Sat,  8 Dec 2018 18:48:09 +0300	[thread overview]
Message-ID: <f2aeb399c560a0fc017a510f0620f215a22839f3.1544282224.git.vdavydov.dev@gmail.com> (raw)
In-Reply-To: <cover.1544282224.git.vdavydov.dev@gmail.com>
In-Reply-To: <cover.1544282224.git.vdavydov.dev@gmail.com>

Garbage collection module seems to be the best way to accommodate the
checkpoint daemon, but to move it there, we first need to move the code
performing checkpoints there to avoid cross-dependency between box.cc
and gc.c.
---
 src/box/box.cc | 44 +------------------------------------
 src/box/gc.c   | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 src/box/gc.h   | 28 ++++++++++++++++--------
 3 files changed, 87 insertions(+), 53 deletions(-)

diff --git a/src/box/box.cc b/src/box/box.cc
index d146ec16..121ad787 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -90,11 +90,6 @@ static void title(const char *new_status)
 const struct vclock *box_vclock = &replicaset.vclock;
 
 /**
- * Set if there's a fiber performing box_checkpoint() right now.
- */
-static bool checkpoint_is_in_progress;
-
-/**
  * Set if backup is in progress, i.e. box_backup_start() was
  * called but box_backup_stop() hasn't been yet.
  */
@@ -2185,45 +2180,8 @@ box_checkpoint()
 	/* Signal arrived before box.cfg{} */
 	if (! is_box_configured)
 		return 0;
-	int rc = 0;
-	if (checkpoint_is_in_progress) {
-		diag_set(ClientError, ER_CHECKPOINT_IN_PROGRESS);
-		return -1;
-	}
-	checkpoint_is_in_progress = true;
-	/* create checkpoint files */
-	latch_lock(&schema_lock);
-	if ((rc = engine_begin_checkpoint()))
-		goto end;
-
-	struct vclock vclock;
-	if ((rc = wal_begin_checkpoint(&vclock)))
-		goto end;
 
-	if ((rc = engine_commit_checkpoint(&vclock)))
-		goto end;
-
-	wal_commit_checkpoint(&vclock);
-	gc_add_checkpoint(&vclock);
-end:
-	if (rc)
-		engine_abort_checkpoint();
-
-	latch_unlock(&schema_lock);
-	checkpoint_is_in_progress = false;
-
-	/*
-	 * Wait for background garbage collection that might
-	 * have been triggered by this checkpoint to complete.
-	 * Strictly speaking, it isn't necessary, but it
-	 * simplifies testing as it guarantees that by the
-	 * time box.snapshot() returns, all outdated checkpoint
-	 * files have been removed.
-	 */
-	if (!rc)
-		gc_wait();
-
-	return rc;
+	return gc_checkpoint();
 }
 
 int
diff --git a/src/box/gc.c b/src/box/gc.c
index 87273b8d..153ef65c 100644
--- a/src/box/gc.c
+++ b/src/box/gc.c
@@ -45,11 +45,14 @@
 #include <small/rlist.h>
 
 #include "diag.h"
+#include "errcode.h"
 #include "fiber.h"
 #include "fiber_cond.h"
+#include "latch.h"
 #include "say.h"
 #include "vclock.h"
 #include "cbus.h"
+#include "schema.h"
 #include "engine.h"		/* engine_collect_garbage() */
 #include "wal.h"		/* wal_collect_garbage() */
 
@@ -242,7 +245,11 @@ gc_schedule(void)
 		fiber_wakeup(gc.fiber);
 }
 
-void
+/**
+ * Wait for background garbage collection scheduled prior
+ * to this point to complete.
+ */
+static void
 gc_wait(void)
 {
 	unsigned scheduled = gc.scheduled;
@@ -320,6 +327,65 @@ gc_add_checkpoint(const struct vclock *vclock)
 	gc_schedule();
 }
 
+int
+gc_checkpoint(void)
+{
+	int rc;
+	struct vclock vclock;
+
+	if (gc.checkpoint_is_in_progress) {
+		diag_set(ClientError, ER_CHECKPOINT_IN_PROGRESS);
+		return -1;
+	}
+	gc.checkpoint_is_in_progress = true;
+
+	/*
+	 * We don't support DDL operations while making a checkpoint.
+	 * Lock them out.
+	 */
+	latch_lock(&schema_lock);
+
+	/*
+	 * Rotate WAL and call engine callbacks to create a checkpoint
+	 * on disk for each registered engine.
+	 */
+	rc = engine_begin_checkpoint();
+	if (rc != 0)
+		goto out;
+	rc = wal_begin_checkpoint(&vclock);
+	if (rc != 0)
+		goto out;
+	rc = engine_commit_checkpoint(&vclock);
+	if (rc != 0)
+		goto out;
+	wal_commit_checkpoint(&vclock);
+
+	/*
+	 * Finally, track the newly created checkpoint in the garbage
+	 * collector state.
+	 */
+	gc_add_checkpoint(&vclock);
+out:
+	if (rc != 0)
+		engine_abort_checkpoint();
+
+	latch_unlock(&schema_lock);
+	gc.checkpoint_is_in_progress = false;
+
+	/*
+	 * Wait for background garbage collection that might
+	 * have been triggered by this checkpoint to complete.
+	 * Strictly speaking, it isn't necessary, but it
+	 * simplifies testing as it guarantees that by the
+	 * time box.snapshot() returns, all outdated checkpoint
+	 * files have been removed.
+	 */
+	if (rc == 0)
+		gc_wait();
+
+	return rc;
+}
+
 void
 gc_ref_checkpoint(struct gc_checkpoint *checkpoint,
 		  struct gc_checkpoint_ref *ref, const char *format, ...)
diff --git a/src/box/gc.h b/src/box/gc.h
index a141ace6..84a441f2 100644
--- a/src/box/gc.h
+++ b/src/box/gc.h
@@ -31,6 +31,7 @@
  * SUCH DAMAGE.
  */
 
+#include <stdbool.h>
 #include <stddef.h>
 #include <small/rlist.h>
 
@@ -141,6 +142,10 @@ struct gc_state {
 	 * taken at that moment of time.
 	 */
 	unsigned completed, scheduled;
+	/**
+	 * Set if there's a fiber making a checkpoint right now.
+	 */
+	bool checkpoint_is_in_progress;
 };
 extern struct gc_state gc;
 
@@ -192,13 +197,6 @@ void
 gc_free(void);
 
 /**
- * Wait for background garbage collection scheduled prior
- * to this point to complete.
- */
-void
-gc_wait(void);
-
-/**
  * Advance the garbage collector vclock to the given position.
  * Deactivate WAL consumers that need older data.
  */
@@ -217,14 +215,26 @@ void
 gc_set_min_checkpoint_count(int min_checkpoint_count);
 
 /**
- * Track a new checkpoint in the garbage collector state.
- * Note, this function may run garbage collector to remove
+ * Track an existing checkpoint in the garbage collector state.
+ * Note, this function may trigger garbage collection to remove
  * old checkpoints.
  */
 void
 gc_add_checkpoint(const struct vclock *vclock);
 
 /**
+ * Make a checkpoint.
+ *
+ * This function runs engine/WAL callbacks to create a checkpoint
+ * on disk, then tracks the new checkpoint in the garbage collector
+ * state (see gc_add_checkpoint()).
+ *
+ * Returns 0 on success. On failure returns -1 and sets diag.
+ */
+int
+gc_checkpoint(void);
+
+/**
  * Get a reference to @checkpoint and store it in @ref.
  * This will block the garbage collector from deleting
  * the checkpoint files until the reference is released
-- 
2.11.0

  parent reply	other threads:[~2018-12-08 15:48 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-08 15:48 [PATCH v2 00/10] Allow to limit size of WAL files Vladimir Davydov
2018-12-08 15:48 ` [PATCH v2 01/10] gc: do not use WAL watcher API for deactivating stale consumers Vladimir Davydov
2018-12-08 21:41   ` Konstantin Osipov
2018-12-08 15:48 ` [PATCH v2 02/10] wal: simplify watcher API Vladimir Davydov
2018-12-08 21:41   ` Konstantin Osipov
2018-12-08 15:48 ` [PATCH v2 03/10] box: fix certain cfg options initialized twice on recovery Vladimir Davydov
2018-12-08 21:42   ` Konstantin Osipov
2018-12-08 15:48 ` [PATCH v2 04/10] box: don't use box_checkpoint_is_in_progress outside box.cc Vladimir Davydov
2018-12-08 21:43   ` Konstantin Osipov
2018-12-08 15:48 ` Vladimir Davydov [this message]
2018-12-08 21:44   ` [PATCH v2 05/10] box: move checkpointing to gc module Konstantin Osipov
2018-12-08 15:48 ` [PATCH v2 06/10] gc: some renames Vladimir Davydov
2018-12-08 21:44   ` Konstantin Osipov
2018-12-08 15:48 ` [PATCH v2 07/10] Introduce checkpoint schedule module Vladimir Davydov
2018-12-08 21:45   ` Konstantin Osipov
2018-12-08 15:48 ` [PATCH v2 08/10] Rewrite checkpoint daemon in C Vladimir Davydov
2018-12-08 21:45   ` Konstantin Osipov
2018-12-08 15:48 ` [PATCH v2 09/10] wal: pass struct instead of vclock to checkpoint methods Vladimir Davydov
2018-12-08 21:46   ` Konstantin Osipov
2018-12-08 15:48 ` [PATCH v2 10/10] wal: trigger checkpoint if there are too many WALs Vladimir Davydov
2018-12-08 21:48   ` Konstantin Osipov
2018-12-09 11:20 ` [PATCH v2 00/10] Allow to limit size of WAL files Vladimir Davydov

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

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

  git send-email \
    --in-reply-to=f2aeb399c560a0fc017a510f0620f215a22839f3.1544282224.git.vdavydov.dev@gmail.com \
    --to=vdavydov.dev@gmail.com \
    --cc=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [PATCH v2 05/10] box: move checkpointing to gc module' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox