FoundatioFx / Foundatio.Redis

Foundatio Redis
Apache License 2.0
98 stars 31 forks source link

Support different read mode in RedisHybridCacheClient #83

Closed zinniawang149 closed 8 months ago

zinniawang149 commented 8 months ago

At the moment, Even if we setup the Read mode in RedisHybridCacheClientOptions while creating the RedisHybridCacheClient instance, it doesn't work when we access the Redis.

The reason for that is because when we call the base object like this, we didn't pass the options.ReadMode in. https://github.com/FoundatioFx/Foundatio.Redis/blob/ceb704b100ef519ea9adbc391b45fe56a5384691/src/Foundatio.Redis/Cache/RedisHybridCacheClient.cs#L9

So the base readmode is always CommandFlags.None. https://github.com/FoundatioFx/Foundatio.Redis/blob/ceb704b100ef519ea9adbc391b45fe56a5384691/src/Foundatio.Redis/Cache/RedisCacheClientOptions.cs#L17

Here is the example before/after the code update when we set the read mode as PreferReplica, the GetTypeCmds indicates the total number of read-only type commands. This is derived from the Redis commandstats statistic by summing all of the read-only type commands (get, hget, scard, lrange, and so on.):

The 001 nodes of both shards are the masters:

image

Before the code update, we can see all the read call still goes to the master node.

image

After the code update, the application starts fetching data from read replica.

image
CLAassistant commented 8 months ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Zinnia Wang seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant commented 8 months ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Zinnia Wang seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

zinniawang149 commented 8 months ago

Have signed the Contributor License Agreement.

niemyjski commented 8 months ago

Thanks for the PR! Also, those charts were awesome, would you mind sharing more on this (do you have a script that generates images automatically? I've been needing to look to see if there is a good way to do this with otel.):

the GetTypeCmds indicates the total number of read-only type commands. This is derived from the Redis commandstats statistic by summing all of the read-only type commands (get, hget, scard, lrange, and so on.):

zinniawang149 commented 8 months ago

@niemyjski Thanks for accepting the change. The charts were actually screenshots I took from AWS CloudWatch metrics for Elasticache haha.

Also, wondering if you can release a new version? Because I need this change to improve the performance of our system.

niemyjski commented 8 months ago

Please use a nightly feed (would be good to see if you see any other issues), I need to check to see what else we need to do before next release.

zinniawang149 commented 8 months ago

@niemyjski I tried to run this cmd: dotnet add package Foundatio.Redis --version 10.7.1-alpha.0.1 https://github.com/FoundatioFx/Foundatio.Redis/pkgs/nuget/Foundatio.Redis/189858430 but got the error of Package 'Foundatio.Redis' is incompatible with 'all' frameworks in project It looks to me like it is unable to find package Foundatio.Redis with version (>= 10.7.1-alpha.0.1) on Nuget

Can you see if it is working on your side?

niemyjski commented 7 months ago

Do you have a package source for the nighties defined? https://github.com/exceptionless/Exceptionless/blob/883d75b47a1bab7009e960a615429223162f5be4/NuGet.Config#L4

zinniawang149 commented 7 months ago

Right. It's working! Thanks for your help.