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

Better FHIR Bundle transaction support #1369

Open
rahul1 opened this issue Dec 20, 2022 · 4 comments
Open

Better FHIR Bundle transaction support #1369

rahul1 opened this issue Dec 20, 2022 · 4 comments
Assignees
Labels
fhir-datastore Related to the FHIR datastore, includes API and FHIR operations

Comments

@rahul1
Copy link
Member

rahul1 commented Dec 20, 2022

We might want to consider rolling back changes to FHIR bundles that have partial failures to avoid getting into an inconsistent state.

An example issue: when a batch request fails to create a resource, but successfully creates a resource with a reference to the failed resource.

@rahul1 rahul1 added the fhir-datastore Related to the FHIR datastore, includes API and FHIR operations label Dec 20, 2022
@rahul1 rahul1 added this to the Milestone Quality milestone Sep 22, 2023
@mattwiller
Copy link
Member

I did some investigation around what it would take to more fully support transaction Bundles: we'll likely need to move the existing batch implementation from @medplum/fhir-router into @medplum/server and refactor it significantly.

The new logic for handling batch and transaction Bundles should follow the algorithm outlined in the FHIR HTTP spec. As I understand it, the procedure is as follows:

  • For both kinds of Bundle, group and order the entries by Bundle.entry.request.method
    • Ordered as: deletions, creations, updates, reads
  • While grouping the entries, also resolve each delete, create, or update entry to a relative reference (e.g. Patient/12345), and record a map from Bundle.entry.fullUrl to that reference
    • For DELETE entries, the ID is undefined
    • For POST entries, generate a new ID for the resource
    • For PUT and PATCH entries, use the ID of the resource presented, or resolve the search query in Bundle.entry.request.url for conditional update
    • If any two entries resolve to the same reference, return an error for the entire operation
  • Rewrite all references in the Bundle to point to the correctly-resolved reference strings:
    • Replace all instances within each Bundle.entry.resource of each mapped fullUrl to its corresponding relative reference
      • Not just in Reference elements, but also Resource.id and elements of type uri, url, oid, and uuid
      • Elements of type canonical must NOT be rewritten
    • Also resolve conditional references by performing the given search query and ensuring it returns a single item — otherwise an error is returned for that entry
  • If Bundle is a transaction, start the database transaction
  • Process the entries in order of the groups defined above as they would normally be, issuing DB queries for each one
    • Record the results for each entry to construct the Bundle response
  • If transaction, commit the database transaction

@rahul1
Copy link
Member Author

rahul1 commented Jan 19, 2024

@codyebberson @mattwiller - question about the scope here. Would this implementation also provide transaction guarantees for transaction bundles with ifNoneExist entries? I.e., if two requests land at the same time with same time, both with the same createIfNoneExist entries, only one will go through?

If this functionality isn't included in this ticket, I'll open up a new one - we might have to prioritize that one separately

@codyebberson
Copy link
Member

Bundles: we'll likely need to move the existing batch implementation from @medplum/fhir-router into @medplum/server and refactor it significantly.

I don't think that is necessarily true. I think it could be mitigated by adding a notion of startTransaction() / endTransaction() / rollbackTransaction() to the Repository interface.

In the MockRepository class, those could be no-ops. In the Postgres Repository, those could use BEGIN, COMMIT, ROLLBACK, SAVEPOINT, etc.

The advantage of doing that in the Repository class is that it can manage it's own nested transaction state.

Would this implementation also provide transaction guarantees for transaction bundles with ifNoneExist entries? I.e., if two requests land at the same time with same time, both with the same createIfNoneExist entries, only one will go through?

Yes, it should, as long as we correctly define the interfaces to fully support nested transactions.

@rahul1
Copy link
Member Author

rahul1 commented Jan 22, 2024

Re: the potential performance impact of strict ifNoneExist :

One strategy could be to do something different on transaction vs batch:

  • transaction : slow but reliable
  • batch : fast but inexact

However, this needs to be balanced with potential increase in maintenance burden

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fhir-datastore Related to the FHIR datastore, includes API and FHIR operations
Projects
Status: 👀 In review
Development

No branches or pull requests

3 participants