Skip to content

Commit fbe4de0

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. When I replaced `_G._TARANTOOL` with just `_TARANTOOL`, the luacheck linter said me: | <...>/test/helper.lua:307:31: accessing undefined field match of | global _TARANTOOL On the line: | local major_minor_patch = _TARANTOOL:split('-', 1)[1] But when I added '_TARANTOOL' into globals, it stops to complain. Strange, but okay. I prefer just `_TARANTOOL` without `_G`.
1 parent 5e8c91e commit fbe4de0

File tree

6 files changed

+80
-14
lines changed

6 files changed

+80
-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: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
1313

1414
### Fixed
1515

16+
* Use tuple-merger backed select implementation on tarantool 2.10+ (it gives
17+
less pressure on Lua GC).
18+
1619
## [0.9.0] - 20-10-21
1720

1821
### Added

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 in 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: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,4 +300,27 @@ function helpers.get_other_storage_bucket_id(cluster, bucket_id)
300300
]], {bucket_id})
301301
end
302302

303+
function helpers.tarantool_version_at_least(wanted_major, wanted_minor,
304+
wanted_patch)
305+
-- Borrowed from `determine_enabled_features()` from
306+
-- crud/common/utils.lua.
307+
local major_minor_patch = _TARANTOOL:split('-', 1)[1]
308+
local major_minor_patch_parts = major_minor_patch:split('.', 2)
309+
310+
local major = tonumber(major_minor_patch_parts[1])
311+
local minor = tonumber(major_minor_patch_parts[2])
312+
local patch = tonumber(major_minor_patch_parts[3])
313+
314+
if major < (wanted_major or 0) then return false end
315+
if major > (wanted_major or 0) then return true end
316+
317+
if minor < (wanted_minor or 0) then return false end
318+
if minor > (wanted_minor or 0) then return true end
319+
320+
if patch < (wanted_patch or 0) then return false end
321+
if patch > (wanted_patch or 0) then return true end
322+
323+
return true
324+
end
325+
303326
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)