-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
fix(Ludicrous): Upserts on list type in Dgraph #6754
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor comments. Good work @rahulgurnani . This was a hairy bug!
systest/ludicrous/upsert_test.go
Outdated
} | ||
|
||
func TestSequentialUpdate(t *testing.T) { | ||
t.Log("TestSequentialUpdate") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this please.
systest/ludicrous/upsert_test.go
Outdated
require.NoError(t, err) | ||
schema := ` | ||
name: string @index(exact) . | ||
email: string @index(exact) . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove the email. It seems like we're not using it anywhere.
systest/ludicrous/upsert_test.go
Outdated
@@ -0,0 +1,152 @@ | |||
package worker |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be main
. Also, the license is missing.
systest/ludicrous/upsert_test.go
Outdated
wg.Add(count) | ||
mutation := func(i int) { | ||
defer wg.Done() | ||
query := fmt.Sprintf(`query { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
query := ....
would also work. You don't need the fmt.Sprintf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, thanks!
682b57b
to
b2104bf
Compare
bbc0784
to
f74c9d7
Compare
* Fix DGRAPH-2446: Dgraph missing upserts in ludicrous mode * Add tests * Address review comments * Add a space
* Fix DGRAPH-2446: Dgraph missing upserts in ludicrous mode * Add tests * Address review comments * Add a space
* Fix DGRAPH-2446: Dgraph missing upserts in ludicrous mode * Add tests * Address review comments * Add a space
Cherry-pick of the following into release/v20.07: * fix(Ludicrous): Upserts on list type in Dgraph (#6754) * fix(test): Add sleep before running ludicrous mode tests (#6766) Fixes DGRAPH-2446. Post mortem of issue: https://discuss.dgraph.io/t/dgraph-missing-mutations-in-ludicrous-mode-post-mortem/11019
* Fix DGRAPH-2446: Dgraph missing upserts in ludicrous mode * Add tests * Address review comments * Add a space
* fix(Ludicrous): Upserts on list type in Dgraph (#6754) * Fix DGRAPH-2446: Dgraph missing upserts in ludicrous mode * Add tests * Address review comments * Add a space * fix(test): Add sleep before running ludicrous mode tests (#6766) * Add sleep before running the tests * Fix the assert in tests * Wait for eventual consistency * fix(test): Use dgo/v2 in test for release/v20.03. dgo/v200 is used in release/v20.07 and later. * Add sleep post mutation * Experiment with more sleep Co-authored-by: Daniel Mai <[email protected]>
Fixes DGRAPH-2446
We were not updating startTs for upsert mutations properly in ludicrous mode.
This change is