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

Non-symmetric results with conflicting default null value #7536

Closed
sync-by-unito bot opened this issue Apr 2, 2024 · 10 comments
Closed

Non-symmetric results with conflicting default null value #7536

sync-by-unito bot opened this issue Apr 2, 2024 · 10 comments
Assignees

Comments

@sync-by-unito
Copy link

sync-by-unito bot commented Apr 2, 2024

Hi, I'm running into an odd issue with the StateApplierTool and applying conflicting isDefault writes. It looks like there may be some weirdness around the handling for null payloads/OT

The repro inputs are attached here and detailed below

Changeset #1
Upload Message: [
    {
        "timestamp": 200,
        "instructions": [
          {
             "summary": "CreateObject { table: \"ww\", id: UUID(3e3b5f26-a6b8-c14c-e4d7-4abe8e34ad8d) }",
             "raw": "PwACd3cCAAs+O18mprjBTOTXSr6ONK2N"
          },
          {
             "summary": "Update { (table=\"ww\", ID=UUID(3e3b5f26-a6b8-c14c-e4d7-4abe8e34ad8d), fullPath=\"qu\"), payload: -6.355252517323399e+307, isDefault: true, priorSize: 0 }",
             "raw": "PwACd3c/AQJxdQQACz47XyamuMFM5NdKvo40rY0BAAeqDn59HaDW/wE="
          },
          {
             "summary": "Update { (table=\"ww\", ID=UUID(3e3b5f26-a6b8-c14c-e4d7-4abe8e34ad8d), fullPath=\"ur\"), payload: 6.192209238937289e+307, isDefault: true, priorSize: 0 }",
             "raw": "PwACd3c/AQJ1cgQACz47XyamuMFM5NdKvo40rY0BAAcmfHTmhAvWfwE="
          }
       ]
    }
],

IDENT

Changeset #2
Download Message:[
    {  
       "timestamp": 100,
       "instructions": [
          {
             "summary": "CreateObject { table: \"ww\", id: UUID(3e3b5f26-a6b8-c14c-e4d7-4abe8e34ad8d) }",
             "raw": "PwACd3cCAAs+O18mprjBTOTXSr6ONK2N"
          },
          {
             "summary": "Update { (table=\"ww\", ID=UUID(3e3b5f26-a6b8-c14c-e4d7-4abe8e34ad8d), fullPath=\"qu\"), payload: null, isDefault: true, priorSize: 0 }",
             "raw": "PwACd3c/AQJxdQQACz47XyamuMFM5NdKvo40rY0BAAAB"
          },
          {
             "summary": "Update { (table=\"ww\", ID=UUID(3e3b5f26-a6b8-c14c-e4d7-4abe8e34ad8d), fullPath=\"ur\"), payload: -1.338641371363204e+307, isDefault: true, priorSize: 0 }",
             "raw": "PwACd3c/AQJ1cgQACz47XyamuMFM5NdKvo40rY0BAAfGE0VKGRCz/wE="
          }
       ]
    }
] 

This series of events correctly produces the following realm: 

{
    "_id": 3e3b5f26-a6b8-c14c-e4d7-4abe8e34ad8d,
    "qu": -6.355252517323399e+307,
    "ur": 6.192209238937289e+307,
}

If we instead flip the upload and download messages (Upload Changeset #2 and Download Changeset #1) we get the following realm where the field "qu" gets set to null but "ur" is still set to the correct value from CS1

{
    "_id": 3e3b5f26-a6b8-c14c-e4d7-4abe8e34ad8d,
    "qu": null,
    "ur": 6.192209238937289e+307,
}

Verbose logs from state applier in the case that ends up with the incorrect result

found upload changeset: 1 1 100 1 39
Decoded changeset: [changeset with 3 instructions]
found upload changeset: 2 2 100 1 88
Decoded changeset: [changeset with 3 instructions]
group.get_or_add_table_with_primary_key(group, "class_ww", type_UUID, "_id", true, TopLevel);
integrated local changesets as version 6
sync::create_object_with_primary_key(group, get_table("class_ww"), 3e3b5f26-a6b8-c14c-e4d7-4abe8e34ad8d);
integrated local changesets as version 7
decoding download message. {download: {server: 2, client: 0} upload: {server: 0, client: 0}, latest: 0}
found download changeset: serverVersion: 1, clientVersion: 0, origin: 2 [changeset with 3 instructions]
found download changeset: serverVersion: 2, clientVersion: 0, origin: 2 [changeset with 3 instructions]
Scanning incoming changeset [1/2] (3 instructions)
Scanning incoming changeset [2/2] (3 instructions)
Scanning local changeset [1/4] (3 instructions)
Scanning local changeset [2/4] (3 instructions)
Scanning local changeset [3/4] (1 instructions)
Scanning local changeset [4/4] (4 instructions)
Indexing incoming changeset [1/2] (3 instructions)
Indexing incoming changeset [2/2] (3 instructions)
Finished changeset indexing (incoming: 2 changeset(s) / 6 instructions, local: 4 changeset(s) / 11 instructions, conflict group(s): 2)
Transforming local changeset [1/4] through 2 incoming changeset(s) with 2 conflict group(s)
Transforming local changeset [2/4] through 2 incoming changeset(s) with 2 conflict group(s)
Transforming local changeset [3/4] through 2 incoming changeset(s) with 2 conflict group(s)
Transforming local changeset [4/4] through 2 incoming changeset(s) with 2 conflict group(s)
Finished transforming 4 local changesets through 2 incoming changesets (11 vs 6 instructions, in 2 conflict groups)
Integrated 2 changesets out of 2 
Copy link
Author

sync-by-unito bot commented Apr 2, 2024

➤ On 2024-03-15, danieltabacaru commented:

Do you mean ApplyToStateCommand or is it some other tool? If so, do you still have the input file you apply to the realm?

Copy link
Author

sync-by-unito bot commented Apr 2, 2024

➤ On 2024-03-18, Sean Brandenburg commented:

Hey [~daniel.tabacaru@mongodb.com], sorry I should have left a note about this. The .txt files attached to this ticket are the input files for realm-apply-to-state which does look to be the name of the build output of ApplyToStateCommand

Copy link
Author

sync-by-unito bot commented Apr 2, 2024

➤ On 2024-03-18, danieltabacaru commented:

I see. So those two messages are enough to reproduce it?

Copy link
Author

sync-by-unito bot commented Apr 2, 2024

➤ On 2024-03-18, Sean Brandenburg commented:

Yep, the correctResult.txt is the ordering detailed in the first code block that gives the expected result and in resultsInNull.txt we flip Changeset #1 and Changeset #2 and get the unexpected null result

Copy link
Author

sync-by-unito bot commented Apr 2, 2024

➤ On 2024-03-18, danieltabacaru commented:

great. thanks a lot!

Copy link
Author

sync-by-unito bot commented Apr 2, 2024

➤ On 2024-03-22, danieltabacaru commented:

I tried applying the files, they both fail with the same error:

libc++abi: terminating due to uncaught exception of type realm::sync::BadChangesetError: Unexpected intern index: 5201903. 

I wrote a test to apply the same instructions and I see no issue with it. I checked the OT rule and I can see nothing wrong with it either.

Looking at the state applier logs, I see two download messages (each with 3 instructions) being applied, but you only mention one. There are also 4 local changesets. Is it the same test? And, how did you flip the messages?

Copy link
Author

sync-by-unito bot commented Apr 2, 2024

➤ On 2024-03-22, Sean Brandenburg commented:

Hey [~daniel.tabacaru@mongodb.com] . These 2 output files are generated by our fuzz tests which try to apply the same changesets as the downloaded and uploaded messages with the apply to state tool. I just tried to run this again and it still seems to work from me. It looks like the --version command is broken for the realm-apply-to-state tool, but I downloaded my version of the tool within the last month or two.

I've attached screenshots showing how I'm running the tool and what the resulting realm looks like. Both runs were done with a fresh realm file (it looks like in the verbose output I added earlier I had applied some other stuff to the realm, but the issue persists with a fresh realm)

!correct.png|thumbnail! !incorrect.png|thumbnail!

Copy link
Author

sync-by-unito bot commented Apr 2, 2024

➤ On 2024-03-22, Sean Brandenburg commented:

Also to you point about the log showing 2 downloads, I'm not super sure why that is. The input file only has 1 download changeset and 1 upload changeset, each with 3 instructions (and looking at the input file I do only see 1 of each)

Copy link
Author

sync-by-unito bot commented Apr 2, 2024

➤ On 2024-03-26, danieltabacaru commented:

There is indeed a bug in the replication code. Setting a Mixed field to null generates two instructions, and the first one does not take is_default into account. If it is then merged with an instruction with is_default = true, it wins the conflict resolution (default values lose to non-default values) when maybe it shouldn't have (when both are default values, the timestamp is used as a tiebreak).

Copy link
Author

sync-by-unito bot commented Apr 2, 2024

➤ PM Bot commented:

Jira ticket: RCORE-1980

@sync-by-unito sync-by-unito bot closed this as completed Apr 2, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant