EventStore / EventStore-Client-Dotnet

Dotnet Client SDK for the Event Store gRPC Client API written in C#
Other
140 stars 38 forks source link

What was the reason to implement connection string parser manually with lots of allocations? #261

Closed xperiandri closed 4 months ago

xperiandri commented 1 year ago

Describe the bug Why https://github.com/EventStore/EventStore-Client-Dotnet/blob/d2476d2f9574a014578d9430ed38765b7cc7372d/src/EventStore.Client/EventStoreClientSettings.ConnectionString.cs#L15 has a lot of custom logic full of allocations?

Why not copy https://github.com/dotnet/aspnetcore/blob/main/src/Http/WebUtilities/src/QueryHelpers.cs https://github.com/dotnet/aspnetcore/blob/main/src/Shared/QueryStringEnumerable.cs into this repo, parse a connection string into a Uri and then parse a query string using that highly optimized classes?

Expected behavior Existing code used instead of inveting a wheel

Actual behavior Custom code used

EventStore details

ylorph commented 1 year ago

Because the connection string is not an URI: it can be of the form esdb://node1:2113,node2:2113,node2:2113 ( when using node seed)

new Uri("esdb://node1:2113,node2:2113,node2:2113"); will yield an exception

Remember also that this parsing is done once and the resulting connection instance is expteced and should be shared across the process using it , for the lifetime of the process.

timothycoleman commented 11 months ago

Hi @xperiandri, just in case it's relevant to you, the client is thread safe and is typically reused for the lifetime of the application. Normally there isn't a need to repeatedly instantiate the client and incur those allocations