Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crud: fix explicit bucket_id in *_many operations #438

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## Unreleased

### Fixed
* `insert_many`, `insert_object_many`, `replace_many`, `replace_object_many`,
`upsert_many`, `upsert_object_many` operations no longer fail with
`ShardingHashMismatchError` if a space has custom sharding info and
every tuple/object in the request has `bucket_id` set (#437).

## [1.5.1] - 27-04-24

### Added
Expand Down
10 changes: 4 additions & 6 deletions crud/common/sharding/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -222,13 +222,11 @@ function sharding.split_tuples_by_replicaset(vshard_router, tuples, space, opts)

local batches = {}

local sharding_func_hash
local sharding_key_hash
local skip_sharding_hash_check
local sharding_data
local err
local sharding_func_hash = nil
local sharding_key_hash = nil
local skip_sharding_hash_check = true
for i, tuple in ipairs(tuples) do
sharding_data, err = sharding.tuple_set_and_return_bucket_id(vshard_router, tuple, space)
local sharding_data, err = sharding.tuple_set_and_return_bucket_id(vshard_router, tuple, space)
if err ~= nil then
return nil, BucketIDError:new("Failed to get bucket ID: %s", err)
end
Expand Down
40 changes: 40 additions & 0 deletions test/entrypoint/srv_batch_operations/storage.lua
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,46 @@ return {
unique = true,
if_not_exists = true,
})

local customers_sharded_by_age_space = box.schema.space.create('customers_sharded_by_age', {
format = {
{name = 'id', type = 'unsigned'},
{name = 'bucket_id', type = 'unsigned'},
{name = 'name', type = 'string'},
{name = 'age', type = 'number'},
},
if_not_exists = true,
engine = engine,
})
customers_sharded_by_age_space:create_index('id', {
parts = { {field = 'id'} },
if_not_exists = true,
})
customers_sharded_by_age_space:create_index('bucket_id', {
parts = { {field = 'bucket_id'} },
unique = false,
if_not_exists = true,
})

-- https://github.com/tarantool/migrations/blob/a7c31a17f6ac02d4498b4203c23e495856861444/migrator/utils.lua#L35-L53
if box.space._ddl_sharding_key == nil then
local sharding_space = box.schema.space.create('_ddl_sharding_key', {
format = {
{name = 'space_name', type = 'string', is_nullable = false},
{name = 'sharding_key', type = 'array', is_nullable = false}
},
if_not_exists = true,
})
sharding_space:create_index(
'space_name', {
type = 'TREE',
unique = true,
parts = {{'space_name', 'string', is_nullable = false}},
if_not_exists = true,
}
)
end
box.space._ddl_sharding_key:replace{'customers_sharded_by_age', {'age'}}
end),
wait_until_ready = helper.wait_schema_init,
}
26 changes: 26 additions & 0 deletions test/integration/insert_many_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ local t = require('luatest')
local crud = require('crud')

local helpers = require('test.helper')
local write_scenario = require('test.integration.write_scenario')

local batching_utils = require('crud.common.batching_utils')

Expand All @@ -19,6 +20,7 @@ end)

pgroup.before_each(function(g)
helpers.truncate_space_on_cluster(g.cluster, 'customers')
helpers.truncate_space_on_cluster(g.cluster, 'customers_sharded_by_age')
end)

pgroup.test_non_existent_space = function(g)
Expand Down Expand Up @@ -2051,3 +2053,27 @@ pgroup.test_zero_objects = function(g)
t.assert_str_contains(errs[1].err, "At least one object expected")
t.assert_equals(result, nil)
end

pgroup.test_gh_437_insert_many_explicit_bucket_ids = function(g)
write_scenario.gh_437_many_explicit_bucket_ids(g, 'insert_many')
end

pgroup.test_gh_437_insert_many_partial_explicit_bucket_ids = function(g)
write_scenario.gh_437_many_explicit_bucket_ids(
g,
'insert_many',
{partial_explicit_bucket_ids = true}
)
end

pgroup.test_gh_437_insert_object_many_explicit_bucket_ids = function(g)
write_scenario.gh_437_many_explicit_bucket_ids(g, 'insert_object_many', {objects = true})
end

pgroup.test_gh_437_insert_object_many_partial_explicit_bucket_ids = function(g)
write_scenario.gh_437_many_explicit_bucket_ids(
g,
'insert_object_many',
{objects = true, partial_explicit_bucket_ids = true}
)
end
26 changes: 26 additions & 0 deletions test/integration/replace_many_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ local t = require('luatest')
local crud = require('crud')

local helpers = require('test.helper')
local write_scenario = require('test.integration.write_scenario')

local batching_utils = require('crud.common.batching_utils')

Expand All @@ -20,6 +21,7 @@ end)

pgroup.before_each(function(g)
helpers.truncate_space_on_cluster(g.cluster, 'developers')
helpers.truncate_space_on_cluster(g.cluster, 'customers_sharded_by_age')
end)

pgroup.test_non_existent_space = function(g)
Expand Down Expand Up @@ -2062,3 +2064,27 @@ pgroup.test_zero_objects = function(g)
t.assert_str_contains(errs[1].err, "At least one object expected")
t.assert_equals(result, nil)
end

pgroup.test_gh_437_replace_many_explicit_bucket_ids = function(g)
write_scenario.gh_437_many_explicit_bucket_ids(g, 'replace_many')
end

pgroup.test_gh_437_replace_many_partial_explicit_bucket_ids = function(g)
write_scenario.gh_437_many_explicit_bucket_ids(
g,
'replace_many',
{partial_explicit_bucket_ids = true}
)
end

pgroup.test_gh_437_replace_object_many_explicit_bucket_ids = function(g)
write_scenario.gh_437_many_explicit_bucket_ids(g, 'replace_object_many', {objects = true})
end

pgroup.test_gh_437_replace_object_many_partial_explicit_bucket_ids = function(g)
write_scenario.gh_437_many_explicit_bucket_ids(
g,
'replace_object_many',
{objects = true, partial_explicit_bucket_ids = true}
)
end
30 changes: 30 additions & 0 deletions test/integration/upsert_many_test.lua
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
local t = require('luatest')

local helpers = require('test.helper')
local write_scenario = require('test.integration.write_scenario')

local batching_utils = require('crud.common.batching_utils')

Expand All @@ -18,6 +19,7 @@ end)

pgroup.before_each(function(g)
helpers.truncate_space_on_cluster(g.cluster, 'customers')
helpers.truncate_space_on_cluster(g.cluster, 'customers_sharded_by_age')
end)

pgroup.test_non_existent_space = function(g)
Expand Down Expand Up @@ -2063,3 +2065,31 @@ pgroup.test_zero_objects = function(g)
t.assert_str_contains(errs[1].err, "At least one object expected")
t.assert_equals(result, nil)
end

pgroup.test_gh_437_upsert_many_explicit_bucket_ids = function(g)
write_scenario.gh_437_many_explicit_bucket_ids(g, 'upsert_many', {upsert = true})
end

pgroup.test_gh_437_upsert_many_partial_explicit_bucket_ids = function(g)
write_scenario.gh_437_many_explicit_bucket_ids(
g,
'upsert_many',
{upsert = true, partial_explicit_bucket_ids = true}
)
end

pgroup.test_gh_437_upsert_object_many_explicit_bucket_ids = function(g)
write_scenario.gh_437_many_explicit_bucket_ids(
g,
'upsert_object_many',
{upsert = true, objects = true}
)
end

pgroup.test_gh_437_upsert_object_many_partial_explicit_bucket_ids = function(g)
write_scenario.gh_437_many_explicit_bucket_ids(
g,
'upsert_object_many',
{upsert = true, objects = true, partial_explicit_bucket_ids = true}
)
end
62 changes: 62 additions & 0 deletions test/integration/write_scenario.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
local t = require('luatest')
local checks = require('checks')

-- Scenario is for 'srv_batch_operations' entrypoint.
local function gh_437_many_explicit_bucket_ids(cg, operation, opts)
checks('table', 'string', {
objects = '?boolean',
upsert = '?boolean',
partial_explicit_bucket_ids = '?boolean',
})

opts = opts or {}

local rows = {
{1, 1, 'Kumiko', 18},
{2, 2, 'Reina', 19},
{3, 3, 'Shuuichi', 18},
}

if opts.partial_explicit_bucket_ids then
rows[2][2] = box.NULL
end

local objects = {}
for k, v in ipairs(rows) do
objects[k] = {id = v[1], bucket_id = v[2], name = v[3], age = v[4]}
end

local data
if opts.objects then
data = objects
else
data = rows
end

if opts.upsert then
local update_operations = {}
for k, v in ipairs(data) do
data[k] = {v, update_operations}
end
end

local args = {'customers_sharded_by_age', data}

local result, errs = cg.router:call('crud.' .. operation, args)
t.assert_equals(errs, nil)

local result_rows = table.deepcopy(rows)
if opts.partial_explicit_bucket_ids then
result_rows[2][2] = 1325
end
if opts.upsert then
-- upsert never return anything.
t.assert_equals(result.rows, nil)
else
t.assert_items_equals(result.rows, result_rows)
end
end

return {
gh_437_many_explicit_bucket_ids = gh_437_many_explicit_bucket_ids,
}
Loading