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

[Enhancement] Optimizations for Bundle serialization #3733

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

brendankowitz
Copy link
Member

@brendankowitz brendankowitz commented Feb 29, 2024

Description

High-level goals:

  • Allow Bundle Sub-request Resource to be passed through with async cotext
  • Allows Bundle Handler to use Raw strings when building response
  • This should reduce overall CPU, Memory and Latency while processing bundles.

Sequence of requests:

---
title: Bundle Processing
---

sequenceDiagram
    Request->>JsonFormatter: http
    JsonFormatter->>FHIRController: Deserialize
    FHIRController->>Mediatr: Handle
    Mediatr->>BundleHandler: Process
    
loop Each Request
    BundleHandler-->>+Request: Passes model through FHIRContext
    Request-->>JsonFormatter: Begin Sub-request
    JsonFormatter-->>FHIRController: Read model
    FHIRController-->>Mediatr: Handle operation
    Mediatr-->>DataStore: Serialize and persist
    DataStore--)Mediatr: Returns a "RawResource"
    Mediatr--)FHIRController: Raw Response
    FHIRController--)JsonFormatter: 
    JsonFormatter--)Request: Writes raw response
    Request--)-BundleHandler: Return Raw Entry response
end

    BundleHandler->>FHIRController: Build Bundle
    FHIRController-)JsonFormatter: Serialize Raw Bundle
    JsonFormatter-)Request: http
Loading

Details:

This pull request primarily involves changes to the Microsoft.Health.Fhir.Core project to improve serialization and deserialization of FHIR resources. The most significant changes include refactoring the RawResourceElementExtensions.cs and ResourceDeserializer.cs files to enhance serialization and deserialization respectively, adding a new SubRequest.cs class, and modifying the ResourceDeserializer.cs and RawResourceElement.cs files to improve resource deserialization.

Major changes:

Minor changes:

Related issues

Addresses AB#117384

Testing

Adds tests, existing e2e tests. Manual testing.

FHIR Team Checklist

  • Update the title of the PR to be succinct and less than 65 characters
  • Add a milestone to the PR for the sprint that it is merged (i.e. add S47)
  • Tag the PR with the type of update: Bug, Build, Dependencies, Enhancement, New-Feature or Documentation
  • Tag the PR with Open source, Azure API for FHIR (CosmosDB or common code) or Azure Healthcare APIs (SQL or common code) to specify where this change is intended to be released.
  • Tag the PR with Schema Version backward compatible or Schema Version backward incompatible or Schema Version unchanged if this adds or updates Sql script which is/is not backward compatible with the code.
  • CI is green before merge Build Status
  • Review squash-merge requirements

Semver Change (docs)

Patch (enhancement)

@brendankowitz brendankowitz force-pushed the personal/bkowitz/bundle-perf-serialization branch 3 times, most recently from 76e70fc to 93bb5cd Compare March 1, 2024 00:37
@brendankowitz brendankowitz force-pushed the personal/bkowitz/bundle-perf-serialization branch 2 times, most recently from 12498cd to af3c7fe Compare March 1, 2024 08:31
@brendankowitz brendankowitz force-pushed the personal/bkowitz/bundle-perf-serialization branch 2 times, most recently from af3c7fe to 055a106 Compare March 1, 2024 15:48
@brendankowitz brendankowitz added Enhancement-Refactor Refactor on existing functionality. Azure API for FHIR Label denotes that the issue or PR is relevant to the Azure API for FHIR Azure Healthcare APIs Label denotes that the issue or PR is relevant to the FHIR service in the Azure Healthcare APIs labels Mar 1, 2024
@brendankowitz brendankowitz added this to the S135 milestone Mar 1, 2024
@brendankowitz brendankowitz changed the title Allow Bundle Sub-request Resource to be passed through with async cotext Avoids unnecessary serialization when processing Bundles Mar 1, 2024
@SergeyGaluzo

This comment was marked as resolved.

This comment was marked as outdated.

@brendankowitz brendankowitz marked this pull request as ready for review March 5, 2024 01:41
@brendankowitz brendankowitz requested a review from a team as a code owner March 5, 2024 01:41
@brendankowitz brendankowitz modified the milestones: S135, S136 Mar 5, 2024
@brendankowitz brendankowitz force-pushed the personal/bkowitz/bundle-perf-serialization branch 2 times, most recently from ed37f06 to 96b8305 Compare March 5, 2024 19:07
@brendankowitz

This comment was marked as resolved.

This comment was marked as resolved.

@brendankowitz brendankowitz changed the title Avoids unnecessary serialization when processing Bundles [Enhancement] Optimizations for Bundle serialization Mar 21, 2024
fhibf
fhibf previously approved these changes Apr 18, 2024
@brendankowitz brendankowitz modified the milestones: S136, S140 Apr 30, 2024
@brendankowitz brendankowitz force-pushed the personal/bkowitz/bundle-perf-serialization branch from e174636 to 1ca2a54 Compare May 8, 2024 00:28
@brendankowitz brendankowitz force-pushed the personal/bkowitz/bundle-perf-serialization branch from 1ca2a54 to 33979c2 Compare May 13, 2024 18:09
var rawComponent = component as RawBundleEntryComponent;
var rawOutcomeComponent = component?.Response as RawBundleResponseComponent;

b.WriteOptionalString("fullUrl", component.FullUrl)

Check warning

Code scanning / CodeQL

Dereferenced variable may be null Warning

Variable
component
may be null at this access as suggested by
this
null check.
@mikaelweave mikaelweave modified the milestones: S140, S142 Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure API for FHIR Label denotes that the issue or PR is relevant to the Azure API for FHIR Azure Healthcare APIs Label denotes that the issue or PR is relevant to the FHIR service in the Azure Healthcare APIs Enhancement-Refactor Refactor on existing functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants