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

Upsert transactions may upsert multiple copies of a record #2149

Closed
aphyr opened this Issue Feb 21, 2018 · 6 comments

Comments

Projects
None yet
3 participants
@aphyr

aphyr commented Feb 21, 2018

The howto says

Upsert operations are intended to be run concurrently, as per the needs of the application. As such, it’s possible that two concurrently running operations could try to add the same node at the same time. For example, both try to add a user with the same email address. If they do, then one of the transactions will fail with an error indicating that the transaction was aborted.

However, multiple concurrent upsert transactions for the same key may succeed, resulting in the creation of multiple, rather than one, records for that key. For instance, in 20180216T075315.000-0600.zip, four upserts complete on the same key, resulting in entities 0x01, 0x02, 0x04, and 0x05:

3	:invoke	:upsert	nil
0	:invoke	:upsert	nil
2	:invoke	:upsert	nil
4	:invoke	:upsert	nil
1	:invoke	:upsert	nil
3	:ok	:upsert	"0x1"
4	:ok	:upsert	"0x5"
1	:ok	:upsert	"0x2"
0	:ok	:upsert	"0x4"
2	:fail	:upsert	nil	:conflict
1	:invoke	:read	nil
4	:invoke	:read	nil
3	:invoke	:read	nil
2	:invoke	:read	nil
0	:invoke	:read	nil
4	:ok	:read	("0x1" "0x2" "0x4" "0x5")
2	:ok	:read	("0x1" "0x2" "0x4" "0x5")
3	:ok	:read	("0x1" "0x2" "0x4" "0x5")
1	:ok	:read	("0x1" "0x2" "0x4" "0x5")
0	:ok	:read	("0x1" "0x2" "0x4" "0x5")

This is easy to reproduce on a fresh cluster with no failures. You can demonstrate this failure mode with Jepsen 07abf364c2364c710f08a6f49392851a94f83c76, on 1.0.3, with lein run test -w upsert.

@aphyr

This comment has been minimized.

Show comment
Hide comment
@aphyr

aphyr Feb 21, 2018

(This is just for completeness; this issue was fixed by 7f9c659)

aphyr commented Feb 21, 2018

(This is just for completeness; this issue was fixed by 7f9c659)

@aphyr aphyr closed this Feb 21, 2018

@aphyr

This comment has been minimized.

Show comment
Hide comment
@aphyr

aphyr Mar 30, 2018

I think I may have spoke too soon; this also looks to be present on

Dgraph version   : v1.0.4
Commit SHA-1     : 224b560
Commit timestamp : 2018-03-27 16:56:17 +0530
Branch           : fix/jepsen_delete

For instance, 20180330T110412.000-0500.zip shows 10/10 processes successfully upserting to a single key, where only one should succeed.

aphyr commented Mar 30, 2018

I think I may have spoke too soon; this also looks to be present on

Dgraph version   : v1.0.4
Commit SHA-1     : 224b560
Commit timestamp : 2018-03-27 16:56:17 +0530
Branch           : fix/jepsen_delete

For instance, 20180330T110412.000-0500.zip shows 10/10 processes successfully upserting to a single key, where only one should succeed.

@aphyr aphyr reopened this Mar 30, 2018

@janardhan1993

This comment has been minimized.

Show comment
Hide comment
@janardhan1993

janardhan1993 Apr 2, 2018

Contributor

@aphyr There was a change in v1.0.4, we need to specify @upsert in the schema otherwise index keys won't be used for conflict detection.

Contributor

janardhan1993 commented Apr 2, 2018

@aphyr There was a change in v1.0.4, we need to specify @upsert in the schema otherwise index keys won't be used for conflict detection.

@manishrjain manishrjain added the bug label Apr 2, 2018

@manishrjain manishrjain added this to the Sprint-001 milestone Apr 2, 2018

@janardhan1993

This comment has been minimized.

Show comment
Hide comment
@janardhan1993

janardhan1993 Apr 4, 2018

Contributor

Fixed.

Contributor

janardhan1993 commented Apr 4, 2018

Fixed.

@aphyr

This comment has been minimized.

Show comment
Hide comment
@aphyr

aphyr Apr 4, 2018

Yeah, looks like the dev build you gave me earlier passes, with the upsert directive. I've also expanded the upsert test to try hundreds of keys (instead of one), and with several hours of inserts with and without partitions, I can't reproduce the conflict any more. :)

Dgraph version : v1.0.4
Commit SHA-1 : 224b560
Commit timestamp : 2018-03-27 16:56:17 +0530
Branch : fix/jepsen_delete

aphyr commented Apr 4, 2018

Yeah, looks like the dev build you gave me earlier passes, with the upsert directive. I've also expanded the upsert test to try hundreds of keys (instead of one), and with several hours of inserts with and without partitions, I can't reproduce the conflict any more. :)

Dgraph version : v1.0.4
Commit SHA-1 : 224b560
Commit timestamp : 2018-03-27 16:56:17 +0530
Branch : fix/jepsen_delete

@manishrjain

This comment has been minimized.

Show comment
Hide comment
@manishrjain

manishrjain Apr 5, 2018

Member

Whoop! Whoop!

Member

manishrjain commented Apr 5, 2018

Whoop! Whoop!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment