Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
vinyl: fix check vinyl_dir existence at bootstrap
During implementation of openSUSE build with testing got failed test box-tap/cfg.test.lua. Found that when memtx_dir didn't exist and vinyl_dir existed and also errno was set to ENOENT, box configuration succeeded, but it shouldn't. Reason of this wrong behavior was that not all of the failure paths in xdir_scan() set errno, but the caller assumed it. Debugging the issue found that after xdir_scan() there was incorrect check for errno when it returned negative values. xdir_scan() is not system call and negative return value from it doesn't mean that errno would be set too. Found that in situations when errno was left from previous commands before xdir_scan() and xdir_scan() returned negative value by itself than the check was wrong. The previous failed logic of the check was to catch the error ENOENT which set in the xdir_scan() function to handle the situation when vinyl_dir was not exist. It failed, because checking ENOENT outside the xdir_scan() function, we had to be sure that ENOENT had come from xdir_scan() function call indeed and not from any other functions before. To be sure in it errno had to be reset before xdir_scan() call, because errno could be passed from any other function before call to xdir_scan() and success system calls in xdir_scan() hadn't clean it up. As mentioned above xdir_scan() function is not system call and can be changed in any possible way and it can return any result value without need to setup errno. So check outside of this function on errno could be broken. To avoid of it we must avoid of errno checks outside of the function. Better solution was to use the flag in xdir_scan(), to check if the directory should exist. So errno check was removed and instead of it the check for vinyl_dir existence using flag added. Closes #4594 Needed for #4562 Co-authored-by: Alexander Turenko <alexander.turenko@tarantool.org>
- Loading branch information
1 parent
a102123
commit f80e668
Showing
7 changed files
with
66 additions
and
10 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| #!/usr/bin/env tarantool | ||
|
|
||
| local tap = require('tap') | ||
| local test = tap.test('cfg') | ||
| local fio = require('fio') | ||
| test:plan(1) | ||
|
|
||
| local tarantool_bin = arg[-1] | ||
| local PANIC = 256 | ||
|
|
||
| local function run_script(code) | ||
| local dir = fio.tempdir() | ||
| local script_path = fio.pathjoin(dir, 'script.lua') | ||
| local script = fio.open(script_path, {'O_CREAT', 'O_WRONLY', 'O_APPEND'}, | ||
| tonumber('0777', 8)) | ||
| script:write(code) | ||
| script:write("\nos.exit(0)") | ||
| script:close() | ||
| local cmd = [[/bin/sh -c 'cd "%s" && "%s" ./script.lua 2> /dev/null']] | ||
| local res = os.execute(string.format(cmd, dir, tarantool_bin)) | ||
| fio.rmtree(dir) | ||
| return res | ||
| end | ||
|
|
||
| -- | ||
| -- gh-4594: when memtx_dir is not exists, but vinyl_dir exists and | ||
| -- errno is set to ENOENT, box configuration succeeds, however it | ||
| -- should not | ||
| -- | ||
|
|
||
| -- create vinyl_dir as temporary path | ||
| local vinyl_dir = fio.tempdir() | ||
|
|
||
| -- fill vinyl_dir | ||
| run_script(string.format([[ | ||
| box.cfg{vinyl_dir = '%s'} | ||
| s = box.schema.space.create('test', {engine = 'vinyl'}) | ||
| s:create_index('pk') | ||
| os.exit(0) | ||
| ]], vinyl_dir)) | ||
|
|
||
| -- verify the case described above | ||
| local code = string.format([[ | ||
| local errno = require('errno') | ||
| errno(errno.ENOENT) | ||
| box.cfg{vinyl_dir = '%s'} | ||
| os.exit(0) | ||
| ]], vinyl_dir) | ||
| test:is(run_script(code), PANIC, "bootstrap with ENOENT from non-empty vinyl_dir") | ||
|
|
||
| -- remove vinyl_dir | ||
| fio.rmtree(vinyl_dir) | ||
|
|
||
| os.exit(test:check() and 0 or 1) |