From 90fb237265749cfcbaade6f1a6c9f43d0755de28 Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Date: Wed, 12 Jul 2017 16:46:53 +0300
Subject: [PATCH] vinyl: refactor index recovery from vylog
We have vy_recovery_lookup_index() function to look up an index in a
recovery context by id and vy_recovery_iterate_index() to iterate over
ranges, runs, and slices of a found index. vy_recovery_lookup_index()
used to be a part of vy_recovery_iterate_index() and was factored out
when index logging was moved to be called after WAL write, from
vy_index_commit_create(), because during recovery we need to check if an
index creation record was flushed to vylog before restart - currently we
do it by trying to look it up in the recovery context.
To stop using index lsn as vylog index id and remove lsn from index
options, I'm planning to make the function loading an index from vylog
advance an internal vylog counter so that the next time it is called it
loads a newer incarnation of the same index. vy_recovery_lookup_index()
doesn't fit this concept. So I introduce vy_recovery_load_index() that
calls vy_recovery_lookup_index() and vy_recovery_iterate_index()
internally and make the two functions private to vylog. To deal with
indexes not logged due to vylog errors, I introduce a per index flag,
vy_index->is_committed, which is set if the index record was flushed to
vylog - the same approach is already used to handle index drop (see
vy_index_commit_drop()).
---
src/box/vinyl.c | 5 +++--
src/box/vy_index.c | 47 +++++++++++++++++++++++++++-------------------
src/box/vy_index.h | 5 +++++
src/box/vy_log.c | 15 +++++++++++++--
src/box/vy_log.h | 47 ++++++++++++++++++----------------------------
5 files changed, 67 insertions(+), 52 deletions(-)
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index 18e1b23c1c..f1c3f126ca 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -2500,13 +2500,14 @@ vy_index_commit_create(struct vy_env *env, struct vy_index *index)
* the index isn't in the recovery context and we
* need to retry to log it now.
*/
- if (vy_recovery_lookup_index(env->recovery,
- index->opts.lsn) != NULL) {
+ if (index->is_committed) {
vy_scheduler_add_index(env->scheduler, index);
return;
}
}
+ index->is_committed = true;
+
assert(index->range_count == 1);
struct vy_range *range = vy_range_tree_first(index->tree);
diff --git a/src/box/vy_index.c b/src/box/vy_index.c
index 1dc4b46590..22950e6aa6 100644
--- a/src/box/vy_index.c
+++ b/src/box/vy_index.c
@@ -366,7 +366,7 @@ struct vy_index_recovery_cb_arg {
struct mh_i64ptr_t *run_hash;
};
-/** Index recovery callback, passed to vy_recovery_iterate_index(). */
+/** Index recovery callback, passed to vy_recovery_load_index(). */
static int
vy_index_recovery_cb(const struct vy_log_record *record, void *cb_arg)
{
@@ -380,6 +380,8 @@ vy_index_recovery_cb(const struct vy_log_record *record, void *cb_arg)
struct vy_slice *slice;
bool success = false;
+ assert(record->type == VY_LOG_CREATE_INDEX || index->is_committed);
+
if (record->type == VY_LOG_INSERT_RANGE ||
record->type == VY_LOG_INSERT_SLICE) {
if (record->begin != NULL) {
@@ -397,6 +399,7 @@ vy_index_recovery_cb(const struct vy_log_record *record, void *cb_arg)
switch (record->type) {
case VY_LOG_CREATE_INDEX:
assert(record->index_lsn == index->opts.lsn);
+ index->is_committed = true;
break;
case VY_LOG_DUMP_INDEX:
assert(record->index_lsn == index->opts.lsn);
@@ -478,18 +481,6 @@ vy_index_recover(struct vy_index *index, struct vy_recovery *recovery,
{
assert(index->range_count == 0);
- struct vy_index_recovery_info *ri;
- ri = vy_recovery_lookup_index(recovery, index->opts.lsn);
- if (ri == NULL) {
- if (!allow_missing) {
- diag_set(ClientError, ER_INVALID_VYLOG_FILE,
- tt_sprintf("Index %lld not found",
- (long long)index->opts.lsn));
- return -1;
- }
- return vy_index_create(index);
- }
-
struct vy_index_recovery_cb_arg arg = { .index = index };
arg.run_hash = mh_i64ptr_new();
if (arg.run_hash == NULL) {
@@ -497,27 +488,45 @@ vy_index_recover(struct vy_index *index, struct vy_recovery *recovery,
return -1;
}
- int rc = vy_recovery_iterate_index(ri, false,
- vy_index_recovery_cb, &arg);
+ int rc = vy_recovery_load_index(recovery, index->opts.lsn,
+ vy_index_recovery_cb, &arg);
mh_int_t k;
mh_foreach(arg.run_hash, k) {
struct vy_run *run = mh_i64ptr_node(arg.run_hash, k)->val;
- if (run->refs == 1) {
+ if (run->refs > 1)
+ vy_index_add_run(index, run);
+ if (run->refs == 1 && rc == 0) {
diag_set(ClientError, ER_INVALID_VYLOG_FILE,
tt_sprintf("Unused run %lld in index %lld",
(long long)run->id,
(long long)index->opts.lsn));
rc = -1;
- } else
- vy_index_add_run(index, run);
+ /*
+ * Continue the loop to unreference
+ * all runs in the hash.
+ */
+ }
/* Drop the reference held by the hash. */
vy_run_unref(run);
}
mh_i64ptr_delete(arg.run_hash);
- if (rc != 0)
+ if (rc != 0) {
+ /* Recovery callback failed. */
return -1;
+ }
+
+ if (!index->is_committed) {
+ /* Index was not found in the metadata log. */
+ if (!allow_missing) {
+ diag_set(ClientError, ER_INVALID_VYLOG_FILE,
+ tt_sprintf("Index %lld not found",
+ (long long)index->opts.lsn));
+ return -1;
+ }
+ return vy_index_create(index);
+ }
/*
* Account ranges to the index and check that the range tree
diff --git a/src/box/vy_index.h b/src/box/vy_index.h
index 68227a6ee6..187b73a77a 100644
--- a/src/box/vy_index.h
+++ b/src/box/vy_index.h
@@ -219,6 +219,11 @@ struct vy_index {
* been dumped yet.
*/
int64_t dump_lsn;
+ /**
+ * This flag is set if the index creation was
+ * committed to the metadata log.
+ */
+ bool is_committed;
/**
* This flag is set if the index was dropped.
* It is also set on local recovery if the index
diff --git a/src/box/vy_log.c b/src/box/vy_log.c
index e2e96a90f0..965a83a192 100644
--- a/src/box/vy_log.c
+++ b/src/box/vy_log.c
@@ -1227,7 +1227,7 @@ vy_log_write(const struct vy_log_record *record)
}
/** Lookup a vinyl index in vy_recovery::index_hash map. */
-struct vy_index_recovery_info *
+static struct vy_index_recovery_info *
vy_recovery_lookup_index(struct vy_recovery *recovery, int64_t index_lsn)
{
struct mh_i64ptr_t *h = recovery->index_hash;
@@ -2007,7 +2007,7 @@ vy_recovery_cb_call(vy_recovery_cb cb, void *cb_arg,
return cb(record, cb_arg);
}
-int
+static int
vy_recovery_iterate_index(struct vy_index_recovery_info *index,
bool include_deleted, vy_recovery_cb cb, void *cb_arg)
{
@@ -2152,3 +2152,14 @@ vy_recovery_iterate(struct vy_recovery *recovery, bool include_deleted,
}
return 0;
}
+
+int
+vy_recovery_load_index(struct vy_recovery *recovery, int64_t index_lsn,
+ vy_recovery_cb cb, void *cb_arg)
+{
+ struct vy_index_recovery_info *index;
+ index = vy_recovery_lookup_index(recovery, index_lsn);
+ if (index == NULL)
+ return 0;
+ return vy_recovery_iterate_index(index, false, cb, cb_arg);
+}
diff --git a/src/box/vy_log.h b/src/box/vy_log.h
index 1f529af1b4..06881fcb1a 100644
--- a/src/box/vy_log.h
+++ b/src/box/vy_log.h
@@ -59,7 +59,6 @@ struct vclock;
struct key_def;
struct vy_recovery;
-struct vy_index_recovery_info;
/** Type of a metadata log record. */
enum vy_log_record_type {
@@ -354,30 +353,18 @@ vy_recovery_new(int64_t signature, bool only_snapshot);
void
vy_recovery_delete(struct vy_recovery *recovery);
-/**
- * Lookup a vinyl index in a recovery context.
- *
- * Returns the index handle that can be used for recovering the
- * index structure with the aid of vy_recovery_iterate_index()
- * or NULL if the index is not found.
- */
-struct vy_index_recovery_info *
-vy_recovery_lookup_index(struct vy_recovery *recovery, int64_t index_lsn);
-
typedef int
(*vy_recovery_cb)(const struct vy_log_record *record, void *arg);
/**
- * Recover a vinyl index from a recovery context.
+ * Iterate over all objects stored in a recovery context.
+ *
+ * This function invokes callback @cb for each object (index, run, etc)
+ * stored in the given recovery context. The callback is passed a record
+ * used to log the object and optional argument @cb_arg. If the callback
+ * returns a value different from 0, iteration stops and -1 is returned,
+ * otherwise the function returns 0.
*
- * For each range and run of the index, this function calls @cb passing
- * a log record and an optional @cb_arg to it. A log record type is
- * either VY_LOG_CREATE_INDEX, VY_LOG_CREATE_RUN, VY_LOG_INSERT_RANGE,
- * or VY_LOG_INSERT_SLICE unless the index was dropped. In the latter case,
- * a VY_LOG_DROP_INDEX record is issued in the end.
- * The callback is supposed to rebuild the index structure and open run
- * files. If the callback returns a non-zero value, the function stops
- * iteration over ranges and runs and returns error.
* To ease the work done by the callback, records corresponding to
* slices of a range always go right after the range, in the
* chronological order, while an index's runs go after the index
@@ -386,22 +373,24 @@ typedef int
* If @include_deleted is set, this function will also iterate over
* deleted objects, issuing the corresponding "delete" record for each
* of them.
- *
- * Returns 0 on success, -1 on failure.
*/
int
-vy_recovery_iterate_index(struct vy_index_recovery_info *index,
- bool include_deleted, vy_recovery_cb cb, void *cb_arg);
+vy_recovery_iterate(struct vy_recovery *recovery, bool include_deleted,
+ vy_recovery_cb cb, void *cb_arg);
/**
- * Given a recovery context, iterate over all indexes stored in it.
- * See vy_recovery_iterate_index() for more details.
+ * Load an index from a recovery context.
*
- * Returns 0 on success, -1 on failure.
+ * Call @cb for each object related to the index. Break the loop and
+ * return -1 if @cb returned a non-zero value, otherwise return 0.
+ * Objects are loaded in the same order as by vy_recovery_iterate().
+ *
+ * Note, this function returns 0 if there's no index with the requested
+ * id in the recovery context. In this case, @cb isn't called at all.
*/
int
-vy_recovery_iterate(struct vy_recovery *recovery, bool include_deleted,
- vy_recovery_cb cb, void *cb_arg);
+vy_recovery_load_index(struct vy_recovery *recovery, int64_t index_lsn,
+ vy_recovery_cb cb, void *cb_arg);
/**
* Initialize a log record with default values.
--
GitLab