Cache: Always return staleness

In order for the LRU storage to correctly track the state in the
real storage, we always need to know stale-state of a returned
(or not returned) value.

The return type is now not an a pure enumeration, but a 16-bit
enumeration plus a 16-bit bitmask that can be used for conveying
more information.
This commit is contained in:
Johan Wikman
2016-12-21 13:14:59 +02:00
parent 2ea436a5c7
commit 97fcb94daa
7 changed files with 95 additions and 82 deletions

View File

@ -22,14 +22,23 @@
MXS_BEGIN_DECLS
typedef enum cache_result
typedef enum cache_result_bits
{
CACHE_RESULT_OK,
CACHE_RESULT_NOT_FOUND,
CACHE_RESULT_STALE,
CACHE_RESULT_OUT_OF_RESOURCES,
CACHE_RESULT_ERROR
} cache_result_t;
CACHE_RESULT_OK = 0x01,
CACHE_RESULT_NOT_FOUND = 0x02,
CACHE_RESULT_ERROR = 0x03,
CACHE_RESULT_OUT_OF_RESOURCES = 0x04,
CACHE_RESULT_STALE = 0x10000 /*< Possibly combined with OK and NOT_FOUND. */
} cache_result_bits_t;
typedef uint32_t cache_result_t;
#define CACHE_RESULT_IS_OK(result) (result & CACHE_RESULT_OK)
#define CACHE_RESULT_IS_NOT_FOUND(result) (result & CACHE_RESULT_NOT_FOUND)
#define CACHE_RESULT_IS_ERROR(result) (result & CACHE_RESULT_ERROR)
#define CACHE_RESULT_IS_OUT_OF_RESOURCES(result) (result & CACHE_RESULT_OUT_OF_RESOURCES)
#define CACHE_RESULT_IS_STALE(result) (result & CACHE_RESULT_STALE)
typedef enum cache_flags
{
@ -209,11 +218,10 @@ typedef struct cache_storage_api
* @param result Pointer to variable that after a successful return will
* point to a GWBUF.
*
* @return CACHE_RESULT_OK if item was found,
* CACHE_RESULT_STALE if CACHE_FLAGS_INCLUDE_STALE was specified in
* flags and the item was found but stale,
* CACHE_RESULT_NOT_FOUND if item was not found (which may be because
* the ttl was reached), or some other error code.
* @return CACHE_RESULT_OK if item was found, CACHE_RESULT_NOT_FOUND if
* item was not found or some other error code. In the OK an NOT_FOUND
* cased, the bit CACHE_RESULT_STALE is set if the item exists but the
* soft TTL has passed.
*/
cache_result_t (*getValue)(CACHE_STORAGE* storage,
const CACHE_KEY* key,

View File

@ -148,9 +148,9 @@ int CacheFilterSession::routeQuery(GWBUF* pPacket)
GWBUF* pResponse;
cache_result_t result = get_cached_response(pPacket, &pResponse);
switch (result)
if (CACHE_RESULT_IS_OK(result))
{
case CACHE_RESULT_STALE:
if (CACHE_RESULT_IS_STALE(result))
{
// The value was found, but it was stale. Now we need to
// figure out whether somebody else is already fetching it.
@ -179,17 +179,17 @@ int CacheFilterSession::routeQuery(GWBUF* pPacket)
fetch_from_server = false;
}
}
break;
case CACHE_RESULT_OK:
if (log_decisions())
else
{
MXS_NOTICE("Using fresh data from cache.");
if (log_decisions())
{
MXS_NOTICE("Using fresh data from cache.");
}
fetch_from_server = false;
}
fetch_from_server = false;
break;
default:
}
else
{
fetch_from_server = true;
}
@ -616,7 +616,7 @@ cache_result_t CacheFilterSession::get_cached_response(const GWBUF *pQuery, GWBU
{
cache_result_t result = m_pCache->get_key(m_zDefaultDb, pQuery, &m_key);
if (result == CACHE_RESULT_OK)
if (CACHE_RESULT_IS_OK(result))
{
uint32_t flags = CACHE_FLAGS_INCLUDE_STALE;
@ -647,13 +647,13 @@ void CacheFilterSession::store_result()
cache_result_t result = m_pCache->put_value(m_key, m_res.pData);
if (result != CACHE_RESULT_OK)
if (!CACHE_RESULT_IS_OK(result))
{
MXS_ERROR("Could not store cache item, deleting it.");
result = m_pCache->del_value(m_key);
if ((result != CACHE_RESULT_OK) || (result != CACHE_RESULT_NOT_FOUND))
if (!CACHE_RESULT_IS_OK(result) || !CACHE_RESULT_IS_NOT_FOUND(result))
{
MXS_ERROR("Could not delete cache item.");
}

View File

@ -67,7 +67,9 @@ cache_result_t LRUStorage::do_get_info(uint32_t what,
json_t* pstorage_info;
if (pstorage_->get_info(what, &pstorage_info) == CACHE_RESULT_OK)
cache_result_t result = pstorage_->get_info(what, &pstorage_info);
if (CACHE_RESULT_IS_OK(result))
{
json_object_set(*ppinfo, "real_storage", pstorage_info);
json_decref(pstorage_info);
@ -104,13 +106,13 @@ cache_result_t LRUStorage::do_put_value(const CACHE_KEY& key, const GWBUF* pvalu
result = get_new_node(key, pvalue, &i, &pnode);
}
if (result == CACHE_RESULT_OK)
if (CACHE_RESULT_IS_OK(result))
{
ss_dassert(pnode);
result = pstorage_->put_value(key, pvalue);
if (result == CACHE_RESULT_OK)
if (CACHE_RESULT_IS_OK(result))
{
if (existed)
{
@ -149,7 +151,7 @@ cache_result_t LRUStorage::do_del_value(const CACHE_KEY& key)
{
result = pstorage_->del_value(key);
if ((result == CACHE_RESULT_OK) || (result == CACHE_RESULT_NOT_FOUND))
if (CACHE_RESULT_IS_OK(result) || CACHE_RESULT_IS_NOT_FOUND(result))
{
// If it wasn't found, we'll assume it was because ttl has hit in.
++stats_.deletes;
@ -173,13 +175,13 @@ cache_result_t LRUStorage::do_get_head(CACHE_KEY* pKey, GWBUF** ppValue) const
// Since it's the head it's unlikely to have happened, but we need to loop to
// cater for the case that ttl has hit in.
while (phead_ && (result == CACHE_RESULT_NOT_FOUND))
while (phead_ && (CACHE_RESULT_IS_NOT_FOUND(result)))
{
ss_dassert(phead_->key());
result = do_get_value(*phead_->key(), CACHE_FLAGS_INCLUDE_STALE, ppValue);
}
if (result == CACHE_RESULT_OK)
if (CACHE_RESULT_IS_OK(result))
{
*pKey = *phead_->key();
}
@ -192,13 +194,13 @@ cache_result_t LRUStorage::do_get_tail(CACHE_KEY* pKey, GWBUF** ppValue) const
cache_result_t result = CACHE_RESULT_NOT_FOUND;
// We need to loop to cater for the case that ttl has hit in.
while (ptail_ && (result == CACHE_RESULT_NOT_FOUND))
while (ptail_ && CACHE_RESULT_IS_NOT_FOUND(result))
{
ss_dassert(ptail_->key());
result = peek_value(*ptail_->key(), CACHE_FLAGS_INCLUDE_STALE, ppValue);
}
if (result == CACHE_RESULT_OK)
if (CACHE_RESULT_IS_OK(result))
{
*pKey = *ptail_->key();
}
@ -232,7 +234,7 @@ cache_result_t LRUStorage::access_value(access_approach_t approach,
{
result = pstorage_->get_value(key, flags, ppvalue);
if ((result == CACHE_RESULT_OK) || (result == CACHE_RESULT_STALE))
if (CACHE_RESULT_IS_OK(result))
{
++stats_.hits;
@ -241,13 +243,15 @@ cache_result_t LRUStorage::access_value(access_approach_t approach,
move_to_head(i->second);
}
}
else if (result == CACHE_RESULT_NOT_FOUND)
else if (CACHE_RESULT_IS_NOT_FOUND(result))
{
++stats_.misses;
// We'll assume this is because ttl has hit in. We need to remove
// the node and the mapping.
free_node(i);
if (!CACHE_RESULT_IS_STALE(result))
{
// If it wasn't just stale we'll remove it.
free_node(i);
}
}
}
else
@ -348,12 +352,14 @@ bool LRUStorage::free_node_data(Node* pnode)
cache_result_t result = pstorage_->del_value(*pkey);
switch (result)
if (CACHE_RESULT_IS_OK(result) || CACHE_RESULT_IS_NOT_FOUND(result))
{
case CACHE_RESULT_NOT_FOUND:
ss_dassert(!true);
MXS_ERROR("Item in LRU list was not found in storage.");
case CACHE_RESULT_OK:
if (CACHE_RESULT_IS_NOT_FOUND(result))
{
ss_dassert(!true);
MXS_ERROR("Item in LRU list was not found in storage.");
}
if (i != nodes_by_key_.end())
{
nodes_by_key_.erase(i);
@ -365,9 +371,9 @@ bool LRUStorage::free_node_data(Node* pnode)
stats_.size -= pnode->size();
stats_.items -= 1;
stats_.evictions += 1;
break;
default:
}
else
{
ss_dassert(!true);
MXS_ERROR("Could not remove value from storage, cannot "
"remove from LRU list or key mapping either.");
@ -472,8 +478,9 @@ cache_result_t LRUStorage::get_existing_node(NodesByKey::iterator& i, const GWBU
result = do_del_value(*pkey);
if (result != CACHE_RESULT_ERROR)
if (!CACHE_RESULT_IS_ERROR(result))
{
// If we failed to remove the value, we do not have enough space.
result = CACHE_RESULT_OUT_OF_RESOURCES;
}
}
@ -573,7 +580,7 @@ cache_result_t LRUStorage::get_new_node(const CACHE_KEY& key,
}
}
if (result == CACHE_RESULT_OK)
if (CACHE_RESULT_IS_OK(result))
{
ss_dassert(pnode);
*ppnode = pnode;

View File

@ -65,11 +65,10 @@ public:
* @param ppValue Pointer to variable that after a successful return will
* point to a GWBUF.
*
* @return CACHE_RESULT_OK if item was found,
* CACHE_RESULT_STALE if CACHE_FLAGS_INCLUDE_STALE was specified in
* flags and the item was found but stale,
* CACHE_RESULT_NOT_FOUND if item was not found (which may be because
* the ttl was reached), or some other error code.
* @return CACHE_RESULT_OK if item was found, CACHE_RESULT_NOT_FOUND if
* item was not found or some other error code. In the OK an NOT_FOUND
* cased, the bit CACHE_RESULT_STALE is set if the item exists but the
* soft TTL has passed.
*/
virtual cache_result_t get_value(const CACHE_KEY& key,
uint32_t flags,

View File

@ -209,8 +209,9 @@ cache_result_t InMemoryStorage::do_get_value(const CACHE_KEY& key, uint32_t flag
uint32_t now = time(NULL);
bool is_stale = m_config.ttl == 0 ? false : (now - entry.time > m_config.ttl);
bool include_stale = ((flags & CACHE_FLAGS_INCLUDE_STALE) != 0);
if (!is_stale || ((flags & CACHE_FLAGS_INCLUDE_STALE) != 0))
if (!is_stale || include_stale)
{
size_t length = entry.value.size();
@ -220,13 +221,11 @@ cache_result_t InMemoryStorage::do_get_value(const CACHE_KEY& key, uint32_t flag
{
memcpy(GWBUF_DATA(*ppResult), entry.value.data(), length);
result = CACHE_RESULT_OK;
if (is_stale)
{
result = CACHE_RESULT_STALE;
}
else
{
result = CACHE_RESULT_OK;
result |= CACHE_RESULT_STALE;
}
}
else
@ -236,7 +235,8 @@ cache_result_t InMemoryStorage::do_get_value(const CACHE_KEY& key, uint32_t flag
}
else
{
MXS_NOTICE("Cache item is stale, not using.");
ss_dassert(is_stale);
result |= CACHE_RESULT_STALE;
}
}
else

View File

@ -464,9 +464,10 @@ cache_result_t RocksDBStorage::get_value(const CACHE_KEY& key, uint32_t flags, G
case rocksdb::Status::kOk:
if (value.length() >= RocksDBInternals::TS_LENGTH)
{
bool isStale = RocksDBInternals::IsStale(value, m_config.ttl, rocksdb::Env::Default());
bool is_stale = RocksDBInternals::IsStale(value, m_config.ttl, rocksdb::Env::Default());
bool include_stale = ((flags & CACHE_FLAGS_INCLUDE_STALE) != 0);
if (!isStale || ((flags & CACHE_FLAGS_INCLUDE_STALE) != 0))
if (!is_stale || include_stale)
{
size_t length = value.length() - RocksDBInternals::TS_LENGTH;
@ -476,20 +477,18 @@ cache_result_t RocksDBStorage::get_value(const CACHE_KEY& key, uint32_t flags, G
{
memcpy(GWBUF_DATA(*ppResult), value.data(), length);
if (isStale)
result = CACHE_RESULT_OK;
if (is_stale)
{
result = CACHE_RESULT_STALE;
}
else
{
result = CACHE_RESULT_OK;
result |= CACHE_RESULT_STALE;
}
}
}
else
{
MXS_NOTICE("Cache item is stale, not using.");
result = CACHE_RESULT_NOT_FOUND;
ss_dassert(is_stale);
result = (CACHE_RESULT_NOT_FOUND | CACHE_RESULT_STALE);
}
}
else

View File

@ -59,7 +59,7 @@ int TesterStorage::HitTask::run()
case STORAGE_PUT:
{
cache_result_t result = m_storage.put_value(cache_item.first, cache_item.second);
if (result == CACHE_RESULT_OK)
if (CACHE_RESULT_IS_OK(result))
{
++m_puts;
}
@ -76,7 +76,7 @@ int TesterStorage::HitTask::run()
GWBUF* pQuery;
cache_result_t result = m_storage.get_value(cache_item.first, 0, &pQuery);
if (result == CACHE_RESULT_OK)
if (CACHE_RESULT_IS_OK(result))
{
ss_dassert(GWBUF_LENGTH(pQuery) == GWBUF_LENGTH(cache_item.second));
ss_dassert(memcmp(GWBUF_DATA(pQuery), GWBUF_DATA(cache_item.second),
@ -85,7 +85,7 @@ int TesterStorage::HitTask::run()
gwbuf_free(pQuery);
++m_gets;
}
else if (result == CACHE_RESULT_NOT_FOUND)
else if (CACHE_RESULT_IS_NOT_FOUND(result))
{
++m_misses;
}
@ -101,11 +101,11 @@ int TesterStorage::HitTask::run()
{
cache_result_t result = m_storage.del_value(cache_item.first);
if (result == CACHE_RESULT_OK)
if (CACHE_RESULT_IS_OK(result))
{
++m_dels;
}
else if (result == CACHE_RESULT_NOT_FOUND)
else if (CACHE_RESULT_IS_NOT_FOUND(result))
{
++m_misses;
}
@ -323,7 +323,7 @@ int TesterStorage::test_ttl(const CacheItems& cache_items, Storage& storage)
cache_result_t result = storage.put_value(cache_item.first, cache_item.second);
if (result != CACHE_RESULT_OK)
if (!CACHE_RESULT_IS_OK(result))
{
out() << "Could not put item." << endl;
rv = EXIT_FAILURE;
@ -337,7 +337,7 @@ int TesterStorage::test_ttl(const CacheItems& cache_items, Storage& storage)
pValue = NULL;
result = storage.get_value(cache_item.first, 0, &pValue);
if (result != CACHE_RESULT_OK)
if (!CACHE_RESULT_IS_OK(result))
{
out() << "Did not get value withing ttl." << endl;
rv = EXIT_FAILURE;
@ -350,12 +350,12 @@ int TesterStorage::test_ttl(const CacheItems& cache_items, Storage& storage)
pValue = NULL;
result = storage.get_value(cache_item.first, CACHE_FLAGS_INCLUDE_STALE, &pValue);
if (result == CACHE_RESULT_OK)
if (CACHE_RESULT_IS_OK(result) && !CACHE_RESULT_IS_STALE(result))
{
out() << "Got value normally when accepting stale, although ttl has passed." << endl;
rv = EXIT_FAILURE;
}
else if (result != CACHE_RESULT_STALE)
else if (!CACHE_RESULT_IS_STALE(result))
{
out() << "Did not get expected stale value after ttl." << endl;
rv = EXIT_FAILURE;
@ -366,12 +366,12 @@ int TesterStorage::test_ttl(const CacheItems& cache_items, Storage& storage)
pValue = NULL;
result = storage.get_value(cache_item.first, 0, &pValue);
if (result == CACHE_RESULT_OK)
if (CACHE_RESULT_IS_OK(result))
{
out() << "Got value normally, although ttl has passed." << endl;
rv = EXIT_FAILURE;
}
else if (result != CACHE_RESULT_NOT_FOUND)
else if (!CACHE_RESULT_IS_NOT_FOUND(result))
{
out() << "Unexpected failure." << endl;
rv = EXIT_FAILURE;