Implement CloneAsync

[tovyhnal]

We are seeing performance degradation of our services (thread processing gets stuck) and after initial investigation one of the suspect is Clone method of ServiceClient. I checked the implementation of Clone method and there are two things, which can potentially cause performance/stability issues:

• Sync over async
• Not accepting/honoring cancellation token and creating new one instead when calling DV

Sync over async Found at least two places, when we do sync over async during Clone operation, this can block whole thread, which can lead to thread pool starvation.

-WhoAmIResponse resp = Task.Run(async () => await _connectionSvc.GetWhoAmIDetails(this).ConfigureAwait(false)).ConfigureAwait(false).GetAwaiter().GetResult();

-RefreshInstanceDetails(svc, _targetInstanceUriToConnectTo).ConfigureAwait(false).GetAwaiter().GetResult();

Not honoring cancellation token Also there are places, which don't accept cancellation tokens when calling external service. This can lead to stuck requests and blocking calling thread. Here is one of multiple examples: resp = (WhoAmIResponse)(await Command_WebAPIProcess_ExecuteAsync( req, null, false, null, Guid.Empty, false, _configuration.Value.MaxRetryCount, _configuration.Value.RetryPauseTime, new CancellationToken(), inLoginFlow:true).ConfigureAwait(false));

Would be possible to create CloneAsync(CancellationToken ct), which would be fully async and honor passed cancellation token?

We use v1.1.16

[MattB-msft]

@tovyhnal

Thanks for the feedback. In DVSC, you should move away from using Clone + Sync methods to using Async Methods only without clone. Clone + Sync is present for backward compatibility for folks upgrading from the CrmServiceClient.

You will find that the Async methods do not require a cloned connection to scale horizontally.

Regarding cancelation tokens, the tokens are honored only withing the Dataverse service client itself, the API to the server does not currently support them. Once the request is executed on against the server, it cannot be canceled.

Regarding 1.1.16 and clone, We fixed a few issues that were reported with clone in 1.1.17, You can see the full change set here: https://github.com/microsoft/PowerPlatform-DataverseServiceClient/releases/tag/1.1.17 The perf issue your seeing, I believe, we fixed in 1.1.17. ( Fix for Clone being called concurrently causing unnecessary calls to dataverse. GitHub reported - Fix https://github.com/microsoft/PowerPlatform-DataverseServiceClient/issues/422)

Please let us know if 1.1.17 solves that for you.. if that does not fix it for you please reopen the issue. Also, please look at adopting the async methods primarily vs using clone.

[tovyhnal]

@MattB-msft thanks for reply. I believe the reason, why we Clone the client is described in this issue #93. When calling DV server, we always use async methods

[MattB-msft]

@tovyhnal You should not need to use clone if your using a Singleton. Just do not dispose the client when your done with it.

that said. try it out with the current version of DVSC and see if the perf issue persists. pretty sure we got that cleaned up.

[mathiasbl]

@tovyhnal

Thanks for the feedback. In DVSC, you should move away from using Clone + Sync methods to using Async Methods only without clone. Clone + Sync is present for backward compatibility for folks upgrading from the CrmServiceClient.

You will find that the Async methods do not require a cloned connection to scale horizontally.

Regarding cancelation tokens, the tokens are honored only withing the Dataverse service client itself, the API to the server does not currently support them. Once the request is executed on against the server, it cannot be canceled.

Regarding 1.1.16 and clone, We fixed a few issues that were reported with clone in 1.1.17, You can see the full change set here: https://github.com/microsoft/PowerPlatform-DataverseServiceClient/releases/tag/1.1.17 The perf issue your seeing, I believe, we fixed in 1.1.17. ( Fix for Clone being called concurrently causing unnecessary calls to dataverse. GitHub reported - Fix #422)

Please let us know if 1.1.17 solves that for you.. if that does not fix it for you please reopen the issue. Also, please look at adopting the async methods primarily vs using clone.

@MattB-msft Since you are advising to move away from the Sync methods I want to point your attention to the linq provider. The linq provider (OrgServiceContext) is still very popular to easily write queries. However this is still using the sync methods forcing us to use the Clone method to initialize a context.

Could the Linq provider get some async love?

[MattB-msft]

@tovyhnal We would like to do that at some point. However the limitations on that have to do with how Async is currently implemented. Async support is available only in the client library's right now.. its not available server side yet.