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

Remove parallel upserts for child entities #617

Conversation

jmbrunskill
Copy link
Contributor

Fixes #616

Description

Discovered that parallel child node inserts seems to trigger dgraph transaction errors.
For some reason this was not seen with earlier inserts...

I've increased the retry time to 20ms, so hopefully increases the chances for the dgraph system to recover before retries kick in.

Tests:

  • No new tests required

I've tested adding Diclofenac as well as editing it.
I'm slightly concerned there was a reason I did children first, before parent but seems to work fine in this order...

};
child_codes.push(child_entity.code);
))
})??;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is double ?? ok, or better to be explicit?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine to me? If you're down in the weeds enough to care about one ?, you can probably notice two?

@jmbrunskill
Copy link
Contributor Author

jmbrunskill commented Mar 21, 2024

We still have an issue where a pending change might not be insertable in future, however I think it should now at least create the parent element first, so worst case we can reject a broken pending change an re-enter the other records.

It would be possible for all entities to be inserted by their links could be missing. Not sure if there's a good way to handle without a transction.
We could create our own transaction of sorts where we try to undo but that sounds pretty complex...

Copy link
Collaborator

@lache-melvin lache-melvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm slightly concerned there was a reason I did children first, before parent but seems to work fine in this order...

Yeah I had the same thought... but working fine for me too - let's get it in! 🤞

@jmbrunskill jmbrunskill merged commit 6326299 into develop Mar 21, 2024
1 check passed
@jmbrunskill jmbrunskill deleted the 616-when-approving-drug-it-saved-itself-but-does-not-appear-in-the-drug-list-and-says-it-already-exists branch March 21, 2024 04:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

when approving drug it saved itself but does not appear in the drug list and says it already exists
2 participants