Skip to content

Increment cache_first_available_item by one in uwsgi_cache_set when an index is reused from cache_unused_stack and cache_first_available_item is equal to the index reused.#160

Closed
laurentluce wants to merge 1 commit intounbit:uwsgi-1.4from
laurentluce:uwsgi-1.4

Conversation

@laurentluce
Copy link

Cache is empty to start with:
cache_unused_stack_ptr = 0
cache_first_available_item = 1

Add key "A":
key "A" -> cache_items[1]
cache_first_available_item = 2
cache_hashtable[hash("A")] = 1

Add key "B":
key "B" -> cache_items[2]
cache_first_available_item = 3
cache_hashtable[hash("B")] = 2

Delete key "B"
cache_unused_stack[1] = 2
cache_unused_stack_ptr = 1
cache_hashtable[hash("B")] = 0
cache_first_available_item = 2

Add key "C":
key C -> cache_items[2] - reused from cache_unused_stack[1]
cache_unused_stack_ptr = 0
cache_hashtable[hash("C")] = 2

At this point, cache_first_available_item is still equal to 2 and there is nothing to reuse anymore.
Add key "D":
key D -> cache_items[2] - Issue here is that key "C" is already here so it gets overwritten.
cache_hashtable[hash("D")] = 2

Get key "C":
cache_get_index returns 0 because the hash at cache_items[2] is not the same as hash("C"). cache_hashtable[hash("C")] does contains index 2 though.

Now if you try to add key "C". The key "C" will be added over and over and create more and more collisions. Getting key "C" will still return None.

I think cache_first_available_item should be incremented by one in uwsgi_cache_set when an index is reused from cache_unused_stack and cache_first_available_item is equal to the index reused.

…n index is reused from cache_unused_stack and cache_first_available_item is equal to the index reused.
@laurentluce laurentluce reopened this Feb 27, 2013
@unbit
Copy link
Owner

unbit commented Feb 27, 2013

Great, you have found the problem, but i think it is better to remove the (useless) optimization in cache_del:

ce7d84b

can you check if it solve your problem ? (i have applied it only in 1.4 branch)

@laurentluce
Copy link
Author

I ran the following test:

cache_set('A', pickle.dumps('test1'))
cache_set('B', pickle.dumps('test2'))
cache_del('B')
cache_set('C', pickle.dumps('test3'))
cache_set('D', pickle.dumps('test4'))
if not cache_get('C')
    print 'ERROR'

With both patches (yours and mine), cache_get returns the value for the key 'C' while it does not with the original code. The issue I was seeing seems fixed now.

I agree that your patch is better as it gets rid of the useless optimization in the first place versus mine which is correcting the first available item when the index is reused.

@unbit
Copy link
Owner

unbit commented Feb 27, 2013

ok, i will make a couple more tests and i will release 1.4.7.

Thanks a lot for your help (your credits will be in the public changelog)

@laurentluce
Copy link
Author

Do you have an ETA for 1.4.7?

@unbit
Copy link
Owner

unbit commented Mar 1, 2013

today ;)

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.

2 participants