From ca30b335154bec9c0f7f7c25a07182557dbb257d Mon Sep 17 00:00:00 2001 From: Serris Lew Date: Mon, 16 May 2022 12:08:43 -0700 Subject: [PATCH 1/9] Update slice to only prefetch when first block is miss/hit-stale --- doc/admin-guide/plugins/slice.en.rst | 6 +- .../cache_range_requests.cc | 2 + plugins/experimental/slice/Config.h | 2 + plugins/experimental/slice/Data.h | 2 +- plugins/experimental/slice/intercept.cc | 11 ++- plugins/experimental/slice/prefetch.cc | 1 - plugins/experimental/slice/slice.cc | 6 ++ plugins/experimental/slice/util.cc | 21 +++-- .../pluginTest/slice/gold/slice_prefetch.gold | 27 ++++++ .../pluginTest/slice/slice_prefetch.test.py | 92 +++++++++++-------- 10 files changed, 117 insertions(+), 53 deletions(-) create mode 100644 tests/gold_tests/pluginTest/slice/gold/slice_prefetch.gold diff --git a/doc/admin-guide/plugins/slice.en.rst b/doc/admin-guide/plugins/slice.en.rst index 5a22bf1d3ee..3582581628f 100644 --- a/doc/admin-guide/plugins/slice.en.rst +++ b/doc/admin-guide/plugins/slice.en.rst @@ -129,8 +129,10 @@ The slice plugin supports the following options:: --prefetch-count= (optional) Default is 0 Prefetches successive 'n' slice block requests in the background - and cached. Especially for large objects, prefetching can improve - cache miss latency. + and caches (with `cache_range_requests` plugin). If the first + block is a miss or hit-stale, it will prefetch. Otherwise, + prefetching is disabled. Especially for large objects, + prefetching can improve cache miss latency. -f for short Examples:: diff --git a/plugins/cache_range_requests/cache_range_requests.cc b/plugins/cache_range_requests/cache_range_requests.cc index 7c3b708b869..48c5a459760 100644 --- a/plugins/cache_range_requests/cache_range_requests.cc +++ b/plugins/cache_range_requests/cache_range_requests.cc @@ -370,9 +370,11 @@ handle_client_send_response(TSHttpTxn txnp, txndata *const txn_state) DEBUG_LOG("Restoring response header to TS_HTTP_STATUS_PARTIAL_CONTENT."); TSHttpHdrStatusSet(resp_buf, resp_loc, TS_HTTP_STATUS_PARTIAL_CONTENT); } + } else { DEBUG_LOG("Ignoring status code %d; txn_state->origin_status=%d", status, txn_state->origin_status); } + TSHandleMLocRelease(resp_buf, TS_NULL_MLOC, resp_loc); } diff --git a/plugins/experimental/slice/Config.h b/plugins/experimental/slice/Config.h index 8408ffc2076..d7ef1d23019 100644 --- a/plugins/experimental/slice/Config.h +++ b/plugins/experimental/slice/Config.h @@ -49,6 +49,8 @@ struct Config { std::string m_skip_header; std::string m_crr_ims_header; + TSCont m_contp{nullptr}; + bool m_first_block{true}; // Convert optarg to bytes static int64_t bytesFrom(char const *const valstr); diff --git a/plugins/experimental/slice/Data.h b/plugins/experimental/slice/Data.h index 9cfbb2f7ead..8e654a69bc4 100644 --- a/plugins/experimental/slice/Data.h +++ b/plugins/experimental/slice/Data.h @@ -93,7 +93,7 @@ struct Data { Stage m_upstream; Stage m_dnstream; - std::unordered_map m_fetchstates; + bool m_prefetchable{false}; HdrMgr m_req_hdrmgr; // manager for server request HdrMgr m_resp_hdrmgr; // manager for client response diff --git a/plugins/experimental/slice/intercept.cc b/plugins/experimental/slice/intercept.cc index 2b6ef3bdfcd..e3094aebdf9 100644 --- a/plugins/experimental/slice/intercept.cc +++ b/plugins/experimental/slice/intercept.cc @@ -50,7 +50,16 @@ intercept_hook(TSCont contp, TSEvent event, void *edata) case TS_EVENT_ERROR: { abort(contp, data); } break; - + case TS_EVENT_HTTP_READ_RESPONSE_HDR: { + TSHttpTxn const txnp = static_cast(edata); + int lookupStatus; + if (data->m_blocknum == 0 && TS_SUCCESS == TSHttpTxnCacheLookupStatusGet(txnp, &lookupStatus)) { + if (lookupStatus == TS_CACHE_LOOKUP_MISS || lookupStatus == TS_CACHE_LOOKUP_HIT_STALE) { + data->m_prefetchable = true; + } + } + TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE); + } break; default: { // data from client -- only the initial header if (data->m_dnstream.m_read.isOpen() && edata == data->m_dnstream.m_read.m_vio) { diff --git a/plugins/experimental/slice/prefetch.cc b/plugins/experimental/slice/prefetch.cc index 2325db77f4b..8fba88b738a 100644 --- a/plugins/experimental/slice/prefetch.cc +++ b/plugins/experimental/slice/prefetch.cc @@ -97,7 +97,6 @@ BgBlockFetch::fetch(Data *const data) DEBUG_LOG("Headers\n%s", headerstr.c_str()); } - data->m_fetchstates[_blocknum] = true; return true; } diff --git a/plugins/experimental/slice/slice.cc b/plugins/experimental/slice/slice.cc index abb1630fc1d..8f1f4096c80 100644 --- a/plugins/experimental/slice/slice.cc +++ b/plugins/experimental/slice/slice.cc @@ -180,10 +180,16 @@ read_request(TSHttpTxn txnp, Config *const config) // Grab the transaction TSHttpTxnIntercept(icontp, txnp); + config->m_contp = icontp; + config->m_first_block = true; return true; } else { DEBUG_LOG("slice passing GET request through to next plugin"); + if (config->m_prefetchcount > 0 && config->m_first_block) { + TSHttpTxnHookAdd(txnp, TS_HTTP_READ_RESPONSE_HDR_HOOK, config->m_contp); + config->m_first_block = false; + } } } diff --git a/plugins/experimental/slice/util.cc b/plugins/experimental/slice/util.cc index 7e4bc1148ec..18b00b65233 100644 --- a/plugins/experimental/slice/util.cc +++ b/plugins/experimental/slice/util.cc @@ -112,16 +112,19 @@ request_block(TSCont contp, Data *const data) } // if prefetch config set, schedule next block requests in background - for (int i = 0; i < data->m_config->m_prefetchcount; i++) { - int nextblocknum = data->m_blocknum + i + 1; - if (data->m_req_range.blockIsInside(data->m_config->m_blockbytes, nextblocknum) && !data->m_fetchstates[nextblocknum]) { - if (BgBlockFetch::schedule(data, nextblocknum)) { - DEBUG_LOG("Background fetch requested"); - } else { - DEBUG_LOG("Background fetch not requested"); + if (data->m_prefetchable && data->m_config->m_prefetchcount > 0) { + int nextblocknum = 2; + if (data->m_blocknum > 1) { + nextblocknum = data->m_blocknum + data->m_config->m_prefetchcount; + } + for (int i = nextblocknum; i <= data->m_blocknum + data->m_config->m_prefetchcount; i++) { + if (data->m_req_range.blockIsInside(data->m_config->m_blockbytes, i)) { + if (BgBlockFetch::schedule(data, i)) { + DEBUG_LOG("Background fetch requested"); + } else { + DEBUG_LOG("Background fetch not requested"); + } } - } else { - break; } } // get ready for data back from the server diff --git a/tests/gold_tests/pluginTest/slice/gold/slice_prefetch.gold b/tests/gold_tests/pluginTest/slice/gold/slice_prefetch.gold new file mode 100644 index 00000000000..173fac042a6 --- /dev/null +++ b/tests/gold_tests/pluginTest/slice/gold/slice_prefetch.gold @@ -0,0 +1,27 @@ +bytes 0-6/18 miss +bytes ``/18 miss +bytes ``/18 miss +bytes 14-17/18 hit-fresh +- miss, none +bytes 0-6/18 hit-fresh +bytes 7-13/18 hit-fresh +bytes 14-17/18 hit-fresh +- hit-fresh, none +bytes 0-6/18 hit-stale +bytes ``/18 hit-stale +bytes ``/18 hit-stale +bytes 14-17/18 hit-fresh +- hit-stale, none +bytes 0-4/18 miss +bytes ``/18 miss +bytes ``/18 miss +bytes 10-14/18 hit-fresh +bytes ``/18 miss +bytes 15-17/18 hit-fresh +bytes 0-17/18 miss, none +bytes 0-6/18 hit-fresh +bytes 7-13/18 hit-fresh +bytes 14-17/18 hit-fresh +bytes 5-16/18 hit-fresh, none +bytes 0-6/18 hit-fresh +*/18 hit-fresh, none diff --git a/tests/gold_tests/pluginTest/slice/slice_prefetch.test.py b/tests/gold_tests/pluginTest/slice/slice_prefetch.test.py index 5dc6aed134c..4201d6873ff 100644 --- a/tests/gold_tests/pluginTest/slice/slice_prefetch.test.py +++ b/tests/gold_tests/pluginTest/slice/slice_prefetch.test.py @@ -15,6 +15,7 @@ # 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 = ''' slice plugin prefetch feature test @@ -27,6 +28,7 @@ Test.SkipUnless( Condition.PluginExists('slice.so'), Condition.PluginExists('cache_range_requests.so'), + Condition.PluginExists('xdebug.so'), ) Test.ContinueOnFail = False @@ -52,7 +54,7 @@ response_header = {"headers": "HTTP/1.1 200 OK\r\n" + "Connection: close\r\n" + - "Cache-Control: public, max-age=500\r\n" + + "Cache-Control: public, max-age=5\r\n" + "\r\n", "timestamp": "1469733493.993", "body": body, @@ -79,7 +81,7 @@ resp_header = {"headers": "HTTP/1.1 206 Partial Content\r\n" + "Accept-Ranges: bytes\r\n" + - "Cache-Control: public, max-age=500\r\n" + + "Cache-Control: public, max-age=5\r\n" + f"Content-Range: bytes {b0}-{b1}/{bodylen}\r\n" + "Connection: close\r\n" + "\r\n", @@ -88,91 +90,103 @@ } server.addResponse("sessionlog.json", req_header, resp_header) -curl_and_args = 'curl -s -D /dev/stdout -o /dev/stderr -x http://127.0.0.1:{}'.format(ts.Variables.port) +curl_and_args = 'curl -s -D /dev/stdout -o /dev/stderr -x http://127.0.0.1:{} -H "x-debug: x-cache"'.format(ts.Variables.port) ts.Disk.remap_config.AddLines([ - f'map http://sliceprefetch1bytes1/ http://127.0.0.1:{server.Variables.Port}' + - f' @plugin=slice.so @pparam=--blockbytes-test={block_bytes_1} @pparam=--prefetch-count=1' + + f'map http://sliceprefetchbytes1/ http://127.0.0.1:{server.Variables.Port}' + + f' @plugin=slice.so @pparam=--blockbytes-test={block_bytes_1} @pparam=--prefetch-count=1 \\' + ' @plugin=cache_range_requests.so', - f'map http://sliceprefetch2bytes1/ http://127.0.0.1:{server.Variables.Port}' + - f' @plugin=slice.so @pparam=--blockbytes-test={block_bytes_1} @pparam=--prefetch-count=2' + - ' @plugin=cache_range_requests.so', - f'map http://sliceprefetch1bytes2/ http://127.0.0.1:{server.Variables.Port}' + - f' @plugin=slice.so @pparam=--blockbytes-test={block_bytes_2} @pparam=--prefetch-count=1' + - ' @plugin=cache_range_requests.so', - f'map http://sliceprefetch2bytes2/ http://127.0.0.1:{server.Variables.Port}' + - f' @plugin=slice.so @pparam=--blockbytes-test={block_bytes_2} @pparam=--prefetch-count=2' + + f'map http://sliceprefetchbytes2/ http://127.0.0.1:{server.Variables.Port}' + + f' @plugin=slice.so @pparam=--blockbytes-test={block_bytes_2} @pparam=--prefetch-count=3 \\' + ' @plugin=cache_range_requests.so', ]) +ts.Disk.plugin_config.AddLine('xdebug.so') +ts.Disk.logging_yaml.AddLines([ + 'logging:', + ' formats:', + ' - name: cache', + ' format: "%<{Content-Range}psh> %<{X-Cache}psh>"', + ' logs:', + ' - filename: cache', + ' format: cache', + ' mode: ascii', +]) + ts.Disk.records_config.update({ 'proxy.config.diags.debug.enabled': 1, - 'proxy.config.diags.debug.tags': 'slice|cache_range_requests|pvc', + 'proxy.config.diags.debug.tags': 'slice|cache_range_requests|xdebug', }) -# 0 Test - Full object slice with only next block prefetched in background, block bytes= 7 -tr = Test.AddTestRun("Full object slice with only next block prefetched in background, block bytes= 7") +# 0 Test - Full object slice (miss) with only block 14-20 prefetched in background, block bytes= 7 +tr = Test.AddTestRun("Full object slice: first block is miss, only block 14-20 prefetched") ps = tr.Processes.Default ps.StartBefore(server, ready=When.PortOpen(server.Variables.Port)) ps.StartBefore(Test.Processes.ts) -ps.Command = curl_and_args + ' http://sliceprefetch1bytes1/path' +ps.Command = curl_and_args + ' http://sliceprefetchbytes1/path' ps.ReturnCode = 0 ps.Streams.stderr = "gold/slice_200.stderr.gold" ps.Streams.stdout.Content = Testers.ContainsExpression("200 OK", "expected 200 OK response") +ps.Streams.stdout.Content += Testers.ContainsExpression("X-Cache: miss", "expected cache miss") tr.StillRunningAfter = ts -# 1 Test - Full object slice with nest 2 blocks prefetched in background, block bytes= 7 -tr = Test.AddTestRun("Test - Full object slice with nest 2 blocks prefetched in background, block bytes= 7") +# 1 Test - Full object slice (hit-fresh) with no prefetched blocks, block bytes= 7 +tr = Test.AddTestRun("Full object slice: first block is hit-fresh, no blocks prefetched") ps = tr.Processes.Default -ps.Command = curl_and_args + ' http://sliceprefetch2bytes1/path' +ps.Command = 'sleep 1; ' + curl_and_args + ' http://sliceprefetchbytes1/path' ps.ReturnCode = 0 ps.Streams.stderr = "gold/slice_200.stderr.gold" ps.Streams.stdout.Content = Testers.ContainsExpression("200 OK", "expected 200 OK response") +ps.Streams.stdout.Content += Testers.ContainsExpression("X-Cache: hit-fresh", "expected cache fresh") tr.StillRunningAfter = ts -# 2 Test - Full object slice with only next block prefetched in background, block bytes= 5 -tr = Test.AddTestRun("Full object slice with only next block prefetched in background, block bytes= 5") +# 2 Test - Full object slice with only next block prefetched in background, block bytes= 7 +tr = Test.AddTestRun("Full object slice: first block is hit-stale, only block 14-20 prefetched") ps = tr.Processes.Default -ps.Command = curl_and_args + ' http://sliceprefetch1bytes2/path' +ps.Command = 'sleep 5; ' + curl_and_args + ' http://sliceprefetchbytes1/path' ps.ReturnCode = 0 ps.Streams.stderr = "gold/slice_200.stderr.gold" ps.Streams.stdout.Content = Testers.ContainsExpression("200 OK", "expected 200 OK response") +ps.Streams.stdout.Content += Testers.ContainsExpression("X-Cache: hit-stale", "expected cache hit-stale") tr.StillRunningAfter = ts -# 3 Test - Full object slice with nest 2 blocks prefetched in background, block bytes= 5 -tr = Test.AddTestRun("Full object slice with nest 2 blocks prefetched in background, block bytes= 5") +# 3 Test - Full object slice (miss) with only blocks 10-14, 15-19 prefetched in background, block bytes= 5 +tr = Test.AddTestRun("Full object slice: first block is miss, only blocks 10-14, 15-19 prefetched") ps = tr.Processes.Default -ps.Command = curl_and_args + ' http://sliceprefetch2bytes2/path' +ps.Command = curl_and_args + ' http://sliceprefetchbytes2/path' + ' -r 0-' ps.ReturnCode = 0 ps.Streams.stderr = "gold/slice_200.stderr.gold" -ps.Streams.stdout.Content = Testers.ContainsExpression("200 OK", "expected 200 OK response") -tr.StillRunningAfter = ts - -# 4 Test - Whole asset via range -tr = Test.AddTestRun("Whole asset via range") -ps = tr.Processes.Default -ps.Command = curl_and_args + ' http://sliceprefetch1bytes1/path' + ' -r 0-' -ps.ReturnCode = 0 -ps.Streams.stderr = "gold/slice_206.stderr.gold" ps.Streams.stdout.Content = Testers.ContainsExpression("206 Partial Content", "expected 206 response") ps.Streams.stdout.Content += Testers.ContainsExpression("Content-Range: bytes 0-17/18", "mismatch byte content response") +ps.Streams.stdout.Content += Testers.ContainsExpression("X-Cache: miss", "expected cache miss") tr.StillRunningAfter = ts -# 5 Test - Non aligned slice request +# 4 Test - Non aligned slice request tr = Test.AddTestRun("Non aligned slice request") ps = tr.Processes.Default -ps.Command = curl_and_args + ' http://sliceprefetch1bytes1/path' + ' -r 5-16' +ps.Command = curl_and_args + ' http://sliceprefetchbytes1/path' + ' -r 5-16' ps.ReturnCode = 0 ps.Streams.stderr = "gold/slice_mid.stderr.gold" ps.Streams.stdout.Content = Testers.ContainsExpression("206 Partial Content", "expected 206 response") ps.Streams.stdout.Content += Testers.ContainsExpression("Content-Range: bytes 5-16/18", "mismatch byte content response") tr.StillRunningAfter = ts -# 6 Test - special case, begin inside last slice block but outside asset len +# 5 Test - special case, begin inside last slice block but outside asset len tr = Test.AddTestRun("Invalid end range request, 416") beg = len(body) + 1 end = beg + block_bytes ps = tr.Processes.Default -ps.Command = curl_and_args + f' http://sliceprefetch1bytes1/path -r {beg}-{end}' +ps.Command = curl_and_args + f' http://sliceprefetchbytes1/path -r {beg}-{end}' ps.Streams.stdout.Content = Testers.ContainsExpression("416 Requested Range Not Satisfiable", "expected 416 response") tr.StillRunningAfter = ts + +# 6 Test - All requests (client & slice internal) logs to see background fetches +cache_file = os.path.join(ts.Variables.LOGDIR, 'cache.log') +# Wait for log file to appear, then wait one extra second to make sure TS is done writing it. +test_run = Test.AddTestRun("Checking debug logs for background fetches") +test_run.Processes.Default.Command = ( + os.path.join(Test.Variables.AtsTestToolsDir, 'condwait') + ' 60 1 -f ' + + cache_file +) +ts.Disk.File(cache_file).Content = "gold/slice_prefetch.gold" +test_run.Processes.Default.ReturnCode = 0 From 40e82c282d8ea04b70c92d055daf47ebbba95a2a Mon Sep 17 00:00:00 2001 From: Serris Lew Date: Fri, 10 Jun 2022 10:00:25 -0700 Subject: [PATCH 2/9] Remove extra line spaces & generalize autest output --- plugins/cache_range_requests/cache_range_requests.cc | 2 -- .../gold_tests/pluginTest/slice/gold/slice_prefetch.gold | 8 ++++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/plugins/cache_range_requests/cache_range_requests.cc b/plugins/cache_range_requests/cache_range_requests.cc index 48c5a459760..7c3b708b869 100644 --- a/plugins/cache_range_requests/cache_range_requests.cc +++ b/plugins/cache_range_requests/cache_range_requests.cc @@ -370,11 +370,9 @@ handle_client_send_response(TSHttpTxn txnp, txndata *const txn_state) DEBUG_LOG("Restoring response header to TS_HTTP_STATUS_PARTIAL_CONTENT."); TSHttpHdrStatusSet(resp_buf, resp_loc, TS_HTTP_STATUS_PARTIAL_CONTENT); } - } else { DEBUG_LOG("Ignoring status code %d; txn_state->origin_status=%d", status, txn_state->origin_status); } - TSHandleMLocRelease(resp_buf, TS_NULL_MLOC, resp_loc); } diff --git a/tests/gold_tests/pluginTest/slice/gold/slice_prefetch.gold b/tests/gold_tests/pluginTest/slice/gold/slice_prefetch.gold index 173fac042a6..1a82ca139a3 100644 --- a/tests/gold_tests/pluginTest/slice/gold/slice_prefetch.gold +++ b/tests/gold_tests/pluginTest/slice/gold/slice_prefetch.gold @@ -13,10 +13,10 @@ bytes ``/18 hit-stale bytes 14-17/18 hit-fresh - hit-stale, none bytes 0-4/18 miss -bytes ``/18 miss -bytes ``/18 miss -bytes 10-14/18 hit-fresh -bytes ``/18 miss +bytes ``/18 `` +bytes ``/18 `` +bytes ``/18 `` +bytes ``/18 `` bytes 15-17/18 hit-fresh bytes 0-17/18 miss, none bytes 0-6/18 hit-fresh From 7775be64a854319638e7e095413a20dcfdfabfff Mon Sep 17 00:00:00 2001 From: Serris Lew Date: Mon, 13 Jun 2022 09:00:25 -0700 Subject: [PATCH 3/9] Verify cont is valid --- plugins/experimental/slice/slice.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/experimental/slice/slice.cc b/plugins/experimental/slice/slice.cc index 8f1f4096c80..50b474530ed 100644 --- a/plugins/experimental/slice/slice.cc +++ b/plugins/experimental/slice/slice.cc @@ -186,7 +186,7 @@ read_request(TSHttpTxn txnp, Config *const config) return true; } else { DEBUG_LOG("slice passing GET request through to next plugin"); - if (config->m_prefetchcount > 0 && config->m_first_block) { + if (config->m_prefetchcount > 0 && config->m_contp != nullptr && config->m_first_block) { TSHttpTxnHookAdd(txnp, TS_HTTP_READ_RESPONSE_HDR_HOOK, config->m_contp); config->m_first_block = false; } From 665715fe2c24cbd85e73e8ab1b980652b7f4dc38 Mon Sep 17 00:00:00 2001 From: Serris Lew Date: Thu, 23 Jun 2022 14:14:29 -0700 Subject: [PATCH 4/9] Use xdebug to only prefetch when first block is cacheable with miss/hit-stale status --- doc/admin-guide/plugins/slice.en.rst | 9 ++-- plugins/experimental/slice/Config.h | 2 - plugins/experimental/slice/HttpHeader.cc | 28 ++++++++++ plugins/experimental/slice/HttpHeader.h | 11 ++++ plugins/experimental/slice/intercept.cc | 11 +--- plugins/experimental/slice/prefetch.cc | 53 ++++++++++++------- plugins/experimental/slice/prefetch.h | 17 ++---- plugins/experimental/slice/server.cc | 4 ++ plugins/experimental/slice/slice.cc | 6 --- .../pluginTest/slice/add_via_hdr.config | 19 +++++++ .../pluginTest/slice/slice_prefetch.test.py | 5 +- 11 files changed, 111 insertions(+), 54 deletions(-) create mode 100644 tests/gold_tests/pluginTest/slice/add_via_hdr.config diff --git a/doc/admin-guide/plugins/slice.en.rst b/doc/admin-guide/plugins/slice.en.rst index 3582581628f..d949accbfb9 100644 --- a/doc/admin-guide/plugins/slice.en.rst +++ b/doc/admin-guide/plugins/slice.en.rst @@ -129,10 +129,11 @@ The slice plugin supports the following options:: --prefetch-count= (optional) Default is 0 Prefetches successive 'n' slice block requests in the background - and caches (with `cache_range_requests` plugin). If the first - block is a miss or hit-stale, it will prefetch. Otherwise, - prefetching is disabled. Especially for large objects, - prefetching can improve cache miss latency. + and caches (with `cache_range_requests` plugin). Prefetching is only + enabled when first block is written or updated in the cache, which is + a cacheable object with miss or hit-stale status. `xdebug` plugin must + be enabled with `Via` header request. Otherwise, prefetching is disabled. + Especially for large objects, prefetching can improve cache miss latency. -f for short Examples:: diff --git a/plugins/experimental/slice/Config.h b/plugins/experimental/slice/Config.h index d7ef1d23019..8408ffc2076 100644 --- a/plugins/experimental/slice/Config.h +++ b/plugins/experimental/slice/Config.h @@ -49,8 +49,6 @@ struct Config { std::string m_skip_header; std::string m_crr_ims_header; - TSCont m_contp{nullptr}; - bool m_first_block{true}; // Convert optarg to bytes static int64_t bytesFrom(char const *const valstr); diff --git a/plugins/experimental/slice/HttpHeader.cc b/plugins/experimental/slice/HttpHeader.cc index d98d112a3f7..4df8c9aac71 100644 --- a/plugins/experimental/slice/HttpHeader.cc +++ b/plugins/experimental/slice/HttpHeader.cc @@ -320,6 +320,34 @@ HttpHeader::toString() const return res; } +bool +HttpHeader::cacheFilled() const +{ + if (!isValid()) { + return false; + } + + char viastr[8192]; + int vialen = sizeof(viastr); + const char *errptr; + const char *cache_fill; + int erroffset, ovector[OVECTOR_SIZE], match_count; + + // look for expected Via field + bool const hasVia(valueForKey(TS_MIME_FIELD_VIA, TS_MIME_LEN_VIA, viastr, &vialen)); + if (hasVia) { + pcre *m_regex = pcre_compile("\\[(.*?)f(.)(.*?):(.*?)\\]", 0, &errptr, &erroffset, nullptr); + if (m_regex != nullptr) { + match_count = pcre_exec(m_regex, nullptr, viastr, vialen, 0, PCRE_NOTEMPTY, ovector, OVECTOR_SIZE); + if (match_count == 5) { + pcre_get_substring(viastr, ovector, match_count, 2, &cache_fill); + return (!strcmp(cache_fill, "W") || !strcmp(cache_fill, "U")); + } + } + } + return false; +} + /////// HdrMgr TSParseResult diff --git a/plugins/experimental/slice/HttpHeader.h b/plugins/experimental/slice/HttpHeader.h index 15a1b158ef1..3f9409d85b4 100644 --- a/plugins/experimental/slice/HttpHeader.h +++ b/plugins/experimental/slice/HttpHeader.h @@ -36,6 +36,14 @@ #include +#ifdef HAVE_PCRE_PCRE_H +#include +#else +#include +#endif + +#define OVECTOR_SIZE 30 + /** Designed to be a cheap throwaway struct which allows a consumer to make various calls to manipulate headers. @@ -131,6 +139,9 @@ struct HttpHeader { // sets header value as a time_t bool setKeyTime(char const *const key, int const keylen, time_t const timeval); + // read "f cache-fill" status of Via Header + bool cacheFilled() const; + /** dump header into provided char buffer */ std::string toString() const; diff --git a/plugins/experimental/slice/intercept.cc b/plugins/experimental/slice/intercept.cc index e3094aebdf9..2b6ef3bdfcd 100644 --- a/plugins/experimental/slice/intercept.cc +++ b/plugins/experimental/slice/intercept.cc @@ -50,16 +50,7 @@ intercept_hook(TSCont contp, TSEvent event, void *edata) case TS_EVENT_ERROR: { abort(contp, data); } break; - case TS_EVENT_HTTP_READ_RESPONSE_HDR: { - TSHttpTxn const txnp = static_cast(edata); - int lookupStatus; - if (data->m_blocknum == 0 && TS_SUCCESS == TSHttpTxnCacheLookupStatusGet(txnp, &lookupStatus)) { - if (lookupStatus == TS_CACHE_LOOKUP_MISS || lookupStatus == TS_CACHE_LOOKUP_HIT_STALE) { - data->m_prefetchable = true; - } - } - TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE); - } break; + default: { // data from client -- only the initial header if (data->m_dnstream.m_read.isOpen() && edata == data->m_dnstream.m_read.m_vio) { diff --git a/plugins/experimental/slice/prefetch.cc b/plugins/experimental/slice/prefetch.cc index 8fba88b738a..a83d25a6310 100644 --- a/plugins/experimental/slice/prefetch.cc +++ b/plugins/experimental/slice/prefetch.cc @@ -43,13 +43,13 @@ BgBlockFetch::schedule(Data *const data, int blocknum) bool BgBlockFetch::fetch(Data *const data) { - if (_bg_stream.m_read.isOpen()) { + if (m_stream.m_read.isOpen()) { // should never happen since the connection was just initialized ERROR_LOG("Background block request already in flight!"); return false; } - int64_t const blockbeg = (data->m_config->m_blockbytes * _blocknum); + int64_t const blockbeg = (data->m_config->m_blockbytes * m_blocknum); Range blockbe(blockbeg, blockbeg + data->m_config->m_blockbytes); char rangestr[1024]; @@ -69,11 +69,11 @@ BgBlockFetch::fetch(Data *const data) ERROR_LOG("Error trying to set range request header %s", rangestr); return false; } - TSAssert(nullptr == _cont); + TSAssert(nullptr == m_cont); // Setup the continuation - _cont = TSContCreate(handler, TSMutexCreate()); - TSContDataSet(_cont, static_cast(this)); + m_cont = TSContCreate(handler, TSMutexCreate()); + TSContDataSet(m_cont, static_cast(this)); // create virtual connection back into ATS TSHttpConnectOptions options = TSHttpConnectOptionsGet(TS_CONNECT_PLUGIN); @@ -88,14 +88,16 @@ BgBlockFetch::fetch(Data *const data) int const hlen = TSHttpHdrLengthGet(header.m_buffer, header.m_lochdr); // set up connection with the HttpConnect server - _bg_stream.setupConnection(upvc); - _bg_stream.setupVioWrite(_cont, hlen); - TSHttpHdrPrint(header.m_buffer, header.m_lochdr, _bg_stream.m_write.m_iobuf); + m_stream.setupConnection(upvc); + m_stream.setupVioWrite(m_cont, hlen); + TSHttpHdrPrint(header.m_buffer, header.m_lochdr, m_stream.m_write.m_iobuf); if (TSIsDebugTagSet(PLUGIN_NAME)) { std::string const headerstr(header.toString()); DEBUG_LOG("Headers\n%s", headerstr.c_str()); } + // ensure blocks are pulled through to cache + m_stream.setupVioRead(m_cont, INT64_MAX); return true; } @@ -112,22 +114,37 @@ BgBlockFetch::handler(TSCont contp, TSEvent event, void * /* edata ATS_UNUSED */ switch (event) { case TS_EVENT_VCONN_WRITE_COMPLETE: - TSVConnShutdown(bg->_bg_stream.m_vc, 0, 1); - bg->_bg_stream.close(); - delete bg; + TSVConnShutdown(bg->m_stream.m_vc, 0, 1); break; - default: - if (event == TS_EVENT_VCONN_INACTIVITY_TIMEOUT) { - DEBUG_LOG("encountered Inactivity Timeout"); - TSVConnAbort(bg->_bg_stream.m_vc, TS_VC_CLOSE_ABORT); - } else { - TSVConnClose(bg->_bg_stream.m_vc); + case TS_EVENT_VCONN_READ_READY: { + TSIOBufferReader const reader = bg->m_stream.m_read.m_reader; + if (nullptr != reader) { + int64_t const avail = TSIOBufferReaderAvail(reader); + TSIOBufferReaderConsume(reader, avail); + TSVIO const rvio = bg->m_stream.m_read.m_vio; + TSVIONDoneSet(rvio, TSVIONDoneGet(rvio) + avail); + TSVIOReenable(rvio); } - bg->_bg_stream.abort(); + } break; + case TS_EVENT_NET_ACCEPT_FAILED: + case TS_EVENT_VCONN_INACTIVITY_TIMEOUT: + case TS_EVENT_VCONN_ACTIVE_TIMEOUT: + case TS_EVENT_ERROR: + bg->m_stream.abort(); + TSContDataSet(contp, nullptr); + delete bg; + TSContDestroy(contp); + break; + case TS_EVENT_VCONN_READ_COMPLETE: + case TS_EVENT_VCONN_EOS: + bg->m_stream.close(); TSContDataSet(contp, nullptr); delete bg; TSContDestroy(contp); break; + default: + DEBUG_LOG("Unhandled bg fetch event:%s (%d)", TSHttpEventNameLookup(event), event); + break; } return 0; } diff --git a/plugins/experimental/slice/prefetch.h b/plugins/experimental/slice/prefetch.h index e04376bacf6..e5d1edfc247 100644 --- a/plugins/experimental/slice/prefetch.h +++ b/plugins/experimental/slice/prefetch.h @@ -35,22 +35,13 @@ struct BgBlockFetch { static bool schedule(Data *const data, int blocknum); - explicit BgBlockFetch(int blocknum) : _blocknum(blocknum) {} + explicit BgBlockFetch(int blocknum) : m_blocknum(blocknum) {} bool fetch(Data *const data); static int handler(TSCont contp, TSEvent event, void * /* edata ATS_UNUSED */); /* This is for the actual background fetch / NetVC */ - Stage _bg_stream; - - int _blocknum; - TSCont _cont = nullptr; - - ~BgBlockFetch() - { - if (nullptr != _cont) { - TSContDestroy(_cont); - _cont = nullptr; - } - } + Stage m_stream; + int m_blocknum; + TSCont m_cont = nullptr; }; \ No newline at end of file diff --git a/plugins/experimental/slice/server.cc b/plugins/experimental/slice/server.cc index ddf0cc9ad2f..40172b77732 100644 --- a/plugins/experimental/slice/server.cc +++ b/plugins/experimental/slice/server.cc @@ -217,6 +217,10 @@ handleFirstServerHeader(Data *const data, TSCont const contp) data->m_bytessent = hbytes; TSVIOReenable(output_vio); + if (data->m_config->m_prefetchcount > 0 && header.cacheFilled()) { + data->m_prefetchable = true; + } + return HeaderState::Good; } diff --git a/plugins/experimental/slice/slice.cc b/plugins/experimental/slice/slice.cc index 50b474530ed..abb1630fc1d 100644 --- a/plugins/experimental/slice/slice.cc +++ b/plugins/experimental/slice/slice.cc @@ -180,16 +180,10 @@ read_request(TSHttpTxn txnp, Config *const config) // Grab the transaction TSHttpTxnIntercept(icontp, txnp); - config->m_contp = icontp; - config->m_first_block = true; return true; } else { DEBUG_LOG("slice passing GET request through to next plugin"); - if (config->m_prefetchcount > 0 && config->m_contp != nullptr && config->m_first_block) { - TSHttpTxnHookAdd(txnp, TS_HTTP_READ_RESPONSE_HDR_HOOK, config->m_contp); - config->m_first_block = false; - } } } diff --git a/tests/gold_tests/pluginTest/slice/add_via_hdr.config b/tests/gold_tests/pluginTest/slice/add_via_hdr.config new file mode 100644 index 00000000000..2dee8ec1f5d --- /dev/null +++ b/tests/gold_tests/pluginTest/slice/add_via_hdr.config @@ -0,0 +1,19 @@ +# +# 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. + +cond %{READ_REQUEST_HDR_HOOK} + add-header X-Debug Via \ No newline at end of file diff --git a/tests/gold_tests/pluginTest/slice/slice_prefetch.test.py b/tests/gold_tests/pluginTest/slice/slice_prefetch.test.py index 4201d6873ff..c5a5aa2fd8e 100644 --- a/tests/gold_tests/pluginTest/slice/slice_prefetch.test.py +++ b/tests/gold_tests/pluginTest/slice/slice_prefetch.test.py @@ -29,6 +29,7 @@ Condition.PluginExists('slice.so'), Condition.PluginExists('cache_range_requests.so'), Condition.PluginExists('xdebug.so'), + Condition.PluginExists('header_rewrite.so'), ) Test.ContinueOnFail = False @@ -101,7 +102,9 @@ ' @plugin=cache_range_requests.so', ]) +ts.Setup.CopyAs('add_via_hdr.config', Test.RunDirectory) ts.Disk.plugin_config.AddLine('xdebug.so') +ts.Disk.plugin_config.AddLine(f'header_rewrite.so {Test.RunDirectory}/add_via_hdr.config') ts.Disk.logging_yaml.AddLines([ 'logging:', ' formats:', @@ -115,7 +118,7 @@ ts.Disk.records_config.update({ 'proxy.config.diags.debug.enabled': 1, - 'proxy.config.diags.debug.tags': 'slice|cache_range_requests|xdebug', + 'proxy.config.diags.debug.tags': 'slice|cache_range_requests|xdebug|header_rewrite', }) # 0 Test - Full object slice (miss) with only block 14-20 prefetched in background, block bytes= 7 From c6c57a2e646a224aa7fae4250e7837a9dc59b22b Mon Sep 17 00:00:00 2001 From: Serris Lew Date: Mon, 18 Jul 2022 12:26:39 -0700 Subject: [PATCH 5/9] Precompile via regex pattern in config --- plugins/experimental/slice/Config.cc | 6 ++++++ plugins/experimental/slice/Config.h | 1 + plugins/experimental/slice/HttpHeader.cc | 10 ++++------ plugins/experimental/slice/HttpHeader.h | 2 +- plugins/experimental/slice/server.cc | 2 +- 5 files changed, 13 insertions(+), 8 deletions(-) diff --git a/plugins/experimental/slice/Config.cc b/plugins/experimental/slice/Config.cc index 5cd71d47339..13638cba870 100644 --- a/plugins/experimental/slice/Config.cc +++ b/plugins/experimental/slice/Config.cc @@ -44,6 +44,9 @@ Config::~Config() if (nullptr != m_regex) { pcre_free(m_regex); } + if (nullptr != m_via_regex) { + pcre_free(m_via_regex); + } } int64_t @@ -224,6 +227,9 @@ Config::fromArgs(int const argc, char const *const argv[]) } } break; case 'f': { + const char *errptr; + int erroffset; + m_via_regex = pcre_compile("\\[(.*?)f(.)(.*?):(.*?)\\]", 0, &errptr, &erroffset, nullptr); m_prefetchcount = atoi(optarg); } break; default: diff --git a/plugins/experimental/slice/Config.h b/plugins/experimental/slice/Config.h index 8408ffc2076..7cae95baadf 100644 --- a/plugins/experimental/slice/Config.h +++ b/plugins/experimental/slice/Config.h @@ -40,6 +40,7 @@ struct Config { enum RegexType { None, Include, Exclude }; RegexType m_regex_type{None}; pcre *m_regex{nullptr}; + pcre *m_via_regex{nullptr}; pcre_extra *m_regex_extra{nullptr}; int m_paceerrsecs{0}; // -1 disable logging, 0 no pacing, max 60s int m_prefetchcount{0}; // 0 disables prefetching diff --git a/plugins/experimental/slice/HttpHeader.cc b/plugins/experimental/slice/HttpHeader.cc index 4df8c9aac71..c5c3c344143 100644 --- a/plugins/experimental/slice/HttpHeader.cc +++ b/plugins/experimental/slice/HttpHeader.cc @@ -321,7 +321,7 @@ HttpHeader::toString() const } bool -HttpHeader::cacheFilled() const +HttpHeader::cacheFilled(pcre *regex_pat) const { if (!isValid()) { return false; @@ -329,16 +329,14 @@ HttpHeader::cacheFilled() const char viastr[8192]; int vialen = sizeof(viastr); - const char *errptr; const char *cache_fill; - int erroffset, ovector[OVECTOR_SIZE], match_count; + int ovector[OVECTOR_SIZE], match_count; // look for expected Via field bool const hasVia(valueForKey(TS_MIME_FIELD_VIA, TS_MIME_LEN_VIA, viastr, &vialen)); if (hasVia) { - pcre *m_regex = pcre_compile("\\[(.*?)f(.)(.*?):(.*?)\\]", 0, &errptr, &erroffset, nullptr); - if (m_regex != nullptr) { - match_count = pcre_exec(m_regex, nullptr, viastr, vialen, 0, PCRE_NOTEMPTY, ovector, OVECTOR_SIZE); + if (regex_pat != nullptr) { + match_count = pcre_exec(regex_pat, nullptr, viastr, vialen, 0, PCRE_NOTEMPTY, ovector, OVECTOR_SIZE); if (match_count == 5) { pcre_get_substring(viastr, ovector, match_count, 2, &cache_fill); return (!strcmp(cache_fill, "W") || !strcmp(cache_fill, "U")); diff --git a/plugins/experimental/slice/HttpHeader.h b/plugins/experimental/slice/HttpHeader.h index 3f9409d85b4..0c2348f4cb7 100644 --- a/plugins/experimental/slice/HttpHeader.h +++ b/plugins/experimental/slice/HttpHeader.h @@ -140,7 +140,7 @@ struct HttpHeader { bool setKeyTime(char const *const key, int const keylen, time_t const timeval); // read "f cache-fill" status of Via Header - bool cacheFilled() const; + bool cacheFilled(pcre *regex_pat) const; /** dump header into provided char buffer */ diff --git a/plugins/experimental/slice/server.cc b/plugins/experimental/slice/server.cc index 40172b77732..de1ff4f6a9f 100644 --- a/plugins/experimental/slice/server.cc +++ b/plugins/experimental/slice/server.cc @@ -217,7 +217,7 @@ handleFirstServerHeader(Data *const data, TSCont const contp) data->m_bytessent = hbytes; TSVIOReenable(output_vio); - if (data->m_config->m_prefetchcount > 0 && header.cacheFilled()) { + if (data->m_config->m_prefetchcount > 0 && header.cacheFilled(data->m_config->m_via_regex)) { data->m_prefetchable = true; } From afb7e7ee33dcff7a22441743e04207feef7ffe7f Mon Sep 17 00:00:00 2001 From: Serris Lew Date: Fri, 22 Jul 2022 10:57:08 -0700 Subject: [PATCH 6/9] Remove plugin dependency, add cache status header between slice and crr --- doc/admin-guide/plugins/slice.en.rst | 4 +-- .../cache_range_requests.cc | 24 +++++++++++++++++ plugins/experimental/slice/Config.cc | 6 ----- plugins/experimental/slice/Config.h | 1 - plugins/experimental/slice/HttpHeader.cc | 26 ------------------- plugins/experimental/slice/HttpHeader.h | 11 +------- plugins/experimental/slice/server.cc | 3 ++- plugins/experimental/slice/util.cc | 7 +++++ .../pluginTest/slice/add_via_hdr.config | 19 -------------- .../pluginTest/slice/slice_prefetch.test.py | 5 +--- 10 files changed, 36 insertions(+), 70 deletions(-) delete mode 100644 tests/gold_tests/pluginTest/slice/add_via_hdr.config diff --git a/doc/admin-guide/plugins/slice.en.rst b/doc/admin-guide/plugins/slice.en.rst index d949accbfb9..f9f26bdf3fe 100644 --- a/doc/admin-guide/plugins/slice.en.rst +++ b/doc/admin-guide/plugins/slice.en.rst @@ -130,9 +130,7 @@ The slice plugin supports the following options:: Default is 0 Prefetches successive 'n' slice block requests in the background and caches (with `cache_range_requests` plugin). Prefetching is only - enabled when first block is written or updated in the cache, which is - a cacheable object with miss or hit-stale status. `xdebug` plugin must - be enabled with `Via` header request. Otherwise, prefetching is disabled. + enabled when first block is a cacheable object with miss or hit-stale status. Especially for large objects, prefetching can improve cache miss latency. -f for short diff --git a/plugins/cache_range_requests/cache_range_requests.cc b/plugins/cache_range_requests/cache_range_requests.cc index 7c3b708b869..83bc4e9a417 100644 --- a/plugins/cache_range_requests/cache_range_requests.cc +++ b/plugins/cache_range_requests/cache_range_requests.cc @@ -48,6 +48,7 @@ using parent_select_mode_t = enum parent_select_mode { }; constexpr std::string_view DefaultImsHeader = {"X-Crr-Ims"}; +static char const *const SLICE_CRR_HEADER = {"Slice-Crr-Status"}; struct pluginconfig { parent_select_mode_t ps_mode{PS_DEFAULT}; @@ -64,6 +65,8 @@ struct txndata { time_t ims_time{0}; bool verify_cacheability{false}; bool cache_complete_responses{false}; + bool slice_response{false}; + bool slice_request{false}; }; // pluginconfig struct (global plugin only) @@ -296,6 +299,13 @@ range_header_check(TSHttpTxn txnp, pluginconfig *const pc) TSHttpTxnHookAdd(txnp, TS_HTTP_CACHE_LOOKUP_COMPLETE_HOOK, txn_contp); DEBUG_LOG("Also Added TS_HTTP_CACHE_LOOKUP_COMPLETE_HOOK"); } + + // check if slice requests for cache lookup status + TSMLoc const locfield(TSMimeHdrFieldFind(hdr_buf, hdr_loc, SLICE_CRR_HEADER, strlen(SLICE_CRR_HEADER))); + if (nullptr != locfield) { + TSHandleMLocRelease(hdr_buf, hdr_loc, locfield); + txn_state->slice_request = true; + } } TSHandleMLocRelease(hdr_buf, hdr_loc, range_loc); } else { @@ -370,6 +380,12 @@ handle_client_send_response(TSHttpTxn txnp, txndata *const txn_state) DEBUG_LOG("Restoring response header to TS_HTTP_STATUS_PARTIAL_CONTENT."); TSHttpHdrStatusSet(resp_buf, resp_loc, TS_HTTP_STATUS_PARTIAL_CONTENT); } + + remove_header(resp_buf, resp_loc, SLICE_CRR_HEADER, strlen(SLICE_CRR_HEADER)); + if (txn_state->slice_response) { + std::string valStr = "-"; + set_header(resp_buf, resp_loc, SLICE_CRR_HEADER, strlen(SLICE_CRR_HEADER), valStr.c_str(), valStr.length()); + } } else { DEBUG_LOG("Ignoring status code %d; txn_state->origin_status=%d", status, txn_state->origin_status); } @@ -404,6 +420,7 @@ handle_server_read_response(TSHttpTxn txnp, txndata *const txn_state) { TSMBuffer resp_buf = nullptr; TSMLoc resp_loc = TS_NULL_MLOC; + int cache_lookup; if (TS_SUCCESS == TSHttpTxnServerRespGet(txnp, &resp_buf, &resp_loc)) { TSHttpStatus const status = TSHttpHdrStatusGet(resp_buf, resp_loc); @@ -433,6 +450,13 @@ handle_server_read_response(TSHttpTxn txnp, txndata *const txn_state) DEBUG_LOG("Allowing object to be cached."); } } + // slice requesting cache lookup status and cacheability + if (txn_state->slice_request && TSHttpTxnIsCacheable(txnp, nullptr, resp_buf) && + TSHttpTxnCacheLookupStatusGet(txnp, &cache_lookup) == TS_SUCCESS && + (cache_lookup == TS_CACHE_LOOKUP_MISS || cache_lookup == TS_CACHE_LOOKUP_HIT_STALE)) { + txn_state->slice_response = true; + } + TSHandleMLocRelease(resp_buf, TS_NULL_MLOC, resp_loc); } } diff --git a/plugins/experimental/slice/Config.cc b/plugins/experimental/slice/Config.cc index 13638cba870..5cd71d47339 100644 --- a/plugins/experimental/slice/Config.cc +++ b/plugins/experimental/slice/Config.cc @@ -44,9 +44,6 @@ Config::~Config() if (nullptr != m_regex) { pcre_free(m_regex); } - if (nullptr != m_via_regex) { - pcre_free(m_via_regex); - } } int64_t @@ -227,9 +224,6 @@ Config::fromArgs(int const argc, char const *const argv[]) } } break; case 'f': { - const char *errptr; - int erroffset; - m_via_regex = pcre_compile("\\[(.*?)f(.)(.*?):(.*?)\\]", 0, &errptr, &erroffset, nullptr); m_prefetchcount = atoi(optarg); } break; default: diff --git a/plugins/experimental/slice/Config.h b/plugins/experimental/slice/Config.h index 7cae95baadf..8408ffc2076 100644 --- a/plugins/experimental/slice/Config.h +++ b/plugins/experimental/slice/Config.h @@ -40,7 +40,6 @@ struct Config { enum RegexType { None, Include, Exclude }; RegexType m_regex_type{None}; pcre *m_regex{nullptr}; - pcre *m_via_regex{nullptr}; pcre_extra *m_regex_extra{nullptr}; int m_paceerrsecs{0}; // -1 disable logging, 0 no pacing, max 60s int m_prefetchcount{0}; // 0 disables prefetching diff --git a/plugins/experimental/slice/HttpHeader.cc b/plugins/experimental/slice/HttpHeader.cc index c5c3c344143..d98d112a3f7 100644 --- a/plugins/experimental/slice/HttpHeader.cc +++ b/plugins/experimental/slice/HttpHeader.cc @@ -320,32 +320,6 @@ HttpHeader::toString() const return res; } -bool -HttpHeader::cacheFilled(pcre *regex_pat) const -{ - if (!isValid()) { - return false; - } - - char viastr[8192]; - int vialen = sizeof(viastr); - const char *cache_fill; - int ovector[OVECTOR_SIZE], match_count; - - // look for expected Via field - bool const hasVia(valueForKey(TS_MIME_FIELD_VIA, TS_MIME_LEN_VIA, viastr, &vialen)); - if (hasVia) { - if (regex_pat != nullptr) { - match_count = pcre_exec(regex_pat, nullptr, viastr, vialen, 0, PCRE_NOTEMPTY, ovector, OVECTOR_SIZE); - if (match_count == 5) { - pcre_get_substring(viastr, ovector, match_count, 2, &cache_fill); - return (!strcmp(cache_fill, "W") || !strcmp(cache_fill, "U")); - } - } - } - return false; -} - /////// HdrMgr TSParseResult diff --git a/plugins/experimental/slice/HttpHeader.h b/plugins/experimental/slice/HttpHeader.h index 0c2348f4cb7..f3b23b0851d 100644 --- a/plugins/experimental/slice/HttpHeader.h +++ b/plugins/experimental/slice/HttpHeader.h @@ -36,13 +36,7 @@ #include -#ifdef HAVE_PCRE_PCRE_H -#include -#else -#include -#endif - -#define OVECTOR_SIZE 30 +static char const *const SLICE_CRR_HEADER = {"Slice-Crr-Status"}; /** Designed to be a cheap throwaway struct which allows a @@ -139,9 +133,6 @@ struct HttpHeader { // sets header value as a time_t bool setKeyTime(char const *const key, int const keylen, time_t const timeval); - // read "f cache-fill" status of Via Header - bool cacheFilled(pcre *regex_pat) const; - /** dump header into provided char buffer */ std::string toString() const; diff --git a/plugins/experimental/slice/server.cc b/plugins/experimental/slice/server.cc index de1ff4f6a9f..fa56f3aeb3c 100644 --- a/plugins/experimental/slice/server.cc +++ b/plugins/experimental/slice/server.cc @@ -20,6 +20,7 @@ #include "Config.h" #include "ContentRange.h" +#include "HttpHeader.h" #include "response.h" #include "transfer.h" #include "util.h" @@ -217,7 +218,7 @@ handleFirstServerHeader(Data *const data, TSCont const contp) data->m_bytessent = hbytes; TSVIOReenable(output_vio); - if (data->m_config->m_prefetchcount > 0 && header.cacheFilled(data->m_config->m_via_regex)) { + if (data->m_config->m_prefetchcount > 0 && header.hasKey(SLICE_CRR_HEADER, strlen(SLICE_CRR_HEADER))) { data->m_prefetchable = true; } diff --git a/plugins/experimental/slice/util.cc b/plugins/experimental/slice/util.cc index 18b00b65233..97183716863 100644 --- a/plugins/experimental/slice/util.cc +++ b/plugins/experimental/slice/util.cc @@ -18,6 +18,7 @@ #include "util.h" #include "prefetch.h" +#include "HttpHeader.h" #include "Config.h" #include "Data.h" @@ -86,6 +87,12 @@ request_block(TSCont contp, Data *const data) return false; } + header.removeKey(SLICE_CRR_HEADER, strlen(SLICE_CRR_HEADER)); + if (data->m_blocknum == 0 && data->m_config->m_prefetchcount > 0) { + std::string valStr = "-"; + header.setKeyVal(SLICE_CRR_HEADER, strlen(SLICE_CRR_HEADER), valStr.c_str(), valStr.length()); + } + // create virtual connection back into ATS TSHttpConnectOptions options = TSHttpConnectOptionsGet(TS_CONNECT_PLUGIN); options.addr = reinterpret_cast(&data->m_client_ip); diff --git a/tests/gold_tests/pluginTest/slice/add_via_hdr.config b/tests/gold_tests/pluginTest/slice/add_via_hdr.config deleted file mode 100644 index 2dee8ec1f5d..00000000000 --- a/tests/gold_tests/pluginTest/slice/add_via_hdr.config +++ /dev/null @@ -1,19 +0,0 @@ -# -# 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. - -cond %{READ_REQUEST_HDR_HOOK} - add-header X-Debug Via \ No newline at end of file diff --git a/tests/gold_tests/pluginTest/slice/slice_prefetch.test.py b/tests/gold_tests/pluginTest/slice/slice_prefetch.test.py index c5a5aa2fd8e..4201d6873ff 100644 --- a/tests/gold_tests/pluginTest/slice/slice_prefetch.test.py +++ b/tests/gold_tests/pluginTest/slice/slice_prefetch.test.py @@ -29,7 +29,6 @@ Condition.PluginExists('slice.so'), Condition.PluginExists('cache_range_requests.so'), Condition.PluginExists('xdebug.so'), - Condition.PluginExists('header_rewrite.so'), ) Test.ContinueOnFail = False @@ -102,9 +101,7 @@ ' @plugin=cache_range_requests.so', ]) -ts.Setup.CopyAs('add_via_hdr.config', Test.RunDirectory) ts.Disk.plugin_config.AddLine('xdebug.so') -ts.Disk.plugin_config.AddLine(f'header_rewrite.so {Test.RunDirectory}/add_via_hdr.config') ts.Disk.logging_yaml.AddLines([ 'logging:', ' formats:', @@ -118,7 +115,7 @@ ts.Disk.records_config.update({ 'proxy.config.diags.debug.enabled': 1, - 'proxy.config.diags.debug.tags': 'slice|cache_range_requests|xdebug|header_rewrite', + 'proxy.config.diags.debug.tags': 'slice|cache_range_requests|xdebug', }) # 0 Test - Full object slice (miss) with only block 14-20 prefetched in background, block bytes= 7 From 1c4ec97195c032569472035ada6c359320071c03 Mon Sep 17 00:00:00 2001 From: Serris Lew Date: Mon, 25 Jul 2022 12:33:49 -0700 Subject: [PATCH 7/9] Only enable prefetching from CRR on 206 partial case from origin --- plugins/cache_range_requests/cache_range_requests.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/plugins/cache_range_requests/cache_range_requests.cc b/plugins/cache_range_requests/cache_range_requests.cc index 83bc4e9a417..df53ce0a3d2 100644 --- a/plugins/cache_range_requests/cache_range_requests.cc +++ b/plugins/cache_range_requests/cache_range_requests.cc @@ -434,7 +434,13 @@ handle_server_read_response(TSHttpTxn txnp, txndata *const txn_state) if (txn_state->verify_cacheability && !TSHttpTxnIsCacheable(txnp, nullptr, resp_buf)) { DEBUG_LOG("transaction is not cacheable; resetting status code to 206"); TSHttpHdrStatusSet(resp_buf, resp_loc, TS_HTTP_STATUS_PARTIAL_CONTENT); + } else if (txn_state->slice_request && TSHttpTxnIsCacheable(txnp, nullptr, resp_buf) && + TSHttpTxnCacheLookupStatusGet(txnp, &cache_lookup) == TS_SUCCESS && + (cache_lookup == TS_CACHE_LOOKUP_MISS || cache_lookup == TS_CACHE_LOOKUP_HIT_STALE)) { + // slice requesting cache lookup status and cacheability + txn_state->slice_response = true; } + } else if (TS_HTTP_STATUS_OK == status) { bool cacheable = txn_state->cache_complete_responses; @@ -450,12 +456,6 @@ handle_server_read_response(TSHttpTxn txnp, txndata *const txn_state) DEBUG_LOG("Allowing object to be cached."); } } - // slice requesting cache lookup status and cacheability - if (txn_state->slice_request && TSHttpTxnIsCacheable(txnp, nullptr, resp_buf) && - TSHttpTxnCacheLookupStatusGet(txnp, &cache_lookup) == TS_SUCCESS && - (cache_lookup == TS_CACHE_LOOKUP_MISS || cache_lookup == TS_CACHE_LOOKUP_HIT_STALE)) { - txn_state->slice_response = true; - } TSHandleMLocRelease(resp_buf, TS_NULL_MLOC, resp_loc); } From 27a933be4566b405bcc962554eb94bb699ae1284 Mon Sep 17 00:00:00 2001 From: Serris Lew Date: Tue, 26 Jul 2022 11:47:22 -0700 Subject: [PATCH 8/9] Update header type, fix 206 case for 304 in CRR, allow header to be used for debugging --- .../cache_range_requests.cc | 24 ++++++++++--------- plugins/experimental/slice/HttpHeader.h | 2 +- plugins/experimental/slice/server.cc | 2 +- plugins/experimental/slice/util.cc | 6 ++--- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/plugins/cache_range_requests/cache_range_requests.cc b/plugins/cache_range_requests/cache_range_requests.cc index df53ce0a3d2..36b81c6f4ec 100644 --- a/plugins/cache_range_requests/cache_range_requests.cc +++ b/plugins/cache_range_requests/cache_range_requests.cc @@ -48,7 +48,7 @@ using parent_select_mode_t = enum parent_select_mode { }; constexpr std::string_view DefaultImsHeader = {"X-Crr-Ims"}; -static char const *const SLICE_CRR_HEADER = {"Slice-Crr-Status"}; +constexpr std::string_view SLICE_CRR_HEADER = {"Slice-Crr-Status"}; struct pluginconfig { parent_select_mode_t ps_mode{PS_DEFAULT}; @@ -301,7 +301,7 @@ range_header_check(TSHttpTxn txnp, pluginconfig *const pc) } // check if slice requests for cache lookup status - TSMLoc const locfield(TSMimeHdrFieldFind(hdr_buf, hdr_loc, SLICE_CRR_HEADER, strlen(SLICE_CRR_HEADER))); + TSMLoc const locfield(TSMimeHdrFieldFind(hdr_buf, hdr_loc, SLICE_CRR_HEADER.data(), SLICE_CRR_HEADER.size())); if (nullptr != locfield) { TSHandleMLocRelease(hdr_buf, hdr_loc, locfield); txn_state->slice_request = true; @@ -381,10 +381,10 @@ handle_client_send_response(TSHttpTxn txnp, txndata *const txn_state) TSHttpHdrStatusSet(resp_buf, resp_loc, TS_HTTP_STATUS_PARTIAL_CONTENT); } - remove_header(resp_buf, resp_loc, SLICE_CRR_HEADER, strlen(SLICE_CRR_HEADER)); + remove_header(resp_buf, resp_loc, SLICE_CRR_HEADER.data(), SLICE_CRR_HEADER.size()); if (txn_state->slice_response) { - std::string valStr = "-"; - set_header(resp_buf, resp_loc, SLICE_CRR_HEADER, strlen(SLICE_CRR_HEADER), valStr.c_str(), valStr.length()); + std::string valStr = "1"; + set_header(resp_buf, resp_loc, SLICE_CRR_HEADER.data(), SLICE_CRR_HEADER.size(), valStr.c_str(), valStr.length()); } } else { DEBUG_LOG("Ignoring status code %d; txn_state->origin_status=%d", status, txn_state->origin_status); @@ -434,13 +434,7 @@ handle_server_read_response(TSHttpTxn txnp, txndata *const txn_state) if (txn_state->verify_cacheability && !TSHttpTxnIsCacheable(txnp, nullptr, resp_buf)) { DEBUG_LOG("transaction is not cacheable; resetting status code to 206"); TSHttpHdrStatusSet(resp_buf, resp_loc, TS_HTTP_STATUS_PARTIAL_CONTENT); - } else if (txn_state->slice_request && TSHttpTxnIsCacheable(txnp, nullptr, resp_buf) && - TSHttpTxnCacheLookupStatusGet(txnp, &cache_lookup) == TS_SUCCESS && - (cache_lookup == TS_CACHE_LOOKUP_MISS || cache_lookup == TS_CACHE_LOOKUP_HIT_STALE)) { - // slice requesting cache lookup status and cacheability - txn_state->slice_response = true; } - } else if (TS_HTTP_STATUS_OK == status) { bool cacheable = txn_state->cache_complete_responses; @@ -457,6 +451,14 @@ handle_server_read_response(TSHttpTxn txnp, txndata *const txn_state) } } + // slice requesting cache lookup status and cacheability (only on miss or validation) + if ((txn_state->origin_status == TS_HTTP_STATUS_PARTIAL_CONTENT || txn_state->origin_status == TS_HTTP_STATUS_NOT_MODIFIED) && + txn_state->slice_request && TSHttpTxnIsCacheable(txnp, nullptr, resp_buf) && + TSHttpTxnCacheLookupStatusGet(txnp, &cache_lookup) == TS_SUCCESS && + (cache_lookup == TS_CACHE_LOOKUP_MISS || cache_lookup == TS_CACHE_LOOKUP_HIT_STALE)) { + txn_state->slice_response = true; + } + TSHandleMLocRelease(resp_buf, TS_NULL_MLOC, resp_loc); } } diff --git a/plugins/experimental/slice/HttpHeader.h b/plugins/experimental/slice/HttpHeader.h index f3b23b0851d..9c4be14d760 100644 --- a/plugins/experimental/slice/HttpHeader.h +++ b/plugins/experimental/slice/HttpHeader.h @@ -36,7 +36,7 @@ #include -static char const *const SLICE_CRR_HEADER = {"Slice-Crr-Status"}; +constexpr std::string_view SLICE_CRR_HEADER = {"Slice-Crr-Status"}; /** Designed to be a cheap throwaway struct which allows a diff --git a/plugins/experimental/slice/server.cc b/plugins/experimental/slice/server.cc index fa56f3aeb3c..a25376bc920 100644 --- a/plugins/experimental/slice/server.cc +++ b/plugins/experimental/slice/server.cc @@ -218,7 +218,7 @@ handleFirstServerHeader(Data *const data, TSCont const contp) data->m_bytessent = hbytes; TSVIOReenable(output_vio); - if (data->m_config->m_prefetchcount > 0 && header.hasKey(SLICE_CRR_HEADER, strlen(SLICE_CRR_HEADER))) { + if (data->m_config->m_prefetchcount > 0 && header.hasKey(SLICE_CRR_HEADER.data(), SLICE_CRR_HEADER.size())) { data->m_prefetchable = true; } diff --git a/plugins/experimental/slice/util.cc b/plugins/experimental/slice/util.cc index 97183716863..080d011488d 100644 --- a/plugins/experimental/slice/util.cc +++ b/plugins/experimental/slice/util.cc @@ -87,10 +87,10 @@ request_block(TSCont contp, Data *const data) return false; } - header.removeKey(SLICE_CRR_HEADER, strlen(SLICE_CRR_HEADER)); + header.removeKey(SLICE_CRR_HEADER.data(), SLICE_CRR_HEADER.size()); if (data->m_blocknum == 0 && data->m_config->m_prefetchcount > 0) { - std::string valStr = "-"; - header.setKeyVal(SLICE_CRR_HEADER, strlen(SLICE_CRR_HEADER), valStr.c_str(), valStr.length()); + std::string valStr = "1"; + header.setKeyVal(SLICE_CRR_HEADER.data(), SLICE_CRR_HEADER.size(), valStr.c_str(), valStr.length()); } // create virtual connection back into ATS From f12e0e6b7e1a232edadce1ae33d185c5a5c23937 Mon Sep 17 00:00:00 2001 From: Serris Lew Date: Wed, 27 Jul 2022 09:45:10 -0700 Subject: [PATCH 9/9] update header val type --- plugins/cache_range_requests/cache_range_requests.cc | 5 +++-- plugins/experimental/slice/HttpHeader.h | 1 + plugins/experimental/slice/util.cc | 3 +-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/plugins/cache_range_requests/cache_range_requests.cc b/plugins/cache_range_requests/cache_range_requests.cc index 36b81c6f4ec..46539f370ad 100644 --- a/plugins/cache_range_requests/cache_range_requests.cc +++ b/plugins/cache_range_requests/cache_range_requests.cc @@ -49,6 +49,7 @@ using parent_select_mode_t = enum parent_select_mode { constexpr std::string_view DefaultImsHeader = {"X-Crr-Ims"}; constexpr std::string_view SLICE_CRR_HEADER = {"Slice-Crr-Status"}; +constexpr std::string_view SLICE_CRR_VAL = "1"; struct pluginconfig { parent_select_mode_t ps_mode{PS_DEFAULT}; @@ -383,8 +384,8 @@ handle_client_send_response(TSHttpTxn txnp, txndata *const txn_state) remove_header(resp_buf, resp_loc, SLICE_CRR_HEADER.data(), SLICE_CRR_HEADER.size()); if (txn_state->slice_response) { - std::string valStr = "1"; - set_header(resp_buf, resp_loc, SLICE_CRR_HEADER.data(), SLICE_CRR_HEADER.size(), valStr.c_str(), valStr.length()); + set_header(resp_buf, resp_loc, SLICE_CRR_HEADER.data(), SLICE_CRR_HEADER.size(), SLICE_CRR_VAL.data(), + SLICE_CRR_VAL.size()); } } else { DEBUG_LOG("Ignoring status code %d; txn_state->origin_status=%d", status, txn_state->origin_status); diff --git a/plugins/experimental/slice/HttpHeader.h b/plugins/experimental/slice/HttpHeader.h index 9c4be14d760..81f1817fb73 100644 --- a/plugins/experimental/slice/HttpHeader.h +++ b/plugins/experimental/slice/HttpHeader.h @@ -37,6 +37,7 @@ #include constexpr std::string_view SLICE_CRR_HEADER = {"Slice-Crr-Status"}; +constexpr std::string_view SLICE_CRR_VAL = "1"; /** Designed to be a cheap throwaway struct which allows a diff --git a/plugins/experimental/slice/util.cc b/plugins/experimental/slice/util.cc index 080d011488d..f9c74ed1aea 100644 --- a/plugins/experimental/slice/util.cc +++ b/plugins/experimental/slice/util.cc @@ -89,8 +89,7 @@ request_block(TSCont contp, Data *const data) header.removeKey(SLICE_CRR_HEADER.data(), SLICE_CRR_HEADER.size()); if (data->m_blocknum == 0 && data->m_config->m_prefetchcount > 0) { - std::string valStr = "1"; - header.setKeyVal(SLICE_CRR_HEADER.data(), SLICE_CRR_HEADER.size(), valStr.c_str(), valStr.length()); + header.setKeyVal(SLICE_CRR_HEADER.data(), SLICE_CRR_HEADER.size(), SLICE_CRR_VAL.data(), SLICE_CRR_VAL.size()); } // create virtual connection back into ATS