From efeaecaef2e1d6c54ffcfb2493535b85da635779 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Tue, 24 Oct 2017 09:41:23 +0300 Subject: [PATCH] MXS-1486 When there is fresh data, update the cache entry If something is SELECTed that should be cached for some, but not for the current user, the cached entry it nevertheless updated. That way the cached data will always be the last fetched value and it is also possible to use this behaviour for explicitly updating the cache entry. --- .../filter/cache/cachefiltersession.cc | 135 +++++++++--------- .../filter/cache/cachefiltersession.hh | 2 - 2 files changed, 66 insertions(+), 71 deletions(-) diff --git a/server/modules/filter/cache/cachefiltersession.cc b/server/modules/filter/cache/cachefiltersession.cc index 1ec9a7deb..fe345396c 100644 --- a/server/modules/filter/cache/cachefiltersession.cc +++ b/server/modules/filter/cache/cachefiltersession.cc @@ -291,73 +291,95 @@ int CacheFilterSession::routeQuery(GWBUF* pPacket) { if (m_pCache->should_store(m_zDefaultDb, pPacket)) { - if (m_pCache->should_use(m_pSession)) + cache_result_t result = m_pCache->get_key(m_zDefaultDb, pPacket, &m_key); + + if (CACHE_RESULT_IS_OK(result)) { - GWBUF* pResponse; - cache_result_t result = get_cached_response(pPacket, &pResponse); - - if (CACHE_RESULT_IS_OK(result)) + if (m_pCache->should_use(m_pSession)) { - if (CACHE_RESULT_IS_STALE(result)) + uint32_t flags = CACHE_FLAGS_INCLUDE_STALE; + GWBUF* pResponse; + result = m_pCache->get_value(m_key, flags, &pResponse); + + if (CACHE_RESULT_IS_OK(result)) { - // The value was found, but it was stale. Now we need to - // figure out whether somebody else is already fetching it. - - if (m_pCache->must_refresh(m_key, this)) + if (CACHE_RESULT_IS_STALE(result)) { - // We were the first ones who hit the stale item. It's - // our responsibility now to fetch it. - if (log_decisions()) + // The value was found, but it was stale. Now we need to + // figure out whether somebody else is already fetching it. + + if (m_pCache->must_refresh(m_key, this)) { - MXS_NOTICE("Cache data is stale, fetching fresh from server."); + // We were the first ones who hit the stale item. It's + // our responsibility now to fetch it. + if (log_decisions()) + { + MXS_NOTICE("Cache data is stale, fetching fresh from server."); + } + + // As we don't use the response it must be freed. + gwbuf_free(pResponse); + + m_refreshing = true; + fetch_from_server = true; + } + else + { + // Somebody is already fetching the new value. So, let's + // use the stale value. No point in hitting the server twice. + if (log_decisions()) + { + MXS_NOTICE("Cache data is stale but returning it, fresh " + "data is being fetched already."); + } + fetch_from_server = false; } - - // As we don't use the response it must be freed. - gwbuf_free(pResponse); - - m_refreshing = true; - fetch_from_server = true; } else { - // Somebody is already fetching the new value. So, let's - // use the stale value. No point in hitting the server twice. if (log_decisions()) { - MXS_NOTICE("Cache data is stale but returning it, fresh " - "data is being fetched already."); + MXS_NOTICE("Using fresh data from cache."); } fetch_from_server = false; } } else { - if (log_decisions()) - { - MXS_NOTICE("Using fresh data from cache."); - } - fetch_from_server = false; + fetch_from_server = true; + } + + if (fetch_from_server) + { + m_state = CACHE_EXPECTING_RESPONSE; + } + else + { + m_state = CACHE_EXPECTING_NOTHING; + gwbuf_free(pPacket); + DCB *dcb = m_pSession->client_dcb; + + // TODO: This is not ok. Any filters before this filter, will not + // TODO: see this data. + rv = dcb->func.write(dcb, pResponse); } } else { - fetch_from_server = true; - } - - if (fetch_from_server) - { + // We will not use any value in the cache, but we will update + // the existing value. + if (log_decisions()) + { + MXS_NOTICE("Unconditionally fetching data from the server, " + "refreshing cache entry."); + } m_state = CACHE_EXPECTING_RESPONSE; } - else - { - m_state = CACHE_EXPECTING_NOTHING; - gwbuf_free(pPacket); - DCB *dcb = m_pSession->client_dcb; - - // TODO: This is not ok. Any filters before this filter, will not - // TODO: see this data. - rv = dcb->func.write(dcb, pResponse); - } + } + else + { + MXS_ERROR("Could not create cache key."); + m_state = CACHE_IGNORING_RESPONSE; } } else @@ -775,31 +797,6 @@ void CacheFilterSession::reset_response_state() m_res.offset = 0; } -/** - * Route a query via the cache. - * - * @param key A SELECT packet. - * @param value The result. - * @return True if the query was satisfied from the query. - */ -cache_result_t CacheFilterSession::get_cached_response(const GWBUF *pQuery, GWBUF **ppResponse) -{ - cache_result_t result = m_pCache->get_key(m_zDefaultDb, pQuery, &m_key); - - if (CACHE_RESULT_IS_OK(result)) - { - uint32_t flags = CACHE_FLAGS_INCLUDE_STALE; - - result = m_pCache->get_value(m_key, flags, ppResponse); - } - else - { - MXS_ERROR("Could not create cache key."); - } - - return result; -} - /** * Store the data. * diff --git a/server/modules/filter/cache/cachefiltersession.hh b/server/modules/filter/cache/cachefiltersession.hh index d1e51985b..e19d309be 100644 --- a/server/modules/filter/cache/cachefiltersession.hh +++ b/server/modules/filter/cache/cachefiltersession.hh @@ -102,8 +102,6 @@ private: void reset_response_state(); - cache_result_t get_cached_response(const GWBUF *pQuery, GWBUF **ppResponse); - bool log_decisions() const { return m_pCache->config().debug & CACHE_DEBUG_DECISIONS ? true : false;