Skip to content

crud.update() may use stale schema for metadata generation #236

Closed
@Totktonada

Description

@Totktonada

We can change existing test to reveal the problem.

diff --git a/test/integration/simple_operations_test.lua b/test/integration/simple_operations_test.lua
index 8c33daa..45041ae 100644
--- a/test/integration/simple_operations_test.lua
+++ b/test/integration/simple_operations_test.lua
@@ -396,14 +396,14 @@ end
 
 pgroup.test_intermediate_nullable_fields_update = function(g)
     local result, err = g.cluster.main_server.net_box:call(
-        'crud.insert', {'developers', {1, box.NULL}})
+        'crud.insert', {'developers', {3, box.NULL}})
     t.assert_equals(err, nil)
 
     local objects = crud.unflatten_rows(result.rows, result.metadata)
     t.assert_equals(objects, {
         {
-            id = 1,
-            bucket_id = 477
+            id = 3,
+            bucket_id = 2804
         }
     })
 
@@ -414,19 +414,22 @@ pgroup.test_intermediate_nullable_fields_update = function(g)
     end)
 
     local result, err = g.cluster.main_server.net_box:call('crud.update',
-        {'developers', 1, {{'=', 'extra_3', { a = { b = {} } } }}})
+        {'developers', 3, {{'=', 'extra_3', { a = { b = {} } } }}})
     t.assert_equals(err, nil)
     objects = crud.unflatten_rows(result.rows, result.metadata)
     t.assert_equals(objects, {
         {
-            id = 1,
-            bucket_id = 477,
+            id = 3,
+            bucket_id = 2804,
             extra_1 = box.NULL,
             extra_2 = box.NULL,
             extra_3 = {a = {b = {}}},
         }
     })
 
+    -- Temporary disabled
+    --[[
+
     -- Test uses jsonpaths so it should be run for version 2.3+
     -- where jsonpaths are supported (https://github.com/tarantool/tarantool/issues/1261).
     -- However since 2.8 Tarantool could update intermediate nullable fields
@@ -486,6 +489,8 @@ pgroup.test_intermediate_nullable_fields_update = function(g)
             extra_12 = 'extra_value_12'
         }
     })
+
+    ]]-- Temporary disabled
 end
 
 pgroup.test_object_with_nullable_fields = function(g)

This patch does not change meaning of the test, just changes storage replicaset to work with.

After this, the test fails:

$ luatest -v -p test_intermediate_nullable_fields_update
<...>
    simple_operations.engine:"memtx".test_intermediate_nullable_fields_update ... (0.058s) fail
...ol-meta/crud/test/integration/simple_operations_test.lua:420: expected: 
{
    {
        bucket_id = 2804,
        extra_1 = cdata<void *>: NULL,
        extra_2 = cdata<void *>: NULL,
        extra_3 = {a = {b = {}}},
        id = 3,
    },
}
actual: 
{{bucket_id = 2804, id = 3}}
<...>

I looked at the problem a bit and my draft of the fix is the following:

diff --git a/crud/update.lua b/crud/update.lua
index 587068c..2942e76 100644
--- a/crud/update.lua
+++ b/crud/update.lua
@@ -103,6 +103,18 @@ local function call_update_on_router(space_name, key, user_operations, opts)
     end
 
     local bucket_id = sharding.key_get_bucket_id(sharding_key, opts.bucket_id)
+
+    -- We should use a space object from the same connection as
+    -- we use for the request itself. Otherwise we can use a stale
+    -- format to generate rows metadata.
+    --
+    -- XXX: This fix is fast written and may have problems. IOW, I
+    -- didn't think much about it.
+    space, err = utils.get_space(space_name, {vshard.router.route(bucket_id)})
+    if err ~= nil then
+        return nil, UpdateError:new("Failed to get space %q: %s", space_name, err), true
+    end
+
     local call_opts = {
         mode = 'write',
         timeout = opts.timeout,

Of course, we should look, whether it is fully correct: we use a space format from a 'wrong' connection before this code and, maybe, it should be revisited too.

I would also verify other operations for problems of this kind.

Metadata

Metadata

Assignees

Labels

1spbugSomething isn't workingteamE

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions