Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ClearRegion not working for redis implementation #64

Open
bigerock opened this issue Jun 15, 2016 · 17 comments · May be fixed by #340
Open

ClearRegion not working for redis implementation #64

bigerock opened this issue Jun 15, 2016 · 17 comments · May be fixed by #340
Assignees
Labels
Milestone

Comments

@bigerock
Copy link

bigerock commented Jun 15, 2016

Although it "shouldn't" matter, First, I am using Redis in Azure. My configuration is as follows. It's two cache handles, Memory and Redis, with a Redis backplane.

var ret = CacheFactory.Build("memandrediscache", settings =>
        {
            settings
                .WithSystemRuntimeCacheHandle("inProcessCache")
                .EnableStatistics()
                .And
                .WithRedisConfiguration(redisKey, config => config
                   .WithAllowAdmin()
                   .WithConnectionTimeout(10000)
                   .WithEndpoint(endPointUrl, endPointPort)
                   .WithPassword(password)
                   .WithSsl()
                )
                .WithJsonSerializer(jsonSer, jsonDeSer)
                .WithMaxRetries(4) //from 1000
                .WithRetryTimeout(10000)
                .WithRedisBackplane(redisKey)
                .WithRedisCacheHandle(redisKey, true);
        });

When I try to clear the region, the following code gives a zero index result, meaning no hashKeys are found and therefore doesn't continue on to clear the region.

code location: CacheManager.StackExchange.Redis\RedisCacheHandle.cs > ClearRegion(string region){} > line 166 in the code i have...
var hashKeys = this.connection.Database.HashKeys(region);

An example of one of the keys I'm trying to clear in Redis:
"data:_cache.data.counts.dcsnav.316914602"

My code to clear the region is as follows:

var cache = CreateInMemoryCacheWithRedisBackplane(); // gets the CacheFactory.Build object
from above
cache.ClearRegion(region.ToLower()); //value of 'region' is 'data'

@TMiNus
Copy link

TMiNus commented Jun 15, 2016

I can confirm that clearing a region works for the redis implementation. I use it on daily basis, so I would assume you are doing something wrong. The configuration part seems fine although a little cumbersome. Can you put the result of the command KEYS * before and after the ClearRegion operation

@MichaCo
Copy link
Owner

MichaCo commented Jun 16, 2016

I do not see any code actually adding items to the cache. Are you doing that somewhere else?

@MichaCo
Copy link
Owner

MichaCo commented Jun 16, 2016

I again tested the clear region code and the unit tests actually testing also the redis layer. All works fine it seems.
I might be able to test it on Azure from home at some point. Don't know why it shouldn't work though ~~

So in the end, I'm sorry, I cannot really reproduce any error. If you can provide us with a small program to really reproduce the behavior, that would be great

@bigerock
Copy link
Author

bigerock commented Jun 16, 2016

ok - i have something for you. you will need to create your own (free) azure redis db and put the url:port and password in the web.config.

you might not even need to get that far and might notice something i'm doing wrong in the setup.

thanks!

here's the web project:
https://goo.gl/xG9d3v

@TMiNus
Copy link

TMiNus commented Jun 16, 2016

I have to debug it further but the initial problem is that in the local redis instance when it saves a key with a region it actually writes two keys, one for the name of the region and another for the actual key in the format {region}:{key}. The problem is that when it writes to Azure it just writes the actual key and not the name of the region. BTW I would recommend the use of an IoC instead of the static class that you have there, if you want to go with the static class I would recommend implementing the Singleton in a class and the logic in another class

@bigerock
Copy link
Author

ok. i'm not sure if you had a chance to set up the azure redis, but it looks like all it does is prefix the region name on the key. for example, 'mykey', with 'someregion' becomes 'someregion:mykey' as the key.

@TMiNus
Copy link

TMiNus commented Jun 16, 2016

All the keys in CacheManager are hashes. The name of the actual key can be whatever you like, in this case it appends : to the name to create the key in the format {region}:{key}. What ClearRegion does is retrieve all the keys that are contained in the hash with the name of the region and then it proceeds to delete all those keys. As you can imagine if in Azure it doesn't write the hash with the name of the region it cannot delete the actual keys cuz it doesn't know where they are. The debug process will be then on the write operation and why it's failing to write the hash with the region name to Azure (local works)

@TMiNus
Copy link

TMiNus commented Jun 16, 2016

OK to further narrow the problem. In your insert operation you are using Put, if you change that to an Add operation it actually works, writing the hash key with the region name to Azure. So.. I would say that the problem is in the Put operation. Another thing that I detected is that if you have key in the format {region}:{key} and you use an Add operation to add another key with the same region, the hash key with the region name is not added

@MichaCo
Copy link
Owner

MichaCo commented Jun 16, 2016

guys, you are totally correct. Put has a bug not setting the region lookup key.
Thanks for catching that!

To clarify, that was introduced during the change to use lua scritps for put and add... I changed the logic slightly. And ofc I used Add in all the unit tests ;)

So as a work around, try use Add instead of Put if you can. Sorry for that

Will be fixed soonish

@MichaCo MichaCo added the bug label Jun 16, 2016
@MichaCo MichaCo self-assigned this Jun 16, 2016
@MichaCo MichaCo added this to the Version 0.9 milestone Jun 16, 2016
@MichaCo
Copy link
Owner

MichaCo commented Jun 16, 2016

@TMiNus
Can you explain that a little bit more?

Another thing that I detected is that if you have key in the format {region}:{key} and you use an Add operation to add another key with the same region, the hash key with the region name is not added

So, you add regionA:keyA and then add regionA:keyA again right? In that case the key exists and will not be added again, which is the purpose of Add.

If you add another key like regionA:keyB the region key regionA should have both, keyA and keyB as hash fields now. The region key itself will be added only if it doesn't exist.
Is there anything else you encountered?

@bigerock
Copy link
Author

thanks all. nothing to be sorry for. it's a great product. look forward to the updates. thanks again!

also, i noticed the same thing happening on Remove() for a given object. not only on the ClearRegion()

@TMiNus
Copy link

TMiNus commented Jun 16, 2016

Oh the Lua scripts, I totally missed that one. I disabled the Lua scripts on my version, that is why it was working all the time for me 😁

@MichaCo what I saw in one testing was that if you have a key in the form regionA:keyA without regionA being added to the keys pool (this bug), if you try to use the Add operation to put something like regionA:keyB, the actual key was added but the key hash with the name of the region was not created. @bigerock can you explain further the behavior that you saw with the Remove operation

@Andrei-
Copy link

Andrei- commented Nov 28, 2023

Looks like this bug is back in v1.2.0.
When using Put, the cache region lookup key is not present when I check KEYS * in redis-cli.
Project running on .NET 6.0, references look like this:

    <PackageReference Include="CacheManager.Core" Version="1.2.0" />
    <PackageReference Include="CacheManager.Microsoft.Extensions.Caching.Memory" Version="1.2.0" />
    <PackageReference Include="CacheManager.Microsoft.Extensions.Configuration" Version="1.2.0" />
    <PackageReference Include="CacheManager.Microsoft.Extensions.Logging" Version="1.2.0" />
    <PackageReference Include="CacheManager.Serialization.Json" Version="1.2.0" />
    <PackageReference Include="CacheManager.StackExchange.Redis" Version="1.2.0" />

@MichaCo
Copy link
Owner

MichaCo commented Nov 29, 2023

@Andrei- I'm not so sure about how that could have happened, but if you have a reproduceable code example, feel free to post it here

Thanks

@Andrei-
Copy link

Andrei- commented Jan 2, 2024

Hi @MichaCo - I found the problem. We have at some point upgraded the reference of StackExchange.Redis package to version="2.6.104" because of some warnings with older versions.
It turns out that this behavior starts with version 2.6.48 of StackExchange.Redis package. (Where cached data via Put with region param will not add the region index, so ClearRegion will not work for these keys.
I'm adding a small code sample to reproduce
Test.ClearRegion.zip

@MichaCo
Copy link
Owner

MichaCo commented Jan 2, 2024

Thanks for the details @Andrei- I'll try to figure out why that happens

@MichaCo
Copy link
Owner

MichaCo commented Aug 9, 2024

can reproduce the error - reopening

@MichaCo MichaCo reopened this Aug 9, 2024
@MichaCo MichaCo modified the milestones: Version 0.9, 2.0.0 Aug 9, 2024
MichaCo added a commit that referenced this issue Aug 9, 2024
* removed a bunch of nugets
* unified new .NET targets - not final yet
* get it compile and stuff...
* fixed #64 again by moving the region key update into the lua script.
@MichaCo MichaCo linked a pull request Aug 9, 2024 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants