From cfa496de18c0c5f613dba104772c99c31bb02195 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Fri, 17 May 2024 14:07:07 +0300 Subject: [PATCH] crud: fix explicit bucket_id _many operations In case `insert_many`, `insert_object_many`, `replace_many`, `replace_object_many`, `upsert_many` or `upsert_object_many` has been called on a space with custom sharding info and every tuple/object in the request had `bucket_id`, `ShardingHashMismatchError` has been returned even though there are no issues with sharding info. For example, some users met this issue while working with tt-ee `tt crud import` command. The reason is as follows. To ensure sharding info consistency between the storage and the router, some metainfo is calculated for a request in case bucket_id is generated. In case no bucket_id is generated, no sharding info is passed and consistency check is skipped (since it is not needed). Before this patch, `*_many` operations haven't skipped consistency check when it was expected due to improper `skip_sharding_hash_check` flag setup. Closes #437 --- CHANGELOG.md | 8 +++ crud/common/sharding/init.lua | 10 ++- .../srv_batch_operations/storage.lua | 40 ++++++++++++ test/integration/insert_many_test.lua | 26 ++++++++ test/integration/replace_many_test.lua | 26 ++++++++ test/integration/upsert_many_test.lua | 30 +++++++++ test/integration/write_scenario.lua | 62 +++++++++++++++++++ 7 files changed, 196 insertions(+), 6 deletions(-) create mode 100644 test/integration/write_scenario.lua diff --git a/CHANGELOG.md b/CHANGELOG.md index f9aa56e4..47cb998b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/crud/common/sharding/init.lua b/crud/common/sharding/init.lua index 72c4dfe7..4a96f316 100644 --- a/crud/common/sharding/init.lua +++ b/crud/common/sharding/init.lua @@ -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 diff --git a/test/entrypoint/srv_batch_operations/storage.lua b/test/entrypoint/srv_batch_operations/storage.lua index 8bec46e2..d7d0937c 100644 --- a/test/entrypoint/srv_batch_operations/storage.lua +++ b/test/entrypoint/srv_batch_operations/storage.lua @@ -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, } diff --git a/test/integration/insert_many_test.lua b/test/integration/insert_many_test.lua index 05162c1a..2228cf5b 100644 --- a/test/integration/insert_many_test.lua +++ b/test/integration/insert_many_test.lua @@ -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') @@ -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) @@ -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 diff --git a/test/integration/replace_many_test.lua b/test/integration/replace_many_test.lua index 93874c9d..9a1228d7 100644 --- a/test/integration/replace_many_test.lua +++ b/test/integration/replace_many_test.lua @@ -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') @@ -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) @@ -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 diff --git a/test/integration/upsert_many_test.lua b/test/integration/upsert_many_test.lua index 2bcb7d89..74b0e2bd 100644 --- a/test/integration/upsert_many_test.lua +++ b/test/integration/upsert_many_test.lua @@ -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') @@ -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) @@ -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 diff --git a/test/integration/write_scenario.lua b/test/integration/write_scenario.lua new file mode 100644 index 00000000..2c6fd9b7 --- /dev/null +++ b/test/integration/write_scenario.lua @@ -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, +}