StackExchange / StackExchange.Redis

General purpose redis client
https://stackexchange.github.io/StackExchange.Redis/
Other
5.87k stars 1.51k forks source link

Do not send tracers when running second iteration of cluster nodes di… #2525

Closed Matiszak closed 12 months ago

Matiszak commented 1 year ago

…scovery (#2520)

Sending tracers is not necessary because we just connected to the nodes a few seconds ago. It causes problems because sent tracers do not have enough time to respond.

Matiszak commented 1 year ago

I'm currently testing the change below on your CI but it seems I somwhow broke 1/2 tests, I'm gonna check why:

// This awaits either the endpoint's initial connection, or a tracer if we're already connected
 // (which is the reconfigure case**, except second iteration which is only for newly discovered cluster members**).
var isFirstIteration = iter == 0;
available[i] = server.OnConnectedAsync(log, sendTracerIfConnected: isFirstIteration, autoConfigureIfConnected: reconfigureAll);
Matiszak commented 1 year ago

It's good that all checks have passed but I noticed this changed version as well as unchanged version (based on tag 2.6.122) is quite flaky and sometimes failes, sometimes not. Examples: https://github.com/StackExchange/StackExchange.Redis/actions/runs/5881471149/job/15950011765?pr=2525 https://github.com/StackExchange/StackExchange.Redis/actions/runs/5881109242/job/15948901359?pr=2525 https://github.com/StackExchange/StackExchange.Redis/actions/runs/5879015803/job/15942221751?pr=2525 https://github.com/StackExchange/StackExchange.Redis/actions/runs/5881109242?pr=2525