Skip to content

Commit d32f401

Browse files
committed
Fix merger support recognition on tarantool 2.10+
We cannot use just string comparison as before, because '2.10.0-<...>' less than, say, '2.3' lexicographically. The module had to use the old select implementation on tarantool 2.10 due to this problem. Implemented more accurate checks, whether built-in and external tuple-merger are supported. Say, it'll not enable the built-in merger on tarantool 2.4.1, where the module has a known problem (see comments in the code). Fixed the problem of the same kind in a test case. The version parsing code is borrowed from https://github.com/tarantool/tuple-keydef (a bit adopted for versions like 2.10.0-beta1-0-g7da4b1438). The luacheck linter said me: | <...>/test/helper.lua:251:40: accessing undefined field match of | global _TARANTOOL On the line: | return tonumber((select(component, _TARANTOOL:match(pattern)))) But when I added '_TARANTOOL' into globals, it stops to complain. Strange, but okay.
1 parent d13fe63 commit d32f401

File tree

6 files changed

+98
-14
lines changed

6 files changed

+98
-14
lines changed

.luacheckrc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
redefined = false
2-
globals = {'box', 'utf8', 'checkers'}
2+
globals = {'box', 'utf8', 'checkers', '_TARANTOOL'}
33
include_files = {'**/*.lua', '*.luacheckrc', '*.rockspec'}
44
exclude_files = {'**/*.rocks/', 'tmp/', 'tarantool-enterprise/'}
55
max_line_length = 120

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
1919
### Fixed
2020

2121
* Damaging of opts table by CRUD methods.
22+
* Use tuple-merger backed select implementation on tarantool 2.10+ (it gives
23+
less pressure on Lua GC).
2224

2325
### Added
2426

crud/common/utils.lua

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,35 @@ local function determine_enabled_features()
210210
-- since Tarantool 2.6.3 / 2.7.2 / 2.8.1
211211
enabled_tarantool_features.jsonpath_indexes = major >= 3 or (major >= 2 and ((minor >= 6 and patch >= 3)
212212
or (minor >= 7 and patch >= 2) or (minor >= 8 and patch >= 1) or minor >= 9))
213+
214+
-- The merger module was implemented 2.2.1, see [1].
215+
-- However it had the critical problem [2], which leads to
216+
-- segfault at attempt to use the module from a fiber serving
217+
-- iproto request. So we don't use it in versions before the
218+
-- fix.
219+
--
220+
-- [1]: https://github.com/tarantool/tarantool/issues/3276
221+
-- [2]: https://github.com/tarantool/tarantool/issues/4954
222+
enabled_tarantool_features.builtin_merger =
223+
(major == 2 and minor == 3 and patch >= 3) or
224+
(major == 2 and minor == 4 and patch >= 2) or
225+
(major == 2 and minor == 5 and patch >= 1) or
226+
(major >= 2 and minor >= 6) or
227+
(major >= 3)
228+
229+
-- The external merger module leans on a set of relatively
230+
-- new APIs in tarantool. So it works only on tarantool
231+
-- versions, which offer those APIs.
232+
--
233+
-- See README of the module:
234+
-- https://github.com/tarantool/tuple-merger
235+
enabled_tarantool_features.external_merger =
236+
(major == 1 and minor == 10 and patch >= 8) or
237+
(major == 2 and minor == 4 and patch >= 3) or
238+
(major == 2 and minor == 5 and patch >= 2) or
239+
(major == 2 and minor == 6 and patch >= 1) or
240+
(major == 2 and minor >= 7) or
241+
(major >= 3)
213242
end
214243

215244
function utils.tarantool_supports_fieldpaths()
@@ -236,6 +265,22 @@ function utils.tarantool_supports_jsonpath_indexes()
236265
return enabled_tarantool_features.jsonpath_indexes
237266
end
238267

268+
function utils.tarantool_has_builtin_merger()
269+
if enabled_tarantool_features.builtin_merger == nil then
270+
determine_enabled_features()
271+
end
272+
273+
return enabled_tarantool_features.builtin_merger
274+
end
275+
276+
function utils.tarantool_supports_external_merger()
277+
if enabled_tarantool_features.external_merger == nil then
278+
determine_enabled_features()
279+
end
280+
281+
return enabled_tarantool_features.external_merger
282+
end
283+
239284
local function add_nullable_fields_recursive(operations, operations_map, space_format, tuple, id)
240285
if id < 2 or tuple[id - 1] ~= box.NULL then
241286
return operations

crud/select.lua

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
local errors = require('errors')
22

3+
local utils = require('crud.common.utils')
34
local select_executor = require('crud.select.executor')
45
local select_filters = require('crud.select.filters')
56
local dev_checks = require('crud.common.dev_checks')
@@ -9,18 +10,12 @@ local SelectError = errors.new_class('SelectError')
910

1011
local select_module
1112

12-
if '2' <= _TARANTOOL and _TARANTOOL <= '2.3.3' then
13-
-- "merger" segfaults here
14-
-- See https://github.com/tarantool/tarantool/issues/4954
15-
select_module = require('crud.select.compat.select_old')
16-
elseif not package.search('tuple.merger') and package.loaded['merger'] == nil then
17-
-- we don't use pcall(require, modile_name) here because it
18-
-- leads to ignoring errors other than 'No LuaRocks module found'
19-
20-
-- "merger" isn't supported here
21-
select_module = require('crud.select.compat.select_old')
22-
else
13+
local has_merger = (utils.tarantool_supports_external_merger() and
14+
package.search('tuple.merger')) or utils.tarantool_has_builtin_merger()
15+
if has_merger then
2316
select_module = require('crud.select.compat.select')
17+
else
18+
select_module = require('crud.select.compat.select_old')
2419
end
2520

2621
local function make_cursor(data)

test/helper.lua

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,4 +230,46 @@ function helpers.get_other_storage_bucket_id(cluster, bucket_id)
230230
]], {bucket_id})
231231
end
232232

233+
local function parse_tarantool_version(component)
234+
-- _TARANTOOL gives a string of the following kind:
235+
--
236+
-- * '2.8.1-0-ge2a1ec0c2' for opensource tarantool;
237+
-- * '2.8.1-0-ge2a1ec0c2-r405' for tarantool enterprise.
238+
--
239+
-- Or, after 2.10:
240+
--
241+
-- * '2.10.0-beta1-0-g7da4b1438' for opensource tarantool;
242+
-- * '2.10.0-beta1-0-g7da4b1438-r427' for tarantool enterprise;
243+
--
244+
-- Nicely, tarantool enterprise reports the opensource
245+
-- version, on which it is based, in the first four components
246+
-- (as well as in the commit hash).
247+
--
248+
-- We're interested in three components, so just ignore
249+
-- everything after them.
250+
local pattern = '^(%d+).(%d+).(%d+)-'
251+
return tonumber((select(component, _TARANTOOL:match(pattern))))
252+
end
253+
254+
local _TARANTOOL_MAJOR = parse_tarantool_version(1)
255+
local _TARANTOOL_MINOR = parse_tarantool_version(2)
256+
local _TARANTOOL_PATCH = parse_tarantool_version(3)
257+
258+
function helpers.tarantool_version_at_least(major, minor, patch)
259+
local major = major or 0
260+
local minor = minor or 0
261+
local patch = patch or 0
262+
263+
if _TARANTOOL_MAJOR < major then return false end
264+
if _TARANTOOL_MAJOR > major then return true end
265+
266+
if _TARANTOOL_MINOR < minor then return false end
267+
if _TARANTOOL_MINOR > minor then return true end
268+
269+
if _TARANTOOL_PATCH < patch then return false end
270+
if _TARANTOOL_PATCH > patch then return true end
271+
272+
return true
273+
end
274+
233275
return helpers

test/integration/simple_operations_test.lua

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -432,11 +432,11 @@ pgroup.test_intermediate_nullable_fields_update = function(g)
432432
-- However since 2.8 Tarantool could update intermediate nullable fields
433433
-- (https://github.com/tarantool/tarantool/issues/3378).
434434
-- So before 2.8 update returns an error but after it update is correct.
435-
if _TARANTOOL > "2.8" then
435+
if helpers.tarantool_version_at_least(2, 8) then
436436
local _, err = g.cluster.main_server.net_box:call('crud.update',
437437
{'developers', 1, {{'=', '[5].a.b[1]', 3}, {'=', 'extra_5', 'extra_value_5'}}})
438438
t.assert_equals(err, nil)
439-
elseif _TARANTOOL >= "2.3" then
439+
elseif helpers.tarantool_version_at_least(2, 3) then
440440
local _, err = g.cluster.main_server.net_box:call('crud.update',
441441
{'developers', 1, {{'=', '[5].a.b[1]', 3}, {'=', 'extra_5', 'extra_value_5'}}})
442442
t.assert_equals(err.err, "Failed to update: Field ''extra_5'' was not found in the tuple")

0 commit comments

Comments
 (0)