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

Should we create a new instance every time we use FhirClient? #2633

Closed
whyfate opened this issue Dec 5, 2023 · 2 comments
Closed

Should we create a new instance every time we use FhirClient? #2633

whyfate opened this issue Dec 5, 2023 · 2 comments

Comments

@whyfate
Copy link
Contributor

whyfate commented Dec 5, 2023

Describe the bug
In concurrent mode, using a singleton fhirclient to query resources has a certain chance of returning other resources, but not every time a new instance is added.

Version used:

  • FHIR Version: [R4]
  • Version: [e.g. 5.4.0]

My test code

static async System.Threading.Tasks.Task ParallelTestAsync()
{
    await Console.Out.WriteLineAsync("Start test...");
    await Console.Out.WriteLineAsync("Singleton mode starting...");
    var watch = Stopwatch.StartNew();
    await ParallelSearchAsync(true);
    watch.Stop();
    await Console.Out.WriteLineAsync($"Singleton mode 1k finish:{watch.ElapsedMilliseconds}ms");

    await Console.Out.WriteLineAsync("Transient mode starting...");
    watch.Restart();
    await ParallelSearchAsync(false);
    watch.Stop();
    await Console.Out.WriteLineAsync($"Transient mode 1k finish:{watch.ElapsedMilliseconds}ms");
}

static async System.Threading.Tasks.Task ParallelSearchAsync(bool useSingleFhirClient)
{
    var baseURL = "http://192.168.120.237:35000/fhir";
    var singleFhirClient = new FhirClient(baseURL);
    var list = new List<int> { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11 };
    var resourceNames = new string[] { "Organization", "Location", "Practitioner", "PractitionerRole", "ServiceRequest", "Device" };
    var index = 0;
    while (index < 1000)
    {
        await Parallel.ForEachAsync(list, async (i, token) =>
        {
            var resourceName = resourceNames[i % 6];
            var url = $"{baseURL}/{resourceName}";

            FhirClient fhirClient = null;

            if (useSingleFhirClient)
            {
                fhirClient = singleFhirClient;
            }
            else
            {
                fhirClient = new FhirClient(baseURL);
            }

            var bundle = await fhirClient.GetAsync(url) as Bundle;
            var targetType = bundle.Entry.FirstOrDefault().Resource.TypeName;
            if (targetType != resourceName)
            {
                var sourceColor = Console.ForegroundColor;
                Console.ForegroundColor = ConsoleColor.Red;
                await Console.Out.WriteLineAsync($"{index}-search error,search resource:{resourceName},target resource:{targetType}...");
                Console.ForegroundColor = sourceColor;
            }

            if (!useSingleFhirClient)
            {
                fhirClient.Dispose();
            }

        });

        index++;
        if (index % 100 == 0)
        {
            await Console.Out.WriteLineAsync($"{index} times completed...");
        }
    }
}

image

@brianpos
Copy link
Collaborator

brianpos commented Dec 6, 2023

I would recommend that yes you should be using a new fhirclient each time, however you may choose to use the factory mode httpclient as a parameter into the constructor if you're under high load which can then pool sockets safely there.
Pretty sure there was another closed issue on this from the past which resulted in the inclusion of that constructor.

@whyfate
Copy link
Contributor Author

whyfate commented Dec 7, 2023

@brianpos Thank you very much for your answer.

@whyfate whyfate closed this as completed Dec 7, 2023
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

No branches or pull requests

2 participants