This is an automated email from the ASF dual-hosted git repository. zwoop pushed a commit to branch 9.2.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 158e25eb3580792c0b43e84d2d843bd88fa6b17c Author: Robert O Butts <[email protected]> AuthorDate: Thu Oct 20 13:51:17 2022 -0600 Fix hosting.config reloading (#9046) (cherry picked from commit 026d906c362b7edce4751bdc59bf6660c67d7f67) --- iocore/cache/Cache.cc | 40 +++++++----- iocore/cache/CacheHosting.cc | 2 +- iocore/cache/CacheVol.cc | 8 ++- iocore/cache/P_CacheHosting.h | 134 +++++++++++++++++++++++++++++++++++++++-- iocore/cache/P_CacheInternal.h | 4 +- 5 files changed, 163 insertions(+), 25 deletions(-) diff --git a/iocore/cache/Cache.cc b/iocore/cache/Cache.cc index 797c08d77..652984ab6 100644 --- a/iocore/cache/Cache.cc +++ b/iocore/cache/Cache.cc @@ -1975,13 +1975,16 @@ CacheProcessor::mark_storage_offline(CacheDisk *d, ///< Target disk Warning("All storage devices offline, cache disabled"); CacheProcessor::cache_ready = 0; } else { // check cache types specifically - if (theCache && !theCache->hosttable->gen_host_rec.vol_hash_table) { - unsigned int caches_ready = 0; - caches_ready = caches_ready | (1 << CACHE_FRAG_TYPE_HTTP); - caches_ready = caches_ready | (1 << CACHE_FRAG_TYPE_NONE); - caches_ready = ~caches_ready; - CacheProcessor::cache_ready &= caches_ready; - Warning("all volumes for http cache are corrupt, http cache disabled"); + if (theCache) { + ReplaceablePtr<CacheHostTable>::ScopedReader hosttable(&theCache->hosttable); + if (!hosttable->gen_host_rec.vol_hash_table) { + unsigned int caches_ready = 0; + caches_ready = caches_ready | (1 << CACHE_FRAG_TYPE_HTTP); + caches_ready = caches_ready | (1 << CACHE_FRAG_TYPE_NONE); + caches_ready = ~caches_ready; + CacheProcessor::cache_ready &= caches_ready; + Warning("all volumes for http cache are corrupt, http cache disabled"); + } } } @@ -2050,9 +2053,13 @@ Cache::open_done() return 0; } - hosttable = new CacheHostTable(this, scheme); - hosttable->register_config_callback(&hosttable); + { + CacheHostTable *hosttable_raw = new CacheHostTable(this, scheme); + hosttable.reset(hosttable_raw); + hosttable_raw->register_config_callback(&hosttable); + } + ReplaceablePtr<CacheHostTable>::ScopedReader hosttable(&this->hosttable); if (hosttable->gen_host_rec.num_cachevols == 0) { ready = CACHE_INIT_FAILED; } else { @@ -3006,9 +3013,10 @@ create_volume(int volume_number, off_t size_in_blocks, int scheme, CacheVol *cp) void rebuild_host_table(Cache *cache) { - build_vol_hash_table(&cache->hosttable->gen_host_rec); - if (cache->hosttable->m_numEntries != 0) { - CacheHostMatcher *hm = cache->hosttable->getHostMatcher(); + ReplaceablePtr<CacheHostTable>::ScopedWriter hosttable(&cache->hosttable); + build_vol_hash_table(&hosttable->gen_host_rec); + if (hosttable->m_numEntries != 0) { + CacheHostMatcher *hm = hosttable->getHostMatcher(); CacheHostRecord *h_rec = hm->getDataArray(); int h_rec_len = hm->getNumElements(); int i; @@ -3022,9 +3030,11 @@ rebuild_host_table(Cache *cache) Vol * Cache::key_to_vol(const CacheKey *key, const char *hostname, int host_len) { - uint32_t h = (key->slice32(2) >> DIR_TAG_WIDTH) % VOL_HASH_TABLE_SIZE; - unsigned short *hash_table = hosttable->gen_host_rec.vol_hash_table; - CacheHostRecord *host_rec = &hosttable->gen_host_rec; + ReplaceablePtr<CacheHostTable>::ScopedReader hosttable(&this->hosttable); + + uint32_t h = (key->slice32(2) >> DIR_TAG_WIDTH) % VOL_HASH_TABLE_SIZE; + unsigned short *hash_table = hosttable->gen_host_rec.vol_hash_table; + const CacheHostRecord *host_rec = &hosttable->gen_host_rec; if (hosttable->m_numEntries > 0 && host_len) { CacheHostResult res; diff --git a/iocore/cache/CacheHosting.cc b/iocore/cache/CacheHosting.cc index 36e8afd59..78f6018d3 100644 --- a/iocore/cache/CacheHosting.cc +++ b/iocore/cache/CacheHosting.cc @@ -229,7 +229,7 @@ int CacheHostTable::config_callback(const char * /* name ATS_UNUSED */, RecDataT /* data_type ATS_UNUSED */, RecData /* data ATS_UNUSED */, void *cookie) { - CacheHostTable **ppt = static_cast<CacheHostTable **>(cookie); + ReplaceablePtr<CacheHostTable> *ppt = static_cast<ReplaceablePtr<CacheHostTable> *>(cookie); eventProcessor.schedule_imm(new CacheHostTableConfig(ppt)); return 0; } diff --git a/iocore/cache/CacheVol.cc b/iocore/cache/CacheVol.cc index bf0a68ed3..9d45c8800 100644 --- a/iocore/cache/CacheVol.cc +++ b/iocore/cache/CacheVol.cc @@ -57,14 +57,18 @@ CacheVC::scanVol(int /* event ATS_UNUSED */, Event * /* e ATS_UNUSED */) if (_action.cancelled) { return free_CacheVC(this); } - CacheHostRecord *rec = &theCache->hosttable->gen_host_rec; + + ReplaceablePtr<CacheHostTable>::ScopedReader hosttable(&theCache->hosttable); + + const CacheHostRecord *rec = &hosttable->gen_host_rec; if (host_len) { CacheHostResult res; - theCache->hosttable->Match(hostname, host_len, &res); + hosttable->Match(hostname, host_len, &res); if (res.record) { rec = res.record; } } + if (!vol) { if (!rec->num_vols) { goto Ldone; diff --git a/iocore/cache/P_CacheHosting.h b/iocore/cache/P_CacheHosting.h index 62e46c120..6fe4887b4 100644 --- a/iocore/cache/P_CacheHosting.h +++ b/iocore/cache/P_CacheHosting.h @@ -22,6 +22,8 @@ */ #pragma once +#include <memory> +#include <shared_mutex> #include "P_Cache.h" #include "tscore/MatcherUtils.h" #include "tscore/HostLookup.h" @@ -105,6 +107,113 @@ private: CacheType type; }; +// ReplaceablePtr provides threadsafe access to an object which may be replaced. +// +// Access is provided via ScopedReader and ScopedWriter classes, which acquire +// shared (read) and exclusive (write) locks respectively on construction and +// release them upon destruction. +// +// The underlying object may be replaced by a concurrent thread via 'reset', +// which acquires an exclusive lock before setting the internal pointer. This +// takes ownership of the given pointer. If this already has ownership of +// another pointer, that object is destroyed and its memory freed. +// +// Direct access without acquiring the lock is intentionally not provided, to +// prevent accidental usage without locking, or forgetting to release the lock. +// +// This may not be copied. To use, construct with one owner, and pass a pointer +// to other users. +// +template <typename T> class ReplaceablePtr +{ +public: + ReplaceablePtr() {} + virtual ~ReplaceablePtr() {} + + // reset acquires an exclusive (write) lock and updates the internal pointer + // with t. + // If an existing pointer is owned, the object is destructed and its memory + // freed. + void + reset(T *t) + { + std::scoped_lock l(m); + h.reset(t); + } + + // ScopedReader constructs an object which is allowed to read from a + // ReplaceablePtr. + // + // The lifetime of the ReplaceablePtr must exceed the lifetime of this. + // The shared (read) lock is immediately acquired upon construction of this, + // and released upon destruction. + class ScopedReader + { + public: + ScopedReader(ReplaceablePtr<T> *ptr) : ptr(ptr) { ptr->m.lock_shared(); } + ~ScopedReader() { ptr->m.unlock_shared(); } + + const T * + operator->() + { + return ptr->h.get(); + } + + const T * + get() + { + return ptr->h.get(); + } + + private: + ScopedReader(const ScopedReader &) = delete; + ScopedReader &operator=(const ScopedReader &) = delete; + + ReplaceablePtr<T> *ptr; + }; + + // ScopedWriter constructs an object which is allowed to read and modify the + // object pointed to by a ReplaceablePtr. + // + // The lifetime of the ReplaceablePtr must exceed the lifetime of this. + // + // An exclusive (write) lock is immediately acquired upon construction of + // this, and released upon destruction. + class ScopedWriter + { + public: + ScopedWriter(ReplaceablePtr<T> *ptr) : ptr(ptr) { ptr->m.lock(); } + ~ScopedWriter() { ptr->m.unlock(); } + + T * + operator->() + { + return ptr->h.get(); + } + + T * + get() + { + return ptr->h.get(); + } + + private: + ScopedWriter(const ScopedWriter &) = delete; + ScopedWriter &operator=(const ScopedWriter &) = delete; + + ReplaceablePtr<T> *ptr; + }; + +private: + ReplaceablePtr(const ReplaceablePtr &) = delete; + ReplaceablePtr &operator=(const ReplaceablePtr &) = delete; + + std::unique_ptr<T> h = nullptr; + std::shared_mutex m; + + friend class ReplaceablePtr::ScopedReader; +}; + class CacheHostTable { public: @@ -133,7 +242,7 @@ public: static int config_callback(const char *, RecDataT, RecData, void *); void - register_config_callback(CacheHostTable **p) + register_config_callback(ReplaceablePtr<CacheHostTable> *p) { REC_RegisterConfigUpdateFunc("proxy.config.cache.hosting_filename", CacheHostTable::config_callback, (void *)p); } @@ -152,20 +261,33 @@ private: struct CacheHostTableConfig; typedef int (CacheHostTableConfig::*CacheHostTabHandler)(int, void *); struct CacheHostTableConfig : public Continuation { - CacheHostTable **ppt; - CacheHostTableConfig(CacheHostTable **appt) : Continuation(nullptr), ppt(appt) { SET_HANDLER(&CacheHostTableConfig::mainEvent); } + CacheHostTableConfig(ReplaceablePtr<CacheHostTable> *appt) : Continuation(nullptr), ppt(appt) + { + SET_HANDLER(&CacheHostTableConfig::mainEvent); + } + + ~CacheHostTableConfig() {} int mainEvent(int event, Event *e) { (void)e; (void)event; - CacheHostTable *t = new CacheHostTable((*ppt)->cache, (*ppt)->type); - CacheHostTable *old = (CacheHostTable *)ink_atomic_swap(&t, *ppt); - new_Deleter(old, CACHE_MEM_FREE_TIMEOUT); + + CacheType type = CACHE_HTTP_TYPE; + Cache *cache = nullptr; + { + ReplaceablePtr<CacheHostTable>::ScopedReader hosttable(ppt); + type = hosttable->type; + cache = hosttable->cache; + } + ppt->reset(new CacheHostTable(cache, type)); delete this; return EVENT_DONE; } + +private: + ReplaceablePtr<CacheHostTable> *ppt; }; /* list of volumes in the volume.config file */ diff --git a/iocore/cache/P_CacheInternal.h b/iocore/cache/P_CacheInternal.h index 2d7a8cb84..a13aaa2a9 100644 --- a/iocore/cache/P_CacheInternal.h +++ b/iocore/cache/P_CacheInternal.h @@ -28,6 +28,7 @@ #include "HTTP.h" #include "P_CacheHttp.h" +#include "P_CacheHosting.h" struct EvacuationBlock; @@ -987,10 +988,11 @@ struct Cache { int total_nvol = 0; int ready = CACHE_INITIALIZING; int64_t cache_size = 0; // in store block size - CacheHostTable *hosttable = nullptr; int total_initialized_vol = 0; CacheType scheme = CACHE_NONE_TYPE; + ReplaceablePtr<CacheHostTable> hosttable; + int open(bool reconfigure, bool fix); int close();
