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

Fix distributed cache key clearing(temporary solution) #6015

Closed
wants to merge 2 commits into from
Closed

Fix distributed cache key clearing(temporary solution) #6015

wants to merge 2 commits into from

Conversation

JadynWong
Copy link
Contributor

@JadynWong JadynWong commented Dec 9, 2021

#5593
We can store the cache key in a cache item, so that multiple instances can read the same cache keys from the cache item.
This is a temporary method, but it will be more available in a multi-instance environment than the current method.

Note that querying the cache keys item will be very expensive when the number of the system cache items is very large. Also since there is no distributed locking, some cache keys may be lost when instantaneously storing a large number of caches in different instances.

@Koushiq
Copy link

Koushiq commented Dec 9, 2021

@JadynWong what do you mean by dedicated cache set ?

@JadynWong
Copy link
Contributor Author

@JadynWong what do you mean by dedicated cache set ?

It may be a description error.

Now all the cache keys in the cache are stored in the static variable keys. To solve the current multi-instance problem, we can store keys as a cache item. So that each instance can read the same keys.

  var (isSet, keys) = TryGetItem<HashSet<string>>(new CacheKey(CACHE_KEYS_KEY));
  if (!isSet)
  {
      keys = new HashSet<string>();
  }
  keys.Add(key.Key);
  _distributedCache.SetString(CACHE_KEYS_KEY, JsonConvert.SerializeObject(keys));

@sinanopcommerce
Copy link

@JadynWong what do you mean by dedicated cache set ?

It may be a description error.

Now all the cache keys in the cache are stored in the static variable keys. To solve the current multi-instance problem, we can store keys as a cache item. So that each instance can read the same keys.

  var (isSet, keys) = TryGetItem<HashSet<string>>(new CacheKey(CACHE_KEYS_KEY));
  if (!isSet)
  {
      keys = new HashSet<string>();
  }
  keys.Add(key.Key);
  _distributedCache.SetString(CACHE_KEYS_KEY, JsonConvert.SerializeObject(keys));

I tried your solution with sql server distributed cache and it is not working. I edited a value and the values it updated at the database not showing at the view.
Scenario
Enable the sqlserver distributed cache then update any value then reload and you will see the same value was showing where the actual value at the database.

Id: Nop.physicalstorelocation.byid.949(Generated by the deafult caching of nopcommerce getbyid);

@JadynWong
Copy link
Contributor Author

JadynWong commented Mar 17, 2022

Hi @sinanopcommerce

Sorry for the very late reply.
I just tested it and could not reproduce your problem. But don't rule out the issue.

Note that querying the cache keys item will be very expensive when the number of the system cache items is very large. Also since there is no distributed locking, some cache keys may be lost when instantaneously storing a large number of caches in different instances.

I will close this PR, and let's wait for a more suitable solution. 🤔

@JadynWong JadynWong closed this Mar 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants