Skip to content

Commit f935c70

Browse files
api: forbid using space id in len
Using space id instead of a space name in sharded systems may be dangerous until one sets all space id manually. This is a breaking change. The feature was deprecated since 0.14.0. Closes #255
1 parent 5717e87 commit f935c70

File tree

6 files changed

+3
-106
lines changed

6 files changed

+3
-106
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
1212
replicaset and for the master connection (#95).
1313

1414
### Changed
15+
* **Breaking**: forbid using space id in `crud.len` (#255).
1516

1617
### Fixed
1718
* Added validation of the master presence in replicaset and the

crud/len.lua

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
local checks = require('checks')
22
local errors = require('errors')
3-
local log = require('log')
43

54
local utils = require('crud.common.utils')
65
local dev_checks = require('crud.common.dev_checks')
@@ -42,23 +41,13 @@ end
4241
-- @treturn[2] table Error description
4342
--
4443
function len.call(space_name, opts)
45-
checks('string|number', {
44+
checks('string', {
4645
timeout = '?number',
4746
vshard_router = '?string|table',
4847
})
4948

5049
opts = opts or {}
5150

52-
if type(space_name) == 'number' then
53-
log.warn('Using space id in crud.len is deprecated and will be removed in future releases.' ..
54-
'Please, use space name instead.')
55-
56-
if opts.vshard_router ~= nil then
57-
log.warn('Using space id in crud.len and custom vshard_router is not supported by statistics.' ..
58-
'Space labels may be inconsistent.')
59-
end
60-
end
61-
6251
local vshard_router, err = utils.get_vshard_router_instance(opts.vshard_router)
6352
if err ~= nil then
6453
return nil, LenError:new(err)

crud/stats/init.lua

Lines changed: 1 addition & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,9 @@ local checks = require('checks')
77
local errors = require('errors')
88
local fiber = require('fiber')
99
local fun = require('fun')
10-
local log = require('log')
11-
local vshard = require('vshard')
1210

1311
local dev_checks = require('crud.common.dev_checks')
1412
local stash = require('crud.common.stash')
15-
local utils = require('crud.common.utils')
1613
local op_module = require('crud.stats.operation')
1714

1815
local StatsError = errors.new_class('StatsError', {capture_stack = false})
@@ -249,34 +246,6 @@ function stats.get(space_name)
249246
return internal:get_registry().get(space_name)
250247
end
251248

252-
local function resolve_space_name(space_id)
253-
-- Resolving name with custom vshard router is not supported.
254-
local vshard_router = vshard.router.static
255-
256-
if vshard_router == nil then
257-
log.warn('Failed to resolve space name for stats: default vshard router not found')
258-
return nil
259-
end
260-
261-
local replicasets = vshard_router:routeall()
262-
if next(replicasets) == nil then
263-
log.warn('Failed to resolve space name for stats: no replicasets found with default router')
264-
return nil
265-
end
266-
267-
local space, err = utils.get_space(space_id, vshard_router)
268-
if err ~= nil then
269-
log.warn("An error occurred during getting space: %s", err)
270-
return nil
271-
end
272-
if space == nil then
273-
log.warn('Failed to resolve space name for stats: no space found for id %d with default router', space_id)
274-
return nil
275-
end
276-
277-
return space.name
278-
end
279-
280249
-- Hack to set __gc for a table in Lua 5.1
281250
-- See https://stackoverflow.com/questions/27426704/lua-5-1-workaround-for-gc-metamethod-for-tables
282251
-- or https://habr.com/ru/post/346892/
@@ -351,26 +320,13 @@ local function wrap_pairs_gen(build_latency, space_name, op, gen, param, state)
351320
end
352321

353322
local function wrap_tail(space_name, op, pairs, start_time, call_status, ...)
354-
dev_checks('string|number', 'string', 'boolean', 'number', 'boolean')
323+
dev_checks('string', 'string', 'boolean', 'number', 'boolean')
355324

356325
local finish_time = clock.monotonic()
357326
local latency = finish_time - start_time
358327

359328
local registry = internal:get_registry()
360329

361-
-- If space id is provided instead of name, try to resolve name.
362-
-- If resolve have failed, use id as string to observe space.
363-
-- If using space id will be deprecated, remove this code as well,
364-
-- see https://github.com/tarantool/crud/issues/255
365-
if type(space_name) ~= 'string' then
366-
local name = resolve_space_name(space_name)
367-
if name ~= nil then
368-
space_name = name
369-
else
370-
space_name = tostring(space_name)
371-
end
372-
end
373-
374330
if call_status == false then
375331
registry.observe(latency, space_name, op, 'error')
376332
error((...), 2)

test/entrypoint/srv_vshard_custom.lua

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ package.preload['customers-storage'] = function()
2424
},
2525
if_not_exists = true,
2626
engine = engine,
27-
id = 542,
2827
})
2928

3029
customers_space:create_index('pk', {

test/integration/stats_test.lua

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,7 @@ local group_metrics = t.group('stats_metrics_integration', {
1616

1717
local helpers = require('test.helper')
1818

19-
local space_id = 542
2019
local space_name = 'customers'
21-
local non_existing_space_id = 100500
2220
local non_existing_space_name = 'non_existing_space'
2321
local new_space_name = 'newspace'
2422

@@ -752,24 +750,6 @@ for name, case in pairs(select_cases) do
752750
end
753751

754752

755-
pgroup.test_resolve_name_from_id = function(g)
756-
local op = 'len'
757-
g.router:call('crud.len', { space_id })
758-
759-
local stats = get_stats(g, space_name)
760-
t.assert_not_equals(stats[op], nil, "Statistics is filled by name")
761-
end
762-
763-
764-
pgroup.test_resolve_nonexisting_space_from_id = function(g)
765-
local op = 'len'
766-
g.router:call('crud.len', { non_existing_space_id })
767-
768-
local stats = get_stats(g, tostring(non_existing_space_id))
769-
t.assert_not_equals(stats[op], nil, "Statistics is filled by id as string")
770-
end
771-
772-
773753
pgroup.before_test(
774754
'test_role_reload_do_not_reset_observations',
775755
generate_stats)

test/integration/vshard_custom_test.lua

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1630,31 +1630,3 @@ pgroup.test_call_upsert_object_many_wrong_option = function(g)
16301630
t.assert_str_contains(errs[1].err,
16311631
"Invalid opts.vshard_router table value, a vshard router instance has been expected")
16321632
end
1633-
1634-
pgroup.before_test('test_call_len_by_space_id_with_stats', function(g)
1635-
g.router:eval('crud.cfg{stats = true}')
1636-
end)
1637-
1638-
pgroup.test_call_len_by_space_id_with_stats = function(g)
1639-
local capture = luatest_capture:new()
1640-
capture:enable()
1641-
1642-
local result, err = g:call_router_opts2('len', 542, {vshard_router = 'customers'})
1643-
t.assert_equals(err, nil)
1644-
t.assert_equals(result, 2)
1645-
1646-
local captured = helpers.fflush_main_server_stdout(g.cluster, capture)
1647-
capture:disable()
1648-
1649-
t.assert_str_contains(captured,
1650-
"Using space id in crud.len and custom vshard_router is not supported by statistics.")
1651-
1652-
local result, err = g.router:call('crud.stats')
1653-
t.assert_equals(err, nil)
1654-
t.assert_type(result.spaces["542"], 'table')
1655-
t.assert_equals(result.spaces["542"]["len"]["ok"]["count"], 1)
1656-
end
1657-
1658-
pgroup.after_test('test_call_len_by_space_id_with_stats', function(g)
1659-
g.router:eval('crud.cfg{stats = false}')
1660-
end)

0 commit comments

Comments
 (0)