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

IDs of task can duplicated when mvсс is on #207

Open
R-omk opened this issue Jun 13, 2023 · 2 comments
Open

IDs of task can duplicated when mvсс is on #207

R-omk opened this issue Jun 13, 2023 · 2 comments
Labels
bug Something isn't working teamE

Comments

@R-omk
Copy link

R-omk commented Jun 13, 2023

function method.put(self, data, opts)
local max = self.space.index.task_id:max()
local id = max and max[1] + 1 or 0
local task = self.space:insert{id, state.READY, data}

To minimize the possibility of conflicts, MVCC uses what is called best-effort visibility: for write transactions it chooses read-committed, for read transactions it chooses read-confirmed.

by default task_id:max read confirmed , this is the key reason why duplicates occur in the case of an race condition

@LeonidVas LeonidVas added teamE bug Something isn't working labels Jun 16, 2023
@better0fdead
Copy link
Contributor

@R-omk Hi, do you have a reproducer?

@R-omk
Copy link
Author

R-omk commented Jun 22, 2023

I just reported an obvious problem that someone encountered in the telegram chat.

I think you should use require('txn_proxy') to reproduce issue without external client. (or net box client)

better0fdead added a commit that referenced this issue Jul 6, 2023
Taking the maximum of the index is an implicit transactions, so it is
always done with 'read-confirmed' mvcc isolation level.
It can lead to errors when trying to make parallel 'put' calls with mvcc enabled.
It is hapenning because 'max' for several puts in parallel will be the same since
read confirmed isolation level makes visible all transactions that finished the commit.
To fix it we wrap it with box.begin/commit and set right isolation level.

Closes #207
better0fdead added a commit that referenced this issue Jul 10, 2023
Taking the maximum of the index is an implicit transactions, so it is
always done with 'read-confirmed' mvcc isolation level.
It can lead to errors when trying to make parallel 'put' calls with mvcc enabled.
It is hapenning because 'max' for several puts in parallel will be the same since
read confirmed isolation level makes visible all transactions that finished the commit.
To fix it we wrap it with box.begin/commit and set right isolation level.

Closes #207
better0fdead added a commit that referenced this issue Jul 10, 2023
Taking the maximum of the index is an implicit transactions, so it is
always done with 'read-confirmed' mvcc isolation level.
It can lead to errors when trying to make parallel 'put' calls with mvcc enabled.
It is hapenning because 'max' for several puts in parallel will be the same since
read confirmed isolation level makes visible all transactions that finished the commit.
To fix it we wrap it with box.begin/commit and set right isolation level.

Closes #207
better0fdead added a commit that referenced this issue Jul 10, 2023
Taking the maximum of the index is an implicit transactions, so it is
always done with 'read-confirmed' mvcc isolation level.
It can lead to errors when trying to make parallel 'put' calls with mvcc enabled.
It is hapenning because 'max' for several puts in parallel will be the same since
read confirmed isolation level makes visible all transactions that finished the commit.
To fix it we wrap it with box.begin/commit and set right isolation level.

Closes #207
better0fdead added a commit that referenced this issue Jul 11, 2023
Taking the maximum of the index is an implicit transactions, so it is
always done with 'read-confirmed' mvcc isolation level.
It can lead to errors when trying to make parallel 'put' calls with mvcc enabled.
It is hapenning because 'max' for several puts in parallel will be the same since
read confirmed isolation level makes visible all transactions that finished the commit.
To fix it we wrap it with box.begin/commit and set right isolation level.

Closes #207
better0fdead added a commit that referenced this issue Jul 11, 2023
Taking the maximum of the index is an implicit transactions, so it is
always done with 'read-confirmed' mvcc isolation level.
It can lead to errors when trying to make parallel 'put' calls with mvcc enabled.
It is hapenning because 'max' for several puts in parallel will be the same since
read confirmed isolation level makes visible all transactions that finished the commit.
To fix it we wrap it with box.begin/commit and set right isolation level.

Closes #207
better0fdead added a commit that referenced this issue Jul 27, 2023
Taking the maximum of the index is an implicit transactions, so it is
always done with 'read-confirmed' mvcc isolation level.
It can lead to errors when trying to make parallel 'put' calls with mvcc enabled.
It is hapenning because 'max' for several puts in parallel will be the same since
read confirmed isolation level makes visible all transactions that finished the commit.
To fix it we wrap it with box.begin/commit and set right isolation level.
Current fix does not resolve that bug in situations when we already are in transaction
since it will open nested transactions.

Part of #207
better0fdead added a commit that referenced this issue Jul 27, 2023
Taking the maximum of the index is an implicit transactions, so it is
always done with 'read-confirmed' mvcc isolation level.
It can lead to errors when trying to make parallel 'put' calls with mvcc enabled.
It is hapenning because 'max' for several puts in parallel will be the same since
read confirmed isolation level makes visible all transactions that finished the commit.
To fix it we wrap it with box.begin/commit and set right isolation level.
Current fix does not resolve that bug in situations when we already are in transaction
since it will open nested transactions.

Part of #207
oleg-jukovec added a commit to tarantool/sharded-queue that referenced this issue Aug 10, 2023
By default idx:max() or idx:min() have read confirmed isolation level.
It could lead to a task duplication or double task take when we
already insert or update a task, commited, but it is not yet
confirmed.

See also:

1. tarantool/queue#207
2. tarantool/queue#211
oleg-jukovec added a commit to tarantool/sharded-queue that referenced this issue Aug 10, 2023
By default idx:max() or idx:min() have read confirmed isolation level.
It could lead to a task duplication or double task take when we
already insert or update a task, commited, but it is not yet
confirmed.

See also:

1. tarantool/queue#207
2. tarantool/queue#211
oleg-jukovec added a commit to tarantool/sharded-queue that referenced this issue Aug 10, 2023
By default idx:max() or idx:min() have read confirmed isolation level.
It could lead to a task duplication or double task take when we
already insert or update a task, commited, but it is not yet
confirmed.

See also:

1. tarantool/queue#207
2. tarantool/queue#211
better0fdead added a commit that referenced this issue Aug 10, 2023
Taking the maximum of the index is an implicit transactions, so it is
always done with 'read-confirmed' mvcc isolation level.
It can lead to errors when trying to make parallel 'put' calls with mvcc enabled.
It is hapenning because 'max' for several puts in parallel will be the same since
read confirmed isolation level makes visible all transactions that finished the commit.
To fix it we wrap it with box.begin/commit and set right isolation level.
Current fix does not resolve that bug in situations when we already are in transaction
since it will open nested transactions.

Part of #207
better0fdead added a commit that referenced this issue Aug 10, 2023
Taking the minimum of the index is an implicit transactions, so it is
always done with 'read-confirmed' mvcc isolation level.
It can lead to errors when trying to make parallel 'take' calls with mvcc enabled.
It is hapenning because 'min' for several takes in parallel will be the same since
read confirmed isolation level makes visible all transactions that finished the commit.
To fix it we wrap it with box.begin/commit and set right isolation level.
Current fix does not resolve that bug in situations when we already are in transaction
since it will open nested transactions.

Part of #207
better0fdead added a commit that referenced this issue Aug 15, 2023
Taking the maximum or minimum of the index is an implicit transactions, so it is
always done with 'read-confirmed' mvcc isolation level.
It can lead to errors when trying to make parallel 'put' or 'take' calls with mvcc enabled.
It is hapenning because 'max' or 'min' for several puts in parallel will be the same since
read confirmed isolation level makes visible all transactions that finished the commit.
To fix it we wrap it with box.begin/commit and set right isolation level.
Current fix does not resolve that bug in situations when we already are in transaction
since it will open nested transactions.

Part of #207
better0fdead added a commit that referenced this issue Aug 15, 2023
Taking the maximum or minimum of the index is an implicit transactions, so it is
always done with 'read-confirmed' mvcc isolation level.
It can lead to errors when trying to make parallel 'put' or 'take' calls with mvcc enabled.
It is hapenning because 'max' or 'min' for several puts in parallel will be the same since
read confirmed isolation level makes visible all transactions that finished the commit.
To fix it we wrap it with box.begin/commit and set right isolation level.
Current fix does not resolve that bug in situations when we already are in transaction
since it will open nested transactions.

Part of #207
better0fdead added a commit that referenced this issue Aug 15, 2023
Taking the maximum or minimum of the index is an implicit transactions, so it is
always done with 'read-confirmed' mvcc isolation level.
It can lead to errors when trying to make parallel 'put' or 'take' calls with mvcc enabled.
It is hapenning because 'max' or 'min' for several puts in parallel will be the same since
read confirmed isolation level makes visible all transactions that finished the commit.
To fix it we wrap it with box.begin/commit and set right isolation level.
Current fix does not resolve that bug in situations when we already are in transaction
since it will open nested transactions.

Part of #207
better0fdead added a commit that referenced this issue Aug 16, 2023
Taking the maximum or minimum of the index is an implicit transactions, so it is
always done with 'read-confirmed' mvcc isolation level.
It can lead to errors when trying to make parallel 'put' or 'take' calls with mvcc enabled.
It is hapenning because 'max' or 'min' for several puts in parallel will be the same since
read confirmed isolation level makes visible all transactions that finished the commit.
To fix it we wrap it with box.begin/commit and set right isolation level.
Current fix does not resolve that bug in situations when we already are in transaction
since it will open nested transactions.

Part of #207
LeonidVas pushed a commit that referenced this issue Aug 16, 2023
Taking the maximum or minimum of the index is an implicit transactions, so it is
always done with 'read-confirmed' mvcc isolation level.
It can lead to errors when trying to make parallel 'put' or 'take' calls with mvcc enabled.
It is hapenning because 'max' or 'min' for several puts in parallel will be the same since
read confirmed isolation level makes visible all transactions that finished the commit.
To fix it we wrap it with box.begin/commit and set right isolation level.
Current fix does not resolve that bug in situations when we already are in transaction
since it will open nested transactions.

Part of #207
LeonidVas pushed a commit that referenced this issue Aug 16, 2023
Taking the maximum or minimum of the index is an implicit transactions, so it is
always done with 'read-confirmed' mvcc isolation level.
It can lead to errors when trying to make parallel 'put' or 'take' calls with mvcc enabled.
It is hapenning because 'max' or 'min' for several puts in parallel will be the same since
read confirmed isolation level makes visible all transactions that finished the commit.
To fix it we wrap it with box.begin/commit and set right isolation level.
Current fix does not resolve that bug in situations when we already are in transaction
since it will open nested transactions.

Part of #207
@better0fdead better0fdead removed their assignment Aug 21, 2023
oleg-jukovec added a commit to tarantool/sharded-queue that referenced this issue Sep 6, 2023
By default idx:max() or idx:min() have read confirmed isolation level.
It could lead to a task duplication or double task take when we
already insert or update a task, commited, but it is not yet
confirmed.

See also:

1. tarantool/queue#207
2. tarantool/queue#211
oleg-jukovec added a commit to tarantool/sharded-queue that referenced this issue Sep 25, 2023
By default idx:max() or idx:min() have read confirmed isolation level.
It could lead to a task duplication or double task take when we
already insert or update a task, commited, but it is not yet
confirmed.

See also:

1. tarantool/queue#207
2. tarantool/queue#211
oleg-jukovec added a commit to tarantool/sharded-queue that referenced this issue Oct 31, 2023
By default idx:max() or idx:min() have read confirmed isolation level.
It could lead to a task duplication or double task take when we
already insert or update a task, commited, but it is not yet
confirmed.

See also:

1. tarantool/queue#207
2. tarantool/queue#211
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working teamE
Projects
None yet
Development

No branches or pull requests

3 participants