From b3020e60f9eaea01bae4eed352bc24b1da9d2aa7 Mon Sep 17 00:00:00 2001 From: Unbit Date: Thu, 20 Feb 2014 14:19:40 +0100 Subject: [PATCH] fixed long standing bug with caches --- core/cache.c | 104 +++++++++++++++++++++------------------------- t/cachebitmap.ini | 2 + t/cachebitmap.py | 73 ++++++++++++++++++++++++++++---- 3 files changed, 115 insertions(+), 64 deletions(-) diff --git a/core/cache.c b/core/cache.c index 08bcfe87..daef1cb3 100644 --- a/core/cache.c +++ b/core/cache.c @@ -157,7 +157,7 @@ static void cache_unmark_blocks(struct uwsgi_cache *uc, uint64_t index, uint64_t mask >>= (7 - last_byte_bit); mask <<= (7 - last_byte_bit); } - + // here we use AND (0+0 = 0 | 1+0 = 0 | 0+1 = 0| 1+1 = 1) // 0 in mask means "unmark", 1 in mask means "do not change" // so we need to invert the mask @@ -306,19 +306,23 @@ next2: void uwsgi_cache_init(struct uwsgi_cache *uc) { uc->hashtable = uwsgi_calloc_shared(sizeof(uint64_t) * uc->hashsize); - uc->unused_blocks_stack = uwsgi_calloc_shared(sizeof(uint64_t) * uc->blocks); - // the first cache item is always zero - uc->first_available_block = 1; + uc->unused_blocks_stack = uwsgi_calloc_shared(sizeof(uint64_t) * uc->max_items); uc->unused_blocks_stack_ptr = 0; uc->filesize = ( (sizeof(struct uwsgi_cache_item)+uc->keysize) * uc->max_items) + (uc->blocksize * uc->blocks); + uint64_t i; + for (i = 1; i < uc->max_items; i++) { + uc->unused_blocks_stack_ptr++; + uc->unused_blocks_stack[uc->unused_blocks_stack_ptr] = i; + } + if (uc->use_blocks_bitmap) { uc->blocks_bitmap_size = uc->blocks/8; uint8_t m = uc->blocks % 8; if (m > 0) uc->blocks_bitmap_size++; uc->blocks_bitmap = uwsgi_calloc_shared(uc->blocks_bitmap_size); if (m > 0) { - uc->blocks_bitmap[uc->blocks_bitmap_size-1] = 0xff >> (8 - m); + uc->blocks_bitmap[uc->blocks_bitmap_size-1] = 0xff >> m; } } @@ -558,42 +562,44 @@ int uwsgi_cache_del2(struct uwsgi_cache *uc, char *key, uint16_t keylen, uint64_ if (index) { uci = cache_item(index); - // unmark blocks - if (uci->keysize > 0 && uc->blocks_bitmap) { - cache_unmark_blocks(uc, uci->first_block, uci->valsize); + if (uci->keysize > 0) { + // unmark blocks + if (uc->blocks_bitmap) cache_unmark_blocks(uc, uci->first_block, uci->valsize); + // put back the block in unused stack + uc->unused_blocks_stack_ptr++; + uc->unused_blocks_stack[uc->unused_blocks_stack_ptr] = index; + + // unlink prev and next (if any) + if (uci->prev) { + struct uwsgi_cache_item *ucii = cache_item(uci->prev); + ucii->next = uci->next; + } + else { + // set next as the new entry point (could be 0) + uc->hashtable[uci->hash % uc->hashsize] = uci->next; + } + + if (uci->next) { + struct uwsgi_cache_item *ucii = cache_item(uci->next); + ucii->prev = uci->prev; + } + + if (!uci->prev && !uci->next) { + // reset hashtable entry + uc->hashtable[uci->hash % uc->hashsize] = 0; + } + uc->n_items--; } + + ret = 0; + uci->keysize = 0; uci->valsize = 0; - uc->unused_blocks_stack_ptr++; - uc->unused_blocks_stack[uc->unused_blocks_stack_ptr] = index; - ret = 0; - // relink collisioned entry - if (uci->prev) { - struct uwsgi_cache_item *ucii = cache_item(uci->prev); - ucii->next = uci->next; - } - else { - // set next as the new entry point (could be 0) - uc->hashtable[uci->hash % uc->hashsize] = uci->next; - } - - if (uci->next) { - struct uwsgi_cache_item *ucii = cache_item(uci->next); - ucii->prev = uci->prev; - } - - if (!uci->prev && !uci->next) { - // reset hashtable entry - //uwsgi_log("!!! resetted hashtable entry !!!\n"); - uc->hashtable[uci->hash % uc->hashsize] = 0; - } uci->hash = 0; uci->prev = 0; uci->next = 0; uci->expires = 0; - uc->n_items--; - if (uc->use_last_modified) { uc->last_modified_at = uwsgi_now(); } @@ -611,7 +617,10 @@ void uwsgi_cache_fix(struct uwsgi_cache *uc) { uint64_t i; unsigned long long restored = 0; - for (i = 0; i < uc->max_items; i++) { + // reset unused blocks + uc->unused_blocks_stack_ptr = 0; + + for (i = 1; i < uc->max_items; i++) { // valid record ? struct uwsgi_cache_item *uci = cache_item(i); if (uci->keysize) { @@ -623,7 +632,6 @@ void uwsgi_cache_fix(struct uwsgi_cache *uc) { } else { // put this record in unused stack - uc->first_available_block = i; uc->unused_blocks_stack_ptr++; uc->unused_blocks_stack[uc->unused_blocks_stack_ptr] = i; } @@ -640,7 +648,6 @@ int uwsgi_cache_set2(struct uwsgi_cache *uc, char *key, uint16_t keylen, char *v struct uwsgi_cache_item *uci, *ucii; // used to reset key allocation in bitmap mode - uint8_t rollback_mode = 0; int ret = -1; time_t now = 0; @@ -658,23 +665,14 @@ int uwsgi_cache_set2(struct uwsgi_cache *uc, char *key, uint16_t keylen, char *v //uwsgi_log("putting cache data in key %.*s %d\n", keylen, key, vallen); index = uwsgi_cache_get_index(uc, key, keylen); if (!index) { - if (uc->first_available_block >= uc->max_items && !uc->unused_blocks_stack_ptr) { + if (!uc->unused_blocks_stack_ptr) { uwsgi_log("*** DANGER cache \"%s\" is FULL !!! ***\n", uc->name); uc->full++; goto end; } - if (uc->unused_blocks_stack_ptr) { - index = uc->unused_blocks_stack[uc->unused_blocks_stack_ptr]; - uc->unused_blocks_stack_ptr--; - } - else { - rollback_mode = 1; - index = uc->first_available_block; - if (uc->first_available_block < uc->max_items) { - rollback_mode = 2; - uc->first_available_block++; - } - } + + index = uc->unused_blocks_stack[uc->unused_blocks_stack_ptr]; + uc->unused_blocks_stack_ptr--; uci = cache_item(index); if (!uc->blocks_bitmap) { @@ -682,16 +680,10 @@ int uwsgi_cache_set2(struct uwsgi_cache *uc, char *key, uint16_t keylen, char *v } else { uci->first_block = uwsgi_cache_find_free_blocks(uc, vallen); - //uwsgi_log("first block = %llu\n", uci->first_block); if (uci->first_block == 0xffffffffffffffffLLU) { uwsgi_log("*** DANGER cache \"%s\" is FULL !!! ***\n", uc->name); uc->full++; - if (rollback_mode == 0) { - uc->unused_blocks_stack_ptr++; - } - else if (rollback_mode == 2) { - uc->first_available_block--; - } + uc->unused_blocks_stack_ptr++; goto end; } // mark used blocks; diff --git a/t/cachebitmap.ini b/t/cachebitmap.ini index 7dcc5374..605e7acc 100644 --- a/t/cachebitmap.ini +++ b/t/cachebitmap.ini @@ -7,4 +7,6 @@ cache2 = name=items_3,blocks=4,items=4,bitmap=1,blocksize=1 cache2 = name=items_4,blocks=5,items=5,bitmap=1,blocksize=1 cache2 = name=items_17,blocks=17,items=17,bitmap=1,blocksize=1 cache2 = name=items_4_10,blocks=5,items=5,bitmap=1,blocksize=10 +cache2 = name=items_1_100000,blocks=1000,items=2,bitmap=1,blocksize=100 +cache2 = name=items_non_bitmap,items=2,blocksize=20 pyrun = t/cachebitmap.py diff --git a/t/cachebitmap.py b/t/cachebitmap.py index ee0b7380..a491ee92 100644 --- a/t/cachebitmap.py +++ b/t/cachebitmap.py @@ -1,16 +1,18 @@ import uwsgi import unittest +import random +import string class BitmapTest(unittest.TestCase): - __caches__ = ['items_1', 'items_2', 'items_3', 'items_4', 'items_17', 'items_4_10'] + __caches__ = ['items_1', 'items_2', 'items_3', 'items_4', 'items_17', 'items_4_10', 'items_1_100000', 'items_non_bitmap'] def setUp(self): for cache in self.__caches__: uwsgi.cache_clear(cache) def test_failed_by_one(self): - self.assertFalse(uwsgi.cache_update('key1', 'HELLO', 0, 'items_1')) + self.assertIsNone(uwsgi.cache_update('key1', 'HELLO', 0, 'items_1')) def test_ok_four_bytes(self): self.assertTrue(uwsgi.cache_update('key1', 'HELL', 0, 'items_1')) @@ -19,31 +21,86 @@ class BitmapTest(unittest.TestCase): self.assertTrue(uwsgi.cache_update('key1', 'HE', 0, 'items_2')) self.assertTrue(uwsgi.cache_update('key2', 'LL', 0, 'items_2')) self.assertTrue(uwsgi.cache_del('key1', 'items_2')) - self.assertFalse(uwsgi.cache_update('key1', 'HEL', 0, 'items_2')) + self.assertIsNone(uwsgi.cache_update('key1', 'HEL', 0, 'items_2')) self.assertTrue(uwsgi.cache_update('key1', 'HE', 0, 'items_2')) def test_overlapping(self): self.assertTrue(uwsgi.cache_update('key1', 'HE', 0, 'items_2')) - self.assertFalse(uwsgi.cache_update('key1', 'HELL', 0, 'items_2')) + self.assertIsNone(uwsgi.cache_update('key1', 'HELL', 0, 'items_2')) self.assertTrue(uwsgi.cache_del('key1', 'items_2')) self.assertTrue(uwsgi.cache_update('key1', 'HELL', 0, 'items_2')) def test_big_item(self): - self.assertFalse(uwsgi.cache_update('key1', 'HELLOHELLOHELLOHEL', 0, 'items_17')) + self.assertIsNone(uwsgi.cache_update('key1', 'HELLOHELLOHELLOHEL', 0, 'items_17')) self.assertTrue(uwsgi.cache_update('key1', 'HELLOHELLOHELLOHE', 0, 'items_17')) def test_set(self): self.assertTrue(uwsgi.cache_set('key1', 'HELLO', 0, 'items_17')) - self.assertFalse(uwsgi.cache_set('key1', 'HELLO', 0, 'items_17')) + self.assertIsNone(uwsgi.cache_set('key1', 'HELLO', 0, 'items_17')) self.assertTrue(uwsgi.cache_del('key1', 'items_17')) self.assertTrue(uwsgi.cache_set('key1', 'HELLO', 0, 'items_17')) - self.assertFalse(uwsgi.cache_set('key1', 'HELLO', 0, 'items_17')) + self.assertIsNone(uwsgi.cache_set('key1', 'HELLO', 0, 'items_17')) def test_too_much_items(self): self.assertTrue(uwsgi.cache_set('key1', 'HELLO', 0, 'items_4_10')) self.assertTrue(uwsgi.cache_set('key2', 'HELLO', 0, 'items_4_10')) self.assertTrue(uwsgi.cache_set('key3', 'HELLO', 0, 'items_4_10')) - self.assertFalse(uwsgi.cache_set('key4', 'HELLO', 0, 'items_4_10')) + self.assertTrue(uwsgi.cache_set('key4', 'HELLO', 0, 'items_4_10')) + self.assertIsNone(uwsgi.cache_set('key5', 'HELLO', 0, 'items_4_10')) + def test_big_delete(self): + self.assertTrue(uwsgi.cache_set('key1', 'X' * 50 , 0, 'items_4_10')) + self.assertTrue(uwsgi.cache_del('key1', 'items_4_10')) + self.assertTrue(uwsgi.cache_set('key1', 'HELLOHELLO', 0, 'items_4_10')) + self.assertTrue(uwsgi.cache_set('key2', 'HELLOHELLO', 0, 'items_4_10')) + self.assertTrue(uwsgi.cache_set('key3', 'HELLOHELLO', 0, 'items_4_10')) + self.assertTrue(uwsgi.cache_set('key4', 'HELLOHELLO', 0, 'items_4_10')) + self.assertIsNone(uwsgi.cache_set('key5', 'HELLOHELLO', 0, 'items_4_10')) + + def test_big_update(self): + self.assertTrue(uwsgi.cache_set('key1', 'X' * 40 , 0, 'items_4_10')) + self.assertTrue(uwsgi.cache_update('key1', 'X' * 10 , 0, 'items_4_10')) + self.assertTrue(uwsgi.cache_del('key1', 'items_4_10')) + self.assertIsNone(uwsgi.cache_update('key1', 'X' * 51 , 0, 'items_4_10')) + self.assertTrue(uwsgi.cache_update('key1', 'X' * 50 , 0, 'items_4_10')) + + def test_multi_clear(self): + for i in range(0, 100): + self.assertTrue(uwsgi.cache_clear('items_4_10')) + + def test_multi_delete(self): + for i in range(0, 100): + self.assertTrue(uwsgi.cache_set('key1', 'X' * 50 , 0, 'items_4_10')) + self.assertTrue(uwsgi.cache_del('key1', 'items_4_10')) + + for i in range(0, 100): + self.assertIsNone(uwsgi.cache_set('key1', 'X' * 51 , 0, 'items_4_10')) + self.assertIsNone(uwsgi.cache_del('key1', 'items_4_10')) + + for i in range(0, 100): + self.assertTrue(uwsgi.cache_set('key1', 'X' * 50 , 0, 'items_4_10')) + self.assertTrue(uwsgi.cache_del('key1', 'items_4_10')) + + def test_big_key(self): + self.assertTrue(uwsgi.cache_set('K' * 2048, 'X' * 50 , 0, 'items_4_10')) + self.assertIsNone(uwsgi.cache_set('K' * 2049, 'X' * 50 , 0, 'items_4_10')) + + def rand_blob(self, n=32): + return ''.join([random.choice(string.ascii_letters + string.digits) for n in range(n)]) + + def test_big_random(self): + blob = self.rand_blob(100000) + self.assertTrue(uwsgi.cache_set('KEY', blob, 0, 'items_1_100000')) + get_blob = uwsgi.cache_get('KEY', 'items_1_100000') + self.assertEqual(blob, get_blob) + self.assertTrue(uwsgi.cache_del('KEY', 'items_1_100000')) + self.assertIsNone(uwsgi.cache_set('KEY', 'X' * 100001, 0, 'items_1_100000')) + self.assertTrue(uwsgi.cache_set('KEY', 'X' * 10000, 0, 'items_1_100000')) + + def test_non_bitmap(self): + self.assertTrue(uwsgi.cache_set('KEY', 'X' * 20, 0, 'items_non_bitmap')) + self.assertTrue(uwsgi.cache_del('KEY', 'items_non_bitmap')) + self.assertIsNone(uwsgi.cache_set('KEY', 'X' * 21, 0, 'items_non_bitmap')) + self.assertTrue(uwsgi.cache_set('KEY', 'X' * 20, 0, 'items_non_bitmap')) unittest.main()