From e1d961708504cd6f9f84dc7c8bb9f108eb5152e5 Mon Sep 17 00:00:00 2001
From: Aleksandr Lyapunov <alyapunov@tarantool.org>
Date: Thu, 25 Aug 2022 17:18:34 +0300
Subject: [PATCH] Fix a bug in qsort
In commit (35334ca13b) qsort was fixed but unfortunately a small
typo was introduced. Due to that typo the qsort made its job wrong.
Fix the problem and add unit test for qsort.
Unfortunately the test right from the issue runs extremely long,
so it should go to long-tests.
Closes #7605
NO_DOC=bugfix
---
changelogs/unreleased/gh-7605-fix-qsort.md | 3 +
.../gh_7605_qsort_recovery_test.lua | 101 ++++++++++++++++++
test/box-luatest/suite.ini | 1 +
test/unit/CMakeLists.txt | 3 +
test/unit/qsort_arg.cc | 85 +++++++++++++++
test/unit/qsort_arg.result | 17 +++
third_party/qsort_arg_mt.c | 2 +-
7 files changed, 211 insertions(+), 1 deletion(-)
create mode 100644 changelogs/unreleased/gh-7605-fix-qsort.md
create mode 100644 test/box-luatest/gh_7605_qsort_recovery_test.lua
create mode 100644 test/unit/qsort_arg.cc
create mode 100644 test/unit/qsort_arg.result
diff --git a/changelogs/unreleased/gh-7605-fix-qsort.md b/changelogs/unreleased/gh-7605-fix-qsort.md
new file mode 100644
index 0000000000..97e52573a1
--- /dev/null
+++ b/changelogs/unreleased/gh-7605-fix-qsort.md
@@ -0,0 +1,3 @@
+## bugfix/core
+
+* Fixed crash in secondary index without hint (gh-7605)
diff --git a/test/box-luatest/gh_7605_qsort_recovery_test.lua b/test/box-luatest/gh_7605_qsort_recovery_test.lua
new file mode 100644
index 0000000000..14cc792a7d
--- /dev/null
+++ b/test/box-luatest/gh_7605_qsort_recovery_test.lua
@@ -0,0 +1,101 @@
+local server = require('test.luatest_helpers.server')
+local t = require('luatest')
+
+local g = t.group()
+
+g.before_all = function()
+ g.server = server:new{
+ alias = 'default',
+ }
+ g.server:start()
+end
+
+g.after_all = function()
+ g.server:drop()
+end
+
+g.before_each(function(cg)
+ cg.server:exec(function()
+ box.schema.space.create('test', { if_not_exists = true })
+ box.space.test:format({
+ {name='id',type='integer'},
+ {name='t1', type='string'}
+ })
+ box.space.test:create_index('pri', { parts = {'id'} })
+ box.space.test:create_index('i', { parts = {'t1'}, hint = false })
+ end)
+end)
+
+g.after_each(function(cg)
+ cg.server:exec(function()
+ box.space.test:drop()
+ end)
+end)
+
+g.test_qsort_recovery = function()
+ g.server:exec(function()
+
+ local PACK = 10000
+ local TOTAL = 2000000
+ local uuid = require "uuid"
+ local fiber = require "fiber"
+
+ for i = 1, TOTAL / PACK do
+ if fiber.set_slice then
+ fiber.set_slice(600)
+ end
+ box.begin()
+ for k = 1, PACK do
+ box.space.test:replace{(i - 1) * 1000 + k, uuid.str()}
+ end
+ box.commit()
+ end
+
+ box.snapshot()
+
+ end)
+
+ g.server:restart()
+
+ -- check secondary index correctness
+ g.server:exec(function()
+ local PACK = 10000
+ local t = require('luatest')
+ local prev_tuple = nil
+ local fiber = require "fiber"
+ local i = 0
+ for _,tuple in box.space.test.index.i:pairs() do
+ if prev_tuple ~= nil then
+ t.assert(prev_tuple[2] < tuple[2], "Unordered!")
+ end
+ prev_tuple = tuple
+ i = i + 1
+ if i == PACK then
+ fiber.yield()
+ i = 0
+ end
+ end
+ end)
+
+ -- original test
+ g.server:exec(function()
+
+ local PACK = 10000
+ local TOTAL = 2000000
+ local uuid = require "uuid"
+ local fiber = require "fiber"
+
+ for i = 1, TOTAL / PACK do
+ if fiber.set_slice then
+ fiber.set_slice(600)
+ end
+ box.begin()
+ for k = 1, PACK do
+ box.space.test:replace{(i - 1) * 1000 + k, uuid.str()}
+ end
+ box.commit()
+ end
+
+ end)
+
+end
diff --git a/test/box-luatest/suite.ini b/test/box-luatest/suite.ini
index 14fbabbe36..c0b11bf72b 100644
--- a/test/box-luatest/suite.ini
+++ b/test/box-luatest/suite.ini
@@ -3,3 +3,4 @@ core = luatest
description = Database tests
is_parallel = True
release_disabled = gh_6819_iproto_watch_not_implemented_test.lua gh_6930_mvcc_net_box_iso_test.lua
+long_run = gh_7605_qsort_recovery_test.lua
diff --git a/test/unit/CMakeLists.txt b/test/unit/CMakeLists.txt
index 2fecf7bc6d..6c5e9b4e98 100644
--- a/test/unit/CMakeLists.txt
+++ b/test/unit/CMakeLists.txt
@@ -328,3 +328,6 @@ target_link_libraries(tt_sigaction.test core unit pthread)
add_executable(string.test string.c core_test_utils.c)
target_link_libraries(string.test core unit)
+
+add_executable(qsort_arg.test qsort_arg.cc core_test_utils.c)
+target_link_libraries(qsort_arg.test misc unit)
diff --git a/test/unit/qsort_arg.cc b/test/unit/qsort_arg.cc
new file mode 100644
index 0000000000..423f22eb63
--- /dev/null
+++ b/test/unit/qsort_arg.cc
@@ -0,0 +1,85 @@
+#include "qsort_arg.h"
+
+#include <algorithm>
+#include <cstdint>
+#include <cstring>
+#include <random>
+#include <vector>
+
+#include "unit.h"
+#include "trivia/util.h"
+
+int
+qsort_cmp(const void *a, const void *b, void *)
+{
+ uint64_t i, j;
+ memcpy(&i, a, sizeof(i));
+ memcpy(&j, b, sizeof(j));
+ return i < j ? -1 : i > j;
+}
+
+/**
+ * Checker of qsort_arg for different sizes. Calls one 'ok(..)' for each size.
+ */
+template <size_t Count>
+static void
+test_qsort_common(const size_t (&sizes)[Count])
+{
+ auto gen = std::mt19937_64{}; /* No seed for reproducibility. */
+ std::vector<uint64_t> data;
+
+ for (size_t N : sizes) {
+ data.resize(N);
+ for (auto &v : data)
+ v = gen();
+
+ qsort_arg(data.data(), N, sizeof(data[0]), qsort_cmp, nullptr);
+
+ ok(std::is_sorted(data.begin(), data.end()), "Must be sorted");
+ }
+}
+
+/**
+ * For low sizes a single-thread version of qsort is called.
+ */
+static void
+test_qsort_st(void)
+{
+ const size_t sizes[] = {1000, 10000, 100000};
+ plan(lengthof(sizes));
+ header();
+
+ test_qsort_common(sizes);
+
+ footer();
+ check_plan();
+}
+
+/**
+ * For big sizes a multi-thread version of qsort is called.
+ */
+static void
+test_qsort_mt(void)
+{
+ size_t sizes[] = {150000, 1000000, 4000000};
+ plan(lengthof(sizes));
+ header();
+
+ test_qsort_common(sizes);
+
+ footer();
+ check_plan();
+}
+
+int
+main(void)
+{
+ plan(2);
+ header();
+
+ test_qsort_st();
+ test_qsort_mt();
+
+ footer();
+ return check_plan();
+}
diff --git a/test/unit/qsort_arg.result b/test/unit/qsort_arg.result
new file mode 100644
index 0000000000..fda97084d1
--- /dev/null
+++ b/test/unit/qsort_arg.result
@@ -0,0 +1,17 @@
+1..2
+ *** main ***
+ 1..3
+ *** test_qsort_st ***
+ ok 1 - Must be sorted
+ ok 2 - Must be sorted
+ ok 3 - Must be sorted
+ *** test_qsort_st: done ***
+ok 1 - subtests
+ 1..3
+ *** test_qsort_mt ***
+ ok 1 - Must be sorted
+ ok 2 - Must be sorted
+ ok 3 - Must be sorted
+ *** test_qsort_mt: done ***
+ok 2 - subtests
+ *** main: done ***
diff --git a/third_party/qsort_arg_mt.c b/third_party/qsort_arg_mt.c
index a978adacd9..afb261cf06 100644
--- a/third_party/qsort_arg_mt.c
+++ b/third_party/qsort_arg_mt.c
@@ -232,7 +232,7 @@ qsort_arg_mt_internal(void *a, size_t n, intptr_t es,
/* Recurse on right partition, then iterate on left partition */
if (d2 > es) {
#pragma omp task
- qsort_arg_mt_internal(a, d1 / es, es, cmp, arg);
+ qsort_arg_mt_internal(pn - d2, d2 / es, es, cmp, arg);
}
if (d1 > es)
{
--
GitLab