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.update() may use stale schema for metadata generation #236

Closed
Totktonada opened this issue Nov 13, 2021 · 0 comments · Fixed by #359
Closed

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

Totktonada opened this issue Nov 13, 2021 · 0 comments · Fixed by #359
Assignees
Labels
1sp bug Something isn't working teamE

Comments

@Totktonada
Copy link
Member

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.

@Totktonada Totktonada added the bug Something isn't working label Nov 13, 2021
@LeonidVas LeonidVas added the teamE label Oct 5, 2022
@LeonidVas LeonidVas added the 1sp label Dec 7, 2022
GRISHNOV added a commit that referenced this issue Apr 5, 2023
Corrects using of an stale schema to generate metadata during
`crud.update` working. Now `utils.get_space` using `schema_reload`
before returning space to `crud.update` logic.

Closes #236
GRISHNOV added a commit that referenced this issue Apr 5, 2023
Corrects using of an stale schema to generate metadata during
`crud.update` working. Now `utils.get_space` using `schema_reload`
before returning space to `crud.update` logic.

Closes #236
GRISHNOV added a commit that referenced this issue Apr 24, 2023
Corrects using of an stale schema to generate metadata during
`crud.update` working. Now `utils.get_space` using `schema_reload`
before returning space to `crud.update` logic.

Closes #236
GRISHNOV added a commit that referenced this issue May 10, 2023
Corrects using of an stale schema to generate metadata during
`crud.update` working. Now `utils.get_space` using `schema_reload`
before returning space to `crud.update` logic.

Closes #236
GRISHNOV added a commit that referenced this issue May 31, 2023
Corrects using of an stale schema to generate metadata during
`crud.update`, `crud.insert`, `crud.insert_*`, `crud.replace`,
`crud.replace_*`, `crud.upsert`, `crud.upsert_*`, `crud.delete`,
`crud.max`, `crud.min`, `crud.select` and `crud.get` working.

If the implemented `fetch_latest_metadata` option is used, it is guaranteed
that the metadata will be up-to-date. Before receiving the space format,
a mismatch check will be performed between the scheme version on all
involved storage and the scheme version in the net_box connection of
the router. In case of mismatch, the schema reload will be triggered.

Closes #236
GRISHNOV added a commit that referenced this issue May 31, 2023
Corrects using of an stale schema to generate metadata during
`crud.update`, `crud.insert`, `crud.insert_*`, `crud.replace`,
`crud.replace_*`, `crud.upsert`, `crud.upsert_*`, `crud.delete`,
`crud.max`, `crud.min`, `crud.select` and `crud.get` working.

If the implemented `fetch_latest_metadata` option is used, it is guaranteed
that the metadata will be up-to-date. Before receiving the space format,
a mismatch check will be performed between the scheme version on all
involved storage and the scheme version in the net_box connection of
the router. In case of mismatch, the schema reload will be triggered.

Closes #236
GRISHNOV added a commit that referenced this issue Jun 1, 2023
Corrects using of an stale schema to generate metadata during
`crud.update`, `crud.insert`, `crud.insert_*`, `crud.replace`,
`crud.replace_*`, `crud.upsert`, `crud.upsert_*`, `crud.delete`,
`crud.max`, `crud.min`, `crud.select` and `crud.get` working.

If the implemented `fetch_latest_metadata` option is used, it is guaranteed
that the metadata will be up-to-date. Before receiving the space format,
a mismatch check will be performed between the scheme version on all
involved storage and the scheme version in the net_box connection of
the router. In case of mismatch, the schema reload will be triggered.

Closes #236
GRISHNOV added a commit that referenced this issue Jun 1, 2023
Corrects using of an stale schema to generate metadata during
`crud.update`, `crud.insert`, `crud.insert_*`, `crud.replace`,
`crud.replace_*`, `crud.upsert`, `crud.upsert_*`, `crud.delete`,
`crud.max`, `crud.min`, `crud.select` and `crud.get` working.

If the implemented `fetch_latest_metadata` option is used, it is guaranteed
that the metadata will be up-to-date. Before receiving the space format,
a mismatch check will be performed between the scheme version on all
involved storage and the scheme version in the net_box connection of
the router. In case of mismatch, the schema reload will be triggered.

Closes #236
GRISHNOV added a commit that referenced this issue Jun 1, 2023
Corrects using of an stale schema to generate metadata during
`crud.update`, `crud.insert`, `crud.insert_*`, `crud.replace`,
`crud.replace_*`, `crud.upsert`, `crud.upsert_*`, `crud.delete`,
`crud.max`, `crud.min`, `crud.select` and `crud.get` working.

If the implemented `fetch_latest_metadata` option is used, it is guaranteed
that the metadata will be up-to-date. Before receiving the space format,
a mismatch check will be performed between the scheme version on all
involved storage and the scheme version in the net_box connection of
the router. In case of mismatch, the schema reload will be triggered.

Closes #236
GRISHNOV added a commit that referenced this issue Jun 3, 2023
Corrects using of an stale schema to generate metadata during
`crud.update`, `crud.insert`, `crud.insert_*`, `crud.replace`,
`crud.replace_*`, `crud.upsert`, `crud.upsert_*`, `crud.delete`,
`crud.max`, `crud.min`, `crud.select` and `crud.get` working.

If the implemented `fetch_latest_metadata` option is used, it is guaranteed
that the metadata will be up-to-date. Before receiving the space format,
a mismatch check will be performed between the scheme version on all
involved storage and the scheme version in the net_box connection of
the router. In case of mismatch, the schema reload will be triggered.

Closes #236
GRISHNOV added a commit that referenced this issue Jun 4, 2023
Corrects using of an stale schema to generate metadata during
`crud.update`, `crud.insert`, `crud.insert_*`, `crud.replace`,
`crud.replace_*`, `crud.upsert`, `crud.upsert_*`, `crud.delete`,
`crud.max`, `crud.min`, `crud.select` and `crud.get` working.

If the implemented `fetch_latest_metadata` option is used, it is guaranteed
that the metadata will be up-to-date. Before receiving the space format,
a mismatch check will be performed between the scheme version on all
involved storage and the scheme version in the net_box connection of
the router. In case of mismatch, the schema reload will be triggered.

Closes #236
GRISHNOV added a commit that referenced this issue Jun 5, 2023
Corrects using of an stale schema to generate metadata during
`crud.update`, `crud.insert`, `crud.insert_*`, `crud.replace`,
`crud.replace_*`, `crud.upsert`, `crud.upsert_*`, `crud.delete`,
`crud.max`, `crud.min`, `crud.select` and `crud.get` working.

If the implemented `fetch_latest_metadata` option is used, it is guaranteed
that the metadata will be up-to-date. Before receiving the space format,
a mismatch check will be performed between the scheme version on all
involved storage and the scheme version in the net_box connection of
the router. In case of mismatch, the schema reload will be triggered.

Closes #236
GRISHNOV added a commit that referenced this issue Jun 5, 2023
Corrects using of an stale schema to generate metadata for
`crud.update`, `crud.insert`, `crud.insert_*`, `crud.replace`,
`crud.replace_*`, `crud.upsert`, `crud.upsert_*`, `crud.delete`,
`crud.max`, `crud.min`, `crud.select`, `crud.pairs` and `crud.get`.

If the implemented `fetch_latest_metadata` option is used,
it is guaranteed that the metadata will be up-to-date.
Before receiving the space format, a mismatch check will be
performed between the scheme version on all involved storage
and the scheme version in the net_box connection of the router.
In case of mismatch, the schema reload will be triggered.

Closes #236
GRISHNOV added a commit that referenced this issue Jun 5, 2023
Corrects using of an stale schema to generate metadata for
`crud.update`, `crud.insert`, `crud.insert_*`, `crud.replace`,
`crud.replace_*`, `crud.upsert`, `crud.upsert_*`, `crud.delete`,
`crud.max`, `crud.min`, `crud.select`, `crud.pairs` and `crud.get`.

If the implemented `fetch_latest_metadata` option is used,
it is guaranteed that the metadata will be up-to-date.
Before receiving the space format, a mismatch check will be
performed between the scheme version on all involved storage
and the scheme version in the net_box connection of the router.
In case of mismatch, the schema reload will be triggered.

Closes #236
GRISHNOV added a commit that referenced this issue Jun 5, 2023
Corrects using of an stale schema to generate metadata for
`crud.update`, `crud.insert`, `crud.insert_*`, `crud.replace`,
`crud.replace_*`, `crud.upsert`, `crud.upsert_*`, `crud.delete`,
`crud.max`, `crud.min`, `crud.select`, `crud.pairs` and `crud.get`.

If the implemented `fetch_latest_metadata` option is used,
it is guaranteed that the metadata will be up-to-date.
Before receiving the space format, a mismatch check will be
performed between the scheme version on all involved storage
and the scheme version in the net_box connection of the router.
In case of mismatch, the schema reload will be triggered.

Closes #236
GRISHNOV added a commit that referenced this issue Jun 5, 2023
Corrects using of an stale schema to generate metadata for
`crud.update`, `crud.insert`, `crud.insert_*`, `crud.replace`,
`crud.replace_*`, `crud.upsert`, `crud.upsert_*`, `crud.delete`,
`crud.max`, `crud.min`, `crud.select`, `crud.pairs` and `crud.get`.

If the implemented `fetch_latest_metadata` option is used,
it is guaranteed that the metadata will be up-to-date.
Before receiving the space format, a mismatch check will be
performed between the scheme version on all involved storage
and the scheme version in the net_box connection of the router.
In case of mismatch, the schema reload will be triggered.

Closes #236
GRISHNOV added a commit that referenced this issue Jun 5, 2023
Corrects using of an stale schema to generate metadata for
`crud.update`, `crud.insert`, `crud.insert_*`, `crud.replace`,
`crud.replace_*`, `crud.upsert`, `crud.upsert_*`, `crud.delete`,
`crud.max`, `crud.min`, `crud.select`, `crud.pairs` and `crud.get`.

If the implemented `fetch_latest_metadata` option is used,
it is guaranteed that the metadata will be up-to-date.
Before receiving the space format, a mismatch check will be
performed between the scheme version on all involved storage
and the scheme version in the net_box connection of the router.
In case of mismatch, the schema reload will be triggered.

Closes #236
GRISHNOV added a commit that referenced this issue Jun 5, 2023
Corrects using of an stale schema to generate metadata for
`crud.update`, `crud.insert`, `crud.insert_*`, `crud.replace`,
`crud.replace_*`, `crud.upsert`, `crud.upsert_*`, `crud.delete`,
`crud.max`, `crud.min`, `crud.select`, `crud.pairs` and `crud.get`.

If the implemented `fetch_latest_metadata` option is used,
it is guaranteed that the metadata will be up-to-date.
Before receiving the space format, a mismatch check will be
performed between the scheme version on all involved storage
and the scheme version in the net_box connection of the router.
In case of mismatch, the schema reload will be triggered.

Closes #236
DifferentialOrange pushed a commit that referenced this issue Jun 6, 2023
Corrects using of an stale schema to generate metadata for
`crud.update`, `crud.insert`, `crud.insert_*`, `crud.replace`,
`crud.replace_*`, `crud.upsert`, `crud.upsert_*`, `crud.delete`,
`crud.max`, `crud.min`, `crud.select`, `crud.pairs` and `crud.get`.

If the implemented `fetch_latest_metadata` option is used,
it is guaranteed that the metadata will be up-to-date.
Before receiving the space format, a mismatch check will be
performed between the scheme version on all involved storage
and the scheme version in the net_box connection of the router.
In case of mismatch, the schema reload will be triggered.

Closes #236
DifferentialOrange added a commit that referenced this issue Jun 6, 2023
Overview

  This release add two new flags: `noreturn` to ignore return values
  excessive transfer and encoding/decoding for insert/replace/etc
  (performance improvement up to 10% for batch requests) and
  `fetch_latest_metadata` to force fetching latest space format metadata
  right after a live migration (performance overhead may be up to 15%).

New features
  * Add `noreturn` option for operations:
    `insert`, `insert_object`, `insert_many`, `insert_object_many`,
    `replace`, `replace_object`, `replace_many`, `insert_object_many`,
    `upsert`, `upsert_object`, `upsert_many`, `upsert_object_many`,
    `update`, `delete` (#267).

Bugfixes
  * Crud DML operations returning stale schema for metadata generation.
    Now you may use `fetch_latest_metadata` flag to work with latest
    schema (#236).
DifferentialOrange added a commit that referenced this issue Jun 6, 2023
Overview

  This release add two new flags: `noreturn` to ignore return values
  excessive transfer and encoding/decoding for insert/replace/etc
  (performance improvement up to 10% for batch requests) and
  `fetch_latest_metadata` to force fetching latest space format metadata
  right after a live migration (performance overhead may be up to 15%).

New features
  * Add `noreturn` option for operations:
    `insert`, `insert_object`, `insert_many`, `insert_object_many`,
    `replace`, `replace_object`, `replace_many`, `insert_object_many`,
    `upsert`, `upsert_object`, `upsert_many`, `upsert_object_many`,
    `update`, `delete` (#267).

Bugfixes
  * Crud DML operations returning stale schema for metadata generation.
    Now you may use `fetch_latest_metadata` flag to work with latest
    schema (#236).
DifferentialOrange added a commit that referenced this issue Jun 7, 2023
Overview

  This release add two new flags: `noreturn` to ignore return values
  excessive transfer and encoding/decoding for insert/replace/etc
  (performance improvement up to 10% for batch requests) and
  `fetch_latest_metadata` to force fetching latest space format metadata
  right after a live migration (performance overhead may be up to 15%).

New features
  * Add `noreturn` option for operations:
    `insert`, `insert_object`, `insert_many`, `insert_object_many`,
    `replace`, `replace_object`, `replace_many`, `insert_object_many`,
    `upsert`, `upsert_object`, `upsert_many`, `upsert_object_many`,
    `update`, `delete` (#267).

Bugfixes
  * Crud DML operations returning stale schema for metadata generation.
    Now you may use `fetch_latest_metadata` flag to work with latest
    schema (#236).
DifferentialOrange added a commit that referenced this issue Jun 7, 2023
Overview

  This release add two new flags: `noreturn` to ignore return values
  excessive transfer and encoding/decoding for insert/replace/etc
  (performance improvement up to 10% for batch requests) and
  `fetch_latest_metadata` to force fetching latest space format metadata
  right after a live migration (performance overhead may be up to 15%).

New features
  * Add `noreturn` option for operations:
    `insert`, `insert_object`, `insert_many`, `insert_object_many`,
    `replace`, `replace_object`, `replace_many`, `insert_object_many`,
    `upsert`, `upsert_object`, `upsert_many`, `upsert_object_many`,
    `update`, `delete` (#267).

Bugfixes
  * Crud DML operations returning stale schema for metadata generation.
    Now you may use `fetch_latest_metadata` flag to work with latest
    schema (#236).
DifferentialOrange added a commit that referenced this issue Jun 7, 2023
Overview

  This release add two new flags: `noreturn` to ignore return values
  excessive transfer and encoding/decoding for insert/replace/etc
  (performance improvement up to 10% for batch requests) and
  `fetch_latest_metadata` to force fetching latest space format metadata
  right after a live migration (performance overhead may be up to 15%).

New features
  * Add `noreturn` option for operations:
    `insert`, `insert_object`, `insert_many`, `insert_object_many`,
    `replace`, `replace_object`, `replace_many`, `insert_object_many`,
    `upsert`, `upsert_object`, `upsert_many`, `upsert_object_many`,
    `update`, `delete` (#267).

Bugfixes
  * Crud DML operations returning stale schema for metadata generation.
    Now you may use `fetch_latest_metadata` flag to work with latest
    schema (#236).
DifferentialOrange added a commit that referenced this issue Jun 7, 2023
Overview

  This release add two new flags: `noreturn` to ignore return values
  excessive transfer and encoding/decoding for insert/replace/etc
  (performance improvement up to 10% for batch requests) and
  `fetch_latest_metadata` to force fetching latest space format metadata
  right after a live migration (performance overhead may be up to 15%).

New features
  * Add `noreturn` option for operations:
    `insert`, `insert_object`, `insert_many`, `insert_object_many`,
    `replace`, `replace_object`, `replace_many`, `insert_object_many`,
    `upsert`, `upsert_object`, `upsert_many`, `upsert_object_many`,
    `update`, `delete` (#267).

Bugfixes
  * Crud DML operations returning stale schema for metadata generation.
    Now you may use `fetch_latest_metadata` flag to work with latest
    schema (#236).
DifferentialOrange added a commit that referenced this issue Jun 7, 2023
Overview

  This release add two new flags: `noreturn` to ignore return values
  excessive transfer and encoding/decoding for insert/replace/etc
  (performance improvement up to 10% for batch requests) and
  `fetch_latest_metadata` to force fetching latest space format metadata
  right after a live migration (performance overhead may be up to 15%).

New features
  * Add `noreturn` option for operations:
    `insert`, `insert_object`, `insert_many`, `insert_object_many`,
    `replace`, `replace_object`, `replace_many`, `insert_object_many`,
    `upsert`, `upsert_object`, `upsert_many`, `upsert_object_many`,
    `update`, `delete` (#267).

Bugfixes
  * Crud DML operations returning stale schema for metadata generation.
    Now you may use `fetch_latest_metadata` flag to work with latest
    schema (#236).
DifferentialOrange added a commit that referenced this issue Jun 7, 2023
Overview

  This release add two new flags: `noreturn` to ignore return values
  excessive transfer and encoding/decoding for insert/replace/etc
  (performance improvement up to 10% for batch requests) and
  `fetch_latest_metadata` to force fetching latest space format metadata
  right after a live migration (performance overhead may be up to 15%).

New features
  * Add `noreturn` option for operations:
    `insert`, `insert_object`, `insert_many`, `insert_object_many`,
    `replace`, `replace_object`, `replace_many`, `insert_object_many`,
    `upsert`, `upsert_object`, `upsert_many`, `upsert_object_many`,
    `update`, `delete` (#267).

Bugfixes
  * Crud DML operations returning stale schema for metadata generation.
    Now you may use `fetch_latest_metadata` flag to work with latest
    schema (#236).
DifferentialOrange added a commit that referenced this issue Jun 7, 2023
Overview

  This release add two new flags: `noreturn` to ignore return values
  excessive transfer and encoding/decoding for insert/replace/etc
  (performance improvement up to 10% for batch requests) and
  `fetch_latest_metadata` to force fetching latest space format metadata
  right after a live migration (performance overhead may be up to 15%).

New features
  * Add `noreturn` option for operations:
    `insert`, `insert_object`, `insert_many`, `insert_object_many`,
    `replace`, `replace_object`, `replace_many`, `insert_object_many`,
    `upsert`, `upsert_object`, `upsert_many`, `upsert_object_many`,
    `update`, `delete` (#267).

Bugfixes
  * Crud DML operations returning stale schema for metadata generation.
    Now you may use `fetch_latest_metadata` flag to work with latest
    schema (#236).
DifferentialOrange added a commit that referenced this issue Jun 7, 2023
Overview

  This release add two new flags: `noreturn` to ignore return values
  excessive transfer and encoding/decoding for insert/replace/etc
  (performance improvement up to 10% for batch requests) and
  `fetch_latest_metadata` to force fetching latest space format metadata
  right after a live migration (performance overhead may be up to 15%).

New features
  * Add `noreturn` option for operations:
    `insert`, `insert_object`, `insert_many`, `insert_object_many`,
    `replace`, `replace_object`, `replace_many`, `insert_object_many`,
    `upsert`, `upsert_object`, `upsert_many`, `upsert_object_many`,
    `update`, `delete` (#267).

Bugfixes
  * Crud DML operations returning stale schema for metadata generation.
    Now you may use `fetch_latest_metadata` flag to work with latest
    schema (#236).
@LeonidVas LeonidVas added the 1sp label Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1sp bug Something isn't working teamE
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants