EventStore / EventStore-Client-Dotnet

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

Performance loss when activating eventstore integration #158

Closed i-dentify closed 2 years ago

i-dentify commented 3 years ago

Describe the bug Made a small POC of dotnet core with graphql and eventstore. Made a nearly identical POC with rust. In both cases the response times where nearly identical as long as I did not had any eventstore integration. Once I did not only return the input but also saved it to eventstore I noticed a massive performance loss on the dotnet side. Whilst I had <3ms response times on both sides without eventstore integration, I had 30+ms on dotnet core and <4ms on rust.

To Reproduce Steps to reproduce the behavior:

  1. See my POC on https://github.com/i-dentify/echarge-vnext

See: https://github.com/EventStore/home/issues/696

thefringeninja commented 3 years ago

@i-dentify,

I have reviewed your core, and there is nothing wrong with the append call you are making. However, we had noticed that appends have been slow because of the overhead of opening a streaming call. To address this we have added an append rpc that uses a single bidirectional call. Please see #121; please note that these changes are unreleased.

thefringeninja commented 3 years ago

@i-dentify,

You can test this now by adding adding the following to your .csproj:

    <PropertyGroup>
        <RestoreSources>
            $(RestoreSources);
            https://api.nuget.org/v3/index.json;
            https://nuget.pkg.github.com/EventStore/index.json;
        </RestoreSources>
    </PropertyGroup>
    <ItemGroup>
        <PackageReference Include="EventStore.Client.Grpc.Streams" Version="21.2.1-*"/>
    </ItemGroup>
i-dentify commented 3 years ago

Oh great - I'll do it asap (it's a private POC - so I have to do my business work first :-) ) and leave a response on it's measures here

i-dentify commented 3 years ago

OK - I updated to the package from the github repository and ran a little script performing the same mutation hundred times. I also ran that script for the rust project.I then disabled eventstore integration and ran the script for the dotnet version again. There is still a significant difference in response times.

Rust version: es-rust-results.txt

Dotnet version: es-dotnet-results.txt

Dotnet version (without EventStore): es-dotnet-results-no-es.txt

YoEight commented 3 years ago

Thanks @i-dentify ! Would you mind sharing your testing code, please? Also what DB configuration you are using?

i-dentify commented 3 years ago

Hi @YoEight,

there is no other DB-configuration than what is configured in the docker-compose.yml, which is available in my test repo noted in my initial message here.

Also there is no other "testing scenario" than just having a curl being executed multiple times...

curl -w \"@$HOME/.curl-format.txt\" -o /dev/null -s

for i in `seq 1 100`; do curltime --request POST --url http://localhost:8100/ --header 'Content-Type: application/json' --data '{"query":"mutation {\n addCar(input: {\n car: {\n name: \"Tesla\", \n batteryCapacity: 70\n }\n }) {\n id\n }\n}\n"}'; done > ./es-rust-results.txt

for i in `seq 1 100`; do curltime --request POST --url http://localhost:5000/graphql --header 'Content-Type: application/json' --data '{"query":"mutation {\n addCar(input: {\n name: \"Tesla\", \n batteryCapacity: 70\n }) {\n id\n }\n}\n"}'; done > ./es-dotnet-results.txt

.curl-format.txt


time_connect:       %{time_connect}s\n
time_appconnect:    %{time_appconnect}s\n
time_pretransfer:   %{time_pretransfer}s\n
time_redirect:      %{time_redirect}s\n
time_starttransfer: %{time_starttransfer}s\n
-----------------------------\n
time_total:         %{time_total}s\n\n\n```
jageall commented 3 years ago

Thanks for the additional info, we are about to go through a QA cycle for the 21.10 release and will look to fold this into the QA tests and let you know when we have further findings.

hayley-jean commented 2 years ago

Thank you for the information @i-dentify, we're still working on adding this to our QA cycle. I'm going to close this issue for now