Fixed CosmosSqlMetadataProvider concurrency issue.

[michaelstaib]

The CosmosSqlMetdataProvider uses a standard dictionary although the metadata provider is accessed and updated concurrently. This causes the issue in #2694. I have updated the implementation to use a ConcurrentDictionary.

Fixes #2694

[Copilot]

Pull Request Overview

This PR fixes a concurrency issue in CosmosSqlMetadataProvider by replacing a non-thread-safe dictionary with a ConcurrentDictionary and adding argument null checks to its partition-key methods.

• Replaced Dictionary<string, string> with ConcurrentDictionary<string, string>
• Added ArgumentNullException.ThrowIfNull guards in GetPartitionKeyPath and SetPartitionKeyPath
• Switched from TryAdd/assignment to a single AddOrUpdate call in SetPartitionKeyPath

Comments suppressed due to low confidence (4)

src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs:554

• Add unit tests that simulate concurrent calls to SetPartitionKeyPath and GetPartitionKeyPath to verify the thread-safe behavior of the new ConcurrentDictionary implementation.

_partitionKeyPaths.AddOrUpdate("${database}/{container}", partitionKeyPath, (key, oldValue) => partitionKeyPath);

src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs:554

• [nitpick] The key format "${database}/{container}" is used in multiple places; consider extracting it into a private helper method to avoid duplication and reduce the risk of mismatches.

_partitionKeyPaths.AddOrUpdate("${database}/{container}", partitionKeyPath, (key, oldValue) => partitionKeyPath);

src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs:539

• Update the XML doc comments for GetPartitionKeyPath and SetPartitionKeyPath to mention that they throw ArgumentNullException for null inputs, so consumers know about these new exceptions.

public string? GetPartitionKeyPath(string database, string container)

src/Core/Services/MetadataProviders/CosmosSqlMetadataProvider.cs:540

• Introducing ArgumentNullException here changes the behavior of GetPartitionKeyPath (it now throws instead of returning null for null inputs). Confirm this breaking change is intended or document it in the public API.

ArgumentNullException.ThrowIfNull(database);

[golfalot]

Hello @sajeetharan , I appreciate the team have lots of other work in progress already, but this fix looks pretty much good to go and should be a quick job to review thanks to the hard work of @michaelstaib.

The problems this causes for us operationally are severe. We would be most grateful if you could bump this up the priority list and get it into a release very soon. Thank you!

[sajeetharan]

@michaelstaib Thanks @golfalot for sure , will track it with next release cc : @Aniruddh25 @sourabh1007 @JerryNixon

[Aniruddh25]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 6 pipeline(s).

[Aniruddh25]

LGTM. Thank you for identifying and fixing this!

[Aniruddh25]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 6 pipeline(s).