This is an automated email from the ASF dual-hosted git repository. masaori pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push: new ebe9978bdb Fix cache.volume_N.stripes metric issue by cleaning up CacheProcessor::cacheInitialized (#10923) ebe9978bdb is described below commit ebe9978bdb8febbbf4b38bc425d12098aa5c0a87 Author: Masaori Koshiba <masa...@apache.org> AuthorDate: Fri Jan 19 09:08:39 2024 +0900 Fix cache.volume_N.stripes metric issue by cleaning up CacheProcessor::cacheInitialized (#10923) * Fix cache.volume_N.stripes metric issue by cleaning up CacheProcessor::cacheInitialized * Add AuTest --- src/iocore/cache/Cache.cc | 173 ++++++++++----------- .../cache/gold/storage_metrics_0_stdout.gold | 7 + .../cache/gold/storage_metrics_1_stdout.gold | 13 ++ tests/gold_tests/cache/storage-metrics.test.py | 83 ++++++++++ 4 files changed, 181 insertions(+), 95 deletions(-) diff --git a/src/iocore/cache/Cache.cc b/src/iocore/cache/Cache.cc index d05f76f5a5..e82a1b45df 100644 --- a/src/iocore/cache/Cache.cc +++ b/src/iocore/cache/Cache.cc @@ -593,9 +593,13 @@ CacheProcessor::diskInitialized() void CacheProcessor::cacheInitialized() { - int i; + if (theCache == nullptr) { + Dbg(dbg_ctl_cache_init, "theCache is nullptr"); + return; + } - if (theCache && (theCache->ready == CACHE_INITIALIZING)) { + if (theCache->ready == CACHE_INITIALIZING) { + Dbg(dbg_ctl_cache_init, "theCache is initializing"); return; } @@ -603,29 +607,19 @@ CacheProcessor::cacheInitialized() int cache_init_ok = 0; /* allocate ram size in proportion to the disk space the volume occupies */ - int64_t total_size = 0; // count in HTTP & MIXT - uint64_t total_cache_bytes = 0; // bytes that can used in total_size - uint64_t total_direntries = 0; // all the direntries in the cache - uint64_t used_direntries = 0; // and used - uint64_t vol_total_cache_bytes = 0; - uint64_t vol_total_direntries = 0; - uint64_t vol_used_direntries = 0; - Stripe *stripe; - - if (theCache) { - total_size += theCache->cache_size; - Dbg(dbg_ctl_cache_init, "CacheProcessor::cacheInitialized - theCache, total_size = %" PRId64 " = %" PRId64 " MB", total_size, - total_size / ((1024 * 1024) / STORE_BLOCK_SIZE)); - if (theCache->ready == CACHE_INIT_FAILED) { - Dbg(dbg_ctl_cache_init, "CacheProcessor::cacheInitialized - failed to initialize the cache " - "for http: cache disabled"); - Warning("failed to initialize the cache for http: cache disabled\n"); - } else { - caches_ready = caches_ready | (1 << CACHE_FRAG_TYPE_HTTP); - caches_ready = caches_ready | (1 << CACHE_FRAG_TYPE_NONE); - caches[CACHE_FRAG_TYPE_HTTP] = theCache; - caches[CACHE_FRAG_TYPE_NONE] = theCache; - } + int64_t total_size = 0; // count in HTTP & MIXT + + total_size += theCache->cache_size; + Dbg(dbg_ctl_cache_init, "theCache, total_size = %" PRId64 " = %" PRId64 " MB", total_size, + total_size / ((1024 * 1024) / STORE_BLOCK_SIZE)); + if (theCache->ready == CACHE_INIT_FAILED) { + Dbg(dbg_ctl_cache_init, "failed to initialize the cache for http: cache disabled"); + Warning("failed to initialize the cache for http: cache disabled\n"); + } else { + caches_ready = caches_ready | (1 << CACHE_FRAG_TYPE_HTTP); + caches_ready = caches_ready | (1 << CACHE_FRAG_TYPE_NONE); + caches[CACHE_FRAG_TYPE_HTTP] = theCache; + caches[CACHE_FRAG_TYPE_NONE] = theCache; } // Update stripe version data. @@ -633,7 +627,7 @@ CacheProcessor::cacheInitialized() cacheProcessor.min_stripe_version = cacheProcessor.max_stripe_version = gstripes[0]->header->version; } // scan the rest of the stripes. - for (i = 1; i < gnstripes; i++) { + for (int i = 1; i < gnstripes; i++) { Stripe *v = gstripes[i]; if (v->header->version < cacheProcessor.min_stripe_version) { cacheProcessor.min_stripe_version = v->header->version; @@ -647,11 +641,9 @@ CacheProcessor::cacheInitialized() Dbg(dbg_ctl_cache_init, "CacheProcessor::cacheInitialized - caches_ready=0x%0X, gnvol=%d", (unsigned int)caches_ready, gnstripes.load()); - int64_t ram_cache_bytes = 0; - if (gnstripes) { // new ram_caches, with algorithm from the config - for (i = 0; i < gnstripes; i++) { + for (int i = 0; i < gnstripes; i++) { switch (cache_config_ram_cache_algorithm) { default: case RAM_CACHE_ALGORITHM_CLFUS: @@ -662,88 +654,79 @@ CacheProcessor::cacheInitialized() break; } } - // let us calculate the Size - if (cache_config_ram_cache_size == AUTO_SIZE_RAM_CACHE) { - Dbg(dbg_ctl_cache_init, "CacheProcessor::cacheInitialized - cache_config_ram_cache_size == AUTO_SIZE_RAM_CACHE"); - for (i = 0; i < gnstripes; i++) { - stripe = gstripes[i]; - - if (gstripes[i]->cache_vol->ramcache_enabled) { - gstripes[i]->ram_cache->init(stripe->dirlen() * DEFAULT_RAM_CACHE_MULTIPLIER, stripe); - ram_cache_bytes += gstripes[i]->dirlen(); - Dbg(dbg_ctl_cache_init, "CacheProcessor::cacheInitialized - ram_cache_bytes = %" PRId64 " = %" PRId64 "Mb", - ram_cache_bytes, ram_cache_bytes / (1024 * 1024)); - Metrics::Gauge::increment(stripe->cache_vol->vol_rsb.ram_cache_bytes_total, gstripes[i]->dirlen()); - } - vol_total_cache_bytes = gstripes[i]->len - gstripes[i]->dirlen(); - total_cache_bytes += vol_total_cache_bytes; - Dbg(dbg_ctl_cache_init, "CacheProcessor::cacheInitialized - total_cache_bytes = %" PRId64 " = %" PRId64 "Mb", - total_cache_bytes, total_cache_bytes / (1024 * 1024)); - Metrics::Gauge::increment(stripe->cache_vol->vol_rsb.bytes_total, vol_total_cache_bytes); - - vol_total_direntries = gstripes[i]->buckets * gstripes[i]->segments * DIR_DEPTH; - total_direntries += vol_total_direntries; - Metrics::Gauge::increment(stripe->cache_vol->vol_rsb.direntries_total, vol_total_direntries); - - vol_used_direntries = dir_entries_used(gstripes[i]); - Metrics::Gauge::increment(stripe->cache_vol->vol_rsb.direntries_used, vol_used_direntries); - used_direntries += vol_used_direntries; - } + int64_t http_ram_cache_size = 0; + // let us calculate the Size + if (cache_config_ram_cache_size == AUTO_SIZE_RAM_CACHE) { + Dbg(dbg_ctl_cache_init, "cache_config_ram_cache_size == AUTO_SIZE_RAM_CACHE"); } else { // we got configured memory size // TODO, should we check the available system memories, or you will // OOM or swapout, that is not a good situation for the server - Dbg(dbg_ctl_cache_init, "CacheProcessor::cacheInitialized - %" PRId64 " != AUTO_SIZE_RAM_CACHE", - cache_config_ram_cache_size); - int64_t http_ram_cache_size = - (theCache) ? - static_cast<int64_t>((static_cast<double>(theCache->cache_size) / total_size) * cache_config_ram_cache_size) : - 0; - Dbg(dbg_ctl_cache_init, "CacheProcessor::cacheInitialized - http_ram_cache_size = %" PRId64 " = %" PRId64 "Mb", - http_ram_cache_size, http_ram_cache_size / (1024 * 1024)); + Dbg(dbg_ctl_cache_init, "%" PRId64 " != AUTO_SIZE_RAM_CACHE", cache_config_ram_cache_size); + http_ram_cache_size = + static_cast<int64_t>((static_cast<double>(theCache->cache_size) / total_size) * cache_config_ram_cache_size); + + Dbg(dbg_ctl_cache_init, "http_ram_cache_size = %" PRId64 " = %" PRId64 "Mb", http_ram_cache_size, + http_ram_cache_size / (1024 * 1024)); int64_t stream_ram_cache_size = cache_config_ram_cache_size - http_ram_cache_size; - Dbg(dbg_ctl_cache_init, "CacheProcessor::cacheInitialized - stream_ram_cache_size = %" PRId64 " = %" PRId64 "Mb", - stream_ram_cache_size, stream_ram_cache_size / (1024 * 1024)); + + Dbg(dbg_ctl_cache_init, "stream_ram_cache_size = %" PRId64 " = %" PRId64 "Mb", stream_ram_cache_size, + stream_ram_cache_size / (1024 * 1024)); // Dump some ram_cache size information in debug mode. Dbg(dbg_ctl_ram_cache, "config: size = %" PRId64 ", cutoff = %" PRId64 "", cache_config_ram_cache_size, cache_config_ram_cache_cutoff); + } + + uint64_t total_cache_bytes = 0; // bytes that can used in total_size + uint64_t total_direntries = 0; // all the direntries in the cache + uint64_t used_direntries = 0; // and used + uint64_t total_ram_cache_bytes = 0; + + for (int i = 0; i < gnstripes; i++) { + Stripe *stripe = gstripes[i]; + int64_t ram_cache_bytes = 0; + + if (stripe->cache_vol->ramcache_enabled) { + if (http_ram_cache_size == 0) { + // AUTO_SIZE_RAM_CACHE + ram_cache_bytes = stripe->dirlen() * DEFAULT_RAM_CACHE_MULTIPLIER; + } else { + ink_assert(stripe->cache != nullptr); - for (i = 0; i < gnstripes; i++) { - stripe = gstripes[i]; - double factor; - if (gstripes[i]->cache == theCache && gstripes[i]->cache_vol->ramcache_enabled) { - ink_assert(gstripes[i]->cache != nullptr); - factor = static_cast<double>(static_cast<int64_t>(gstripes[i]->len >> STORE_BLOCK_SHIFT)) / theCache->cache_size; - Dbg(dbg_ctl_cache_init, "CacheProcessor::cacheInitialized - factor = %f", factor); - gstripes[i]->ram_cache->init(static_cast<int64_t>(http_ram_cache_size * factor), stripe); - ram_cache_bytes += static_cast<int64_t>(http_ram_cache_size * factor); - Metrics::Gauge::increment(stripe->cache_vol->vol_rsb.ram_cache_bytes_total, - - static_cast<int64_t>(http_ram_cache_size * factor)); - } else if (gstripes[i]->cache_vol->ramcache_enabled) { - ink_release_assert(!"Unexpected non-HTTP cache volume"); + double factor = static_cast<double>(static_cast<int64_t>(stripe->len >> STORE_BLOCK_SHIFT)) / theCache->cache_size; + Dbg(dbg_ctl_cache_init, "factor = %f", factor); + + ram_cache_bytes = static_cast<int64_t>(http_ram_cache_size * factor); } + + stripe->ram_cache->init(ram_cache_bytes, stripe); + total_ram_cache_bytes += ram_cache_bytes; + Metrics::Gauge::increment(stripe->cache_vol->vol_rsb.ram_cache_bytes_total, ram_cache_bytes); + Dbg(dbg_ctl_cache_init, "CacheProcessor::cacheInitialized[%d] - ram_cache_bytes = %" PRId64 " = %" PRId64 "Mb", i, ram_cache_bytes, ram_cache_bytes / (1024 * 1024)); - vol_total_cache_bytes = gstripes[i]->len - gstripes[i]->dirlen(); - total_cache_bytes += vol_total_cache_bytes; - Metrics::Gauge::increment(stripe->cache_vol->vol_rsb.bytes_total, vol_total_cache_bytes); - Metrics::Gauge::increment(stripe->cache_vol->vol_rsb.stripes); - Dbg(dbg_ctl_cache_init, "CacheProcessor::cacheInitialized - total_cache_bytes = %" PRId64 " = %" PRId64 "Mb", - total_cache_bytes, total_cache_bytes / (1024 * 1024)); - - vol_total_direntries = gstripes[i]->buckets * gstripes[i]->segments * DIR_DEPTH; - total_direntries += vol_total_direntries; - Metrics::Gauge::increment(stripe->cache_vol->vol_rsb.direntries_total, vol_total_direntries); - - vol_used_direntries = dir_entries_used(gstripes[i]); - Metrics::Gauge::increment(stripe->cache_vol->vol_rsb.direntries_used, vol_used_direntries); - used_direntries += vol_used_direntries; } + + uint64_t vol_total_cache_bytes = stripe->len - stripe->dirlen(); + total_cache_bytes += vol_total_cache_bytes; + Metrics::Gauge::increment(stripe->cache_vol->vol_rsb.bytes_total, vol_total_cache_bytes); + Metrics::Gauge::increment(stripe->cache_vol->vol_rsb.stripes); + + Dbg(dbg_ctl_cache_init, "total_cache_bytes = %" PRId64 " = %" PRId64 "Mb", total_cache_bytes, + total_cache_bytes / (1024 * 1024)); + + uint64_t vol_total_direntries = stripe->buckets * stripe->segments * DIR_DEPTH; + total_direntries += vol_total_direntries; + Metrics::Gauge::increment(stripe->cache_vol->vol_rsb.direntries_total, vol_total_direntries); + + uint64_t vol_used_direntries = dir_entries_used(stripe); + Metrics::Gauge::increment(stripe->cache_vol->vol_rsb.direntries_used, vol_used_direntries); + used_direntries += vol_used_direntries; } + switch (cache_config_ram_cache_compress) { default: Fatal("unknown RAM cache compression type: %d", cache_config_ram_cache_compress); @@ -759,7 +742,7 @@ CacheProcessor::cacheInitialized() break; } - Metrics::Gauge::store(cache_rsb.ram_cache_bytes_total, ram_cache_bytes); + Metrics::Gauge::store(cache_rsb.ram_cache_bytes_total, total_ram_cache_bytes); Metrics::Gauge::store(cache_rsb.bytes_total, total_cache_bytes); Metrics::Gauge::store(cache_rsb.direntries_total, total_direntries); Metrics::Gauge::store(cache_rsb.direntries_used, used_direntries); diff --git a/tests/gold_tests/cache/gold/storage_metrics_0_stdout.gold b/tests/gold_tests/cache/gold/storage_metrics_0_stdout.gold new file mode 100644 index 0000000000..ad9880f3fe --- /dev/null +++ b/tests/gold_tests/cache/gold/storage_metrics_0_stdout.gold @@ -0,0 +1,7 @@ +`` +proxy.process.cache.stripes 1 +`` +proxy.process.cache.span.online 1 +`` +proxy.process.cache.volume_0.stripes 1 +`` diff --git a/tests/gold_tests/cache/gold/storage_metrics_1_stdout.gold b/tests/gold_tests/cache/gold/storage_metrics_1_stdout.gold new file mode 100644 index 0000000000..c8b83cfdd9 --- /dev/null +++ b/tests/gold_tests/cache/gold/storage_metrics_1_stdout.gold @@ -0,0 +1,13 @@ +`` +proxy.process.cache.stripes 4 +`` +proxy.process.cache.span.online 1 +`` +proxy.process.cache.volume_1.stripes 1 +`` +proxy.process.cache.volume_2.stripes 1 +`` +proxy.process.cache.volume_3.stripes 1 +`` +proxy.process.cache.volume_4.stripes 1 +`` diff --git a/tests/gold_tests/cache/storage-metrics.test.py b/tests/gold_tests/cache/storage-metrics.test.py new file mode 100644 index 0000000000..87c5451d35 --- /dev/null +++ b/tests/gold_tests/cache/storage-metrics.test.py @@ -0,0 +1,83 @@ +''' +''' +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os + +Test.Summary = 'Test loading storage metrics' +Test.ContinueOnFail = True + +# CAVEAT: Below test cases doesn't have multiple span cases that requires RAW devices. +test_cases = [ + { + "case": 0, + "description": "default config", + "storage": ''' +storage 256M +''', + "volume": ''' +# empty +''' + }, + { + "case": 1, + "description": "four equally devided volumes", + "storage": ''' +storage 1G +''', + "volume": + ''' +volume=1 scheme=http size=25% +volume=2 scheme=http size=25% +volume=3 scheme=http size=25% +volume=4 scheme=http size=25% +''' + }, +] + + +class StorageMetricsTest: + """ + Test loading storage.config and volume.config + + 1. Spawn TS process with configs in test_cases + 2. Get 'proxy.process.cache.*' metrics + 3. Check with 'gold/storage_N_stdout.gold' file + """ + + def run(self): + for config in test_cases: + i = config["case"] + ts = Test.MakeATSProcess(f"ts_{i}") + ts.Disk.storage_config.AddLine(config["storage"]) + ts.Disk.volume_config.AddLine(config["volume"]) + ts.Disk.records_config.update({ + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'cache', + }) + + tr = Test.AddTestRun() + + tr.Processes.Default.StartBefore(ts) + tr.Processes.Default.Env = ts.Env + tr.Processes.Default.Command = 'traffic_ctl --debug metric match proxy.process.cache.' + tr.Processes.Default.ReturnCode = 0 + tr.Processes.Default.Streams.stdout = f"gold/storage_metrics_{i}_stdout.gold" + tr.StillRunningAfter = ts + + +StorageMetricsTest().run()