Tarantool development patches archive
 help / color / mirror / Atom feed
From: Ilya Kosarev <i.kosarev@tarantool.org>
To: tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [PATCH v2] xlog: make log directory if needed
Date: Tue, 30 Jun 2020 13:19:44 +0300	[thread overview]
Message-ID: <20200630101944.15515-1-i.kosarev@tarantool.org> (raw)
In-Reply-To: <20200629124925.GB2256@grain>

Tarantool's box.backup.start() returns the list of files needed to
successfully run new instance. The problem was that it doesn't include
empty indexes log directories. This means that after starting the
instance using backed up files and inserting something into previously
empty index we could run into log file creation error for example on
box.snapshot() call. Now this is not a problem as far as according
directories are created if needed. Corresponding test case added.

Closes #5090
---
Branch: https://github.com/tarantool/tarantool/tree/i.kosarev/gh-5090-empty-vinyl-backup
Issue: https://github.com/tarantool/tarantool/issues/5090

@ChangeLog:
 * Create missing folders for vinyl spaces and indexes if needed to
 avoid confusing fails of tarantool started from backup.

 Changes in v2:
- specified mkdirpath method contract

 src/box/xlog.c                               |  7 ++
 src/lib/core/util.c                          | 26 +++++++
 src/trivia/util.h                            |  3 +
 test/box/gh-5090-empty-vinyl-backup.result   | 82 ++++++++++++++++++++
 test/box/gh-5090-empty-vinyl-backup.test.lua | 30 +++++++
 test/box/lua/simple_instance.lua             |  3 +
 6 files changed, 151 insertions(+)
 create mode 100644 test/box/gh-5090-empty-vinyl-backup.result
 create mode 100644 test/box/gh-5090-empty-vinyl-backup.test.lua
 create mode 100644 test/box/lua/simple_instance.lua

diff --git a/src/box/xlog.c b/src/box/xlog.c
index b5b082a204..95be05f250 100644
--- a/src/box/xlog.c
+++ b/src/box/xlog.c
@@ -824,6 +824,13 @@ xlog_create(struct xlog *xlog, const char *name, int flags,
 	xlog->is_inprogress = true;
 	snprintf(xlog->filename, sizeof(xlog->filename), "%s%s", name, inprogress_suffix);
 
+	/* Make directory if needed (gh-5090). */
+	if (mkdirpath(xlog->filename) != 0) {
+		diag_set(SystemError, "failed to create path '%s'",
+			 xlog->filename);
+		goto err;
+	}
+
 	flags |= O_RDWR | O_CREAT | O_EXCL;
 
 	/*
diff --git a/src/lib/core/util.c b/src/lib/core/util.c
index d7f2344ed6..dfce317f07 100644
--- a/src/lib/core/util.c
+++ b/src/lib/core/util.c
@@ -216,6 +216,32 @@ abspath(const char *filename)
 	return abspath;
 }
 
+/**
+ * Make missing directories from @a path.
+ * @a path has to be null-terminated.
+ * @a path has to be mutable. It allows to avoid copying.
+ * However it is guaranteed to be unmodified after execution.
+ */
+int
+mkdirpath(char *path)
+{
+	char *path_sep = path;
+	while (*path_sep == '/') {
+		/* Don't create root */
+		++path_sep;
+	}
+	while ((path_sep = strchr(path_sep, '/'))) {
+		/* Recursively create path hierarchy */
+		*path_sep = '\0';
+		int rc = mkdir(path, 0777);
+		*path_sep = '/';
+		if (rc == -1 && errno != EEXIST)
+			return -1;
+		++path_sep;
+	}
+	return 0;
+}
+
 char *
 int2str(long long int val)
 {
diff --git a/src/trivia/util.h b/src/trivia/util.h
index 29c7f01943..16667e27e9 100644
--- a/src/trivia/util.h
+++ b/src/trivia/util.h
@@ -410,6 +410,9 @@ find_path(const char *argv0);
 char *
 abspath(const char *filename);
 
+int
+mkdirpath(char *path);
+
 char *
 int2str(long long int val);
 
diff --git a/test/box/gh-5090-empty-vinyl-backup.result b/test/box/gh-5090-empty-vinyl-backup.result
new file mode 100644
index 0000000000..d746a8fe87
--- /dev/null
+++ b/test/box/gh-5090-empty-vinyl-backup.result
@@ -0,0 +1,82 @@
+-- test-run result file version 2
+test_run = require('test_run').new()
+ | ---
+ | ...
+
+test_run:cmd('create server vinyl with script="box/lua/simple_instance.lua"')
+ | ---
+ | - true
+ | ...
+test_run:cmd('start server vinyl')
+ | ---
+ | - true
+ | ...
+test_run:cmd('switch vinyl')
+ | ---
+ | - true
+ | ...
+
+s = box.schema.space.create('test', {engine = 'vinyl'})
+ | ---
+ | ...
+index = s:create_index('primary', {parts={1, 'unsigned'}})
+ | ---
+ | ...
+
+box.snapshot()
+ | ---
+ | - ok
+ | ...
+backup_files = box.backup.start()
+ | ---
+ | ...
+
+test_run:cmd('switch default')
+ | ---
+ | - true
+ | ...
+
+backup_files = test_run:eval('vinyl', 'backup_files')[1]
+ | ---
+ | ...
+for _, file in pairs(backup_files) do os.execute('cp ' .. file .. ' .') end
+ | ---
+ | ...
+
+test_run:drop_cluster({'vinyl'})
+ | ---
+ | ...
+
+test_run:cmd("create server vinyl with script='box/lua/simple_instance.lua'")
+ | ---
+ | - true
+ | ...
+for _, file in pairs(backup_files) do os.execute('mv ' .. file:match('.*/(.*)') .. ' simple_instance/') end
+ | ---
+ | ...
+test_run:cmd('start server vinyl')
+ | ---
+ | - true
+ | ...
+test_run:cmd('switch vinyl')
+ | ---
+ | - true
+ | ...
+
+box.space.test:insert{1}
+ | ---
+ | - [1]
+ | ...
+box.snapshot()
+ | ---
+ | - ok
+ | ...
+
+-- Cleanup.
+test_run:cmd('switch default')
+ | ---
+ | - true
+ | ...
+test_run:drop_cluster({'vinyl'})
+ | ---
+ | ...
diff --git a/test/box/gh-5090-empty-vinyl-backup.test.lua b/test/box/gh-5090-empty-vinyl-backup.test.lua
new file mode 100644
index 0000000000..d461ad4fad
--- /dev/null
+++ b/test/box/gh-5090-empty-vinyl-backup.test.lua
@@ -0,0 +1,30 @@
+test_run = require('test_run').new()
+
+test_run:cmd('create server vinyl with script="box/lua/simple_instance.lua"')
+test_run:cmd('start server vinyl')
+test_run:cmd('switch vinyl')
+
+s = box.schema.space.create('test', {engine = 'vinyl'})
+index = s:create_index('primary', {parts={1, 'unsigned'}})
+
+box.snapshot()
+backup_files = box.backup.start()
+
+test_run:cmd('switch default')
+
+backup_files = test_run:eval('vinyl', 'backup_files')[1]
+for _, file in pairs(backup_files) do os.execute('cp ' .. file .. ' .') end
+
+test_run:drop_cluster({'vinyl'})
+
+test_run:cmd("create server vinyl with script='box/lua/simple_instance.lua'")
+for _, file in pairs(backup_files) do os.execute('mv ' .. file:match('.*/(.*)') .. ' simple_instance/') end
+test_run:cmd('start server vinyl')
+test_run:cmd('switch vinyl')
+
+box.space.test:insert{1}
+box.snapshot()
+
+-- Cleanup.
+test_run:cmd('switch default')
+test_run:drop_cluster({'vinyl'})
diff --git a/test/box/lua/simple_instance.lua b/test/box/lua/simple_instance.lua
new file mode 100644
index 0000000000..9ca79f6d07
--- /dev/null
+++ b/test/box/lua/simple_instance.lua
@@ -0,0 +1,3 @@
+box.cfg{}
+
+require('console').listen(os.getenv('ADMIN'))
-- 
2.17.1

  reply	other threads:[~2020-06-30 10:19 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-29 12:44 [Tarantool-patches] [PATCH] " Ilya Kosarev
2020-06-29 12:48 ` Cyrill Gorcunov
2020-06-29 12:49   ` Cyrill Gorcunov
2020-06-30 10:19     ` Ilya Kosarev [this message]
2020-06-30 11:17       ` [Tarantool-patches] [PATCH v2] " Cyrill Gorcunov
2020-07-09  8:39 ` [Tarantool-patches] [PATCH] " Kirill Yukhin

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=20200630101944.15515-1-i.kosarev@tarantool.org \
    --to=i.kosarev@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v2] xlog: make log directory if needed' \
    /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