You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by ez...@apache.org on 2019/04/26 19:01:33 UTC
[trafficserver] branch master updated: Add slice plugin config
options to either pace or disable block stitch error logs.
This is an automated email from the ASF dual-hosted git repository.
eze 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 b42fb60 Add slice plugin config options to either pace or disable block stitch error logs.
b42fb60 is described below
commit b42fb6000d5b226d237fe40d8fbdbb683e393881
Author: Brian Olsen <br...@comcast.com>
AuthorDate: Mon Apr 22 12:52:47 2019 +0000
Add slice plugin config options to either pace or disable block stitch error logs.
---
doc/admin-guide/plugins/slice.en.rst | 91 +++++++---
plugins/experimental/slice/Config.cc | 188 +++++++++++++--------
plugins/experimental/slice/Config.h | 26 +--
plugins/experimental/slice/Data.h | 9 +-
plugins/experimental/slice/Makefile.inc | 6 +-
plugins/experimental/slice/README.md | 75 +++++++-
plugins/experimental/slice/client.cc | 8 +-
plugins/experimental/slice/server.cc | 61 ++++---
plugins/experimental/slice/slice.cc | 11 +-
plugins/experimental/slice/slice.h | 4 +-
.../experimental/slice/unit-tests/test_config.cc | 22 ++-
tests/gold_tests/pluginTest/slice/slice.test.py | 2 +-
.../pluginTest/slice/slice_error.test.py | 4 +-
13 files changed, 340 insertions(+), 167 deletions(-)
diff --git a/doc/admin-guide/plugins/slice.en.rst b/doc/admin-guide/plugins/slice.en.rst
index 86ad1a6..58cc610 100644
--- a/doc/admin-guide/plugins/slice.en.rst
+++ b/doc/admin-guide/plugins/slice.en.rst
@@ -61,39 +61,87 @@ In this case, the plugin will use the default behaviour:
Plugin Options
--------------
-Slice block sizes can specified using the blockbytes parameter::
+The slice plugin supports the following options::
- @plugin=slice.so @pparam=blockbytes:1000000 @cache_range_requests.so
+ --blockbytes=<bytes> (optional)
+ Default is 1m or 1048576 bytes
+ -b <bytes> for short.
+ Suffix k,m,g supported
+ Limited to 32k and 32m inclusive.
-In adition to bytes, 'k', 'm' and 'g' suffixes may be used for
-kilobytes, megabytes and gigabytes::
+ --test-blockbytes=<bytes> (optional)
+ Suffix k,m,g supported
+ -t <bytes> for short.
+ Limited to any positive number.
+ Ignored if --blockbytes provided.
- @plugin=slice.so @pparam=blockbytes:5m @cache_range_requests.so
- @plugin=slice.so @pparam=blockbytes:512k @cache_range_requests.so
- @plugin=slice.so @pparam=blockbytes:32m @cache_range_requests.so
+ --pace-errorlog=<seconds> (optional)
+ Limit stitching error logs to every 'n' second(s)
-paramater ``blockbytes`` is checked to be between 32kb and 32mb
-inclusive.
+ --disable-errorlog (optional)
+ Disable writing block stitch errors to the error log.
-For testing and extreme purposes the parameter ``bytesover`` may
+Examples::
+
+ @plugin=slice.so @pparam=--blockbytes=1000000 @plugin=cache_range_requests.so
+
+Or alternatively::
+
+ @plugin=slice.so @pparam=-b @pparam=1000000 @plugin=cache_range_requests.so
+
+Byte suffix examples::
+
+ slice.so --blockbytes=5m
+ slice.so -b 512k
+ slice.so --blockbytes=32m
+
+For testing and extreme purposes the parameter ``test-blockbytes`` may
be used instead which is unchecked::
- @plugin=slice.so @pparam=bytesover:1G @cache_range_requests.so
- @plugin=slice.so @pparam=bytesover:13 @cache_range_requests.so
+ slice.so --test-blockbytes=1G
+ slice.so -t 13
+
+Because the slice plugin is susceptible to errors during block stitching
+extra logs related to stitching are written to ``diags.log``. Worst case
+an error log entry could be generated for every transaction. The
+following options are provided to help with log overrun::
+
+ slice.so --pace-errorlog=5
+ slice.so -p 1
+ slice.so --disable-errorlog
After modifying :file:`remap.config`, restart or reload traffic server
(sudo traffic_ctl config reload) or (sudo traffic_ctl server restart)
to activate the new configuration values.
+Debug Options
+-------------
+
+While the current slice plugin is able to detect block consistency
+errors during the block stitching process, it can only abort the
+client connection. A CDN can only "fix" these by issuing an appropriate
+content revalidation.
+
+Under normal logging these slice block errors tend to show up as::
+
+ pscl value 0
+ crc value ERR_READ_ERROR
+
+By default more detailed stitching errors are written to ``diags.log``.
+An example is as follows::
+
+[Apr 19 20:26:13.639] [ET_NET 17] ERROR: [slice] 1555705573.639 reason="Non 206 internal block response" uri="http://localhost:18080/%7Ep.tex/%7Es.50M/%7Eui.20000/" uas="curl/7.29.0" req_range="bytes=1000000-" norm_range="bytes 1000000-52428799/52428800" etag_exp="%221603934496%22" lm_exp="Fri, 19 Apr 2019 18:53:20 GMT" blk_range="21000000-21999999" status_got="400" cr_got="" etag_got="" lm_got="" cc="no-store" via=""
+
+Whether or how often these detailed log entries are written are
+configurable plugin options.
+
Implementation Notes
====================
-This slice plugin is by no means a best solution for adding
-blocking support to ATS.
-
-The slice plugin as is designed to provide a basic capability to block
-requests for arbitrary range requests and for blocking large assets for
-ease of caching.
+This slice plugin is a stop gap plugin for handling special cases
+involving very large assets that may be range requested. Hopefully
+the slice plugin is deprecated in the future when partial object
+caching is finally implemented.
Slice *ONLY* handles slicing up requests into blocks, it delegates
actual caching and fetching to the cache_range_requests.so plugin.
@@ -133,7 +181,7 @@ the block fetch to ensure the block is cached.
Important Notes
===============
-This plugin also assumes that the content requested is cacheable.
+This plugin assumes that the content requested is cacheable.
Any first block server response that is not a 206 is passed directly
down to the client. If that response is a '200' only the first
@@ -145,7 +193,7 @@ remove these headers.
The only 416 response that this plugin handles itself is if the
requested range is inside the last slice block but past the end of
-the asset contents. Other 416 responses are handled by the parents.
+the asset contents. Other 416 responses are handled by the parent.
If a client aborts mid transaction the current slice block continues to
be read from the server until it is complete to ensure that the block
@@ -166,5 +214,4 @@ check for the existence of any headers they may have created the
first time they have were visited.
Since the Slice plugin is written as an intercept handler it loses the
-ability to use state machine hooks and transaction states.
-
+ability to use normal state machine hooks and transaction states.
diff --git a/plugins/experimental/slice/Config.cc b/plugins/experimental/slice/Config.cc
index 27ec5aa..43d1a7f 100644
--- a/plugins/experimental/slice/Config.cc
+++ b/plugins/experimental/slice/Config.cc
@@ -18,22 +18,23 @@
#include "Config.h"
-#include <algorithm>
#include <cctype>
#include <cinttypes>
-#include <map>
-#include <string>
+#include <cstdlib>
+#include <getopt.h>
+#include <string_view>
+
+#include "ts/experimental.h"
int64_t
-Config::bytesFrom(std::string const &valstr)
+Config::bytesFrom(char const *const valstr)
{
- char const *const nptr = valstr.c_str();
- char *endptr = nullptr;
- int64_t blockbytes = strtoll(nptr, &endptr, 10);
+ char *endptr = nullptr;
+ int64_t blockbytes = strtoll(valstr, &endptr, 10);
- if (nullptr != endptr && nptr < endptr) {
- size_t const dist = endptr - nptr;
- if (dist < valstr.size() && 0 <= blockbytes) {
+ if (nullptr != endptr && valstr < endptr) {
+ size_t const dist = endptr - valstr;
+ if (dist < strlen(valstr) && 0 <= blockbytes) {
switch (tolower(*endptr)) {
case 'g':
blockbytes *= ((int64_t)1024 * (int64_t)1024 * (int64_t)1024);
@@ -58,96 +59,135 @@ Config::bytesFrom(std::string const &valstr)
}
bool
-Config::fromArgs(int const argc, char const *const argv[], char *const errbuf, int const errbuf_size)
+Config::fromArgs(int const argc, char const *const argv[])
{
-#if !defined(SLICE_UNIT_TEST)
DEBUG_LOG("Number of arguments: %d", argc);
for (int index = 0; index < argc; ++index) {
DEBUG_LOG("args[%d] = %s", index, argv[index]);
}
-#endif
- std::map<std::string, std::string> keyvals;
+ // current "best" blockbytes from configuration
+ int64_t blockbytes = 0;
- static std::string const bbstr(blockbytesstr);
- static std::string const bostr(bytesoverstr);
-
- // collect all args
+ // backwards compat: look for blockbytes
for (int index = 0; index < argc; ++index) {
- std::string const argstr = argv[index];
+ std::string_view const argstr = argv[index];
std::size_t const spos = argstr.find_first_of(':');
- if (spos != std::string::npos) {
- std::string key = argstr.substr(0, spos);
- std::string val = argstr.substr(spos + 1);
-
- if (!key.empty()) {
- std::for_each(key.begin(), key.end(), [](char &ch) { ch = tolower(ch); });
-
- // blockbytes and bytesover collide
- if (bbstr == key) {
- keyvals.erase(bostr);
- } else if (bostr == key) {
- keyvals.erase(bbstr);
- }
+ if (spos != std::string_view::npos) {
+ std::string_view const key = argstr.substr(0, spos);
+ std::string_view const val = argstr.substr(spos + 1);
- keyvals[std::move(key)] = std::move(val);
+ if (!key.empty() && !val.empty()) {
+ char const *const valstr = val.data(); // inherits argv's null
+ int64_t const bytesread = bytesFrom(valstr);
+
+ if (blockbytesmin <= bytesread && bytesread <= blockbytesmax) {
+ DEBUG_LOG("Found deprecated blockbytes %" PRId64, bytesread);
+ blockbytes = bytesread;
+ }
}
}
}
- std::map<std::string, std::string>::const_iterator itfind;
+ // standard parsing
+ constexpr const struct option longopts[] = {
+ {const_cast<char *>("blockbytes"), required_argument, nullptr, 'b'},
+ {const_cast<char *>("test-blockbytes"), required_argument, nullptr, 't'},
+ {const_cast<char *>("pace-errorlog"), required_argument, nullptr, 'p'},
+ {const_cast<char *>("disable-errorlog"), no_argument, nullptr, 'd'},
+ {nullptr, 0, nullptr, 0},
+ };
+
+ // getopt assumes args start at '1' so this hack is needed
+ char *const *argvp = ((char *const *)argv - 1);
+
+ for (;;) {
+ int const opt = getopt_long(argc + 1, argvp, "b:t:p:d", longopts, nullptr);
+ if (-1 == opt) {
+ break;
+ }
- // blockbytes checked range string
- itfind = keyvals.find(bbstr);
- if (keyvals.end() != itfind) {
- std::string val = itfind->second;
- if (!val.empty()) {
- int64_t const blockbytes = bytesFrom(val);
+ DEBUG_LOG("processing '%c' %s", (char)opt, argvp[optind - 1]);
- if (blockbytes < blockbytesmin || blockbytesmax < blockbytes) {
-#if !defined(SLICE_UNIT_TEST)
- DEBUG_LOG("Block Bytes %" PRId64 " outside checked limits %" PRId64 "-%" PRId64, blockbytes, blockbytesmin, blockbytesmax);
- DEBUG_LOG("Block Bytes kept at %" PRId64, m_blockbytes);
-#endif
+ switch (opt) {
+ case 'b': {
+ int64_t const bytesread = bytesFrom(optarg);
+ if (blockbytesmin <= bytesread && bytesread <= blockbytesmax) {
+ DEBUG_LOG("Using blockbytes %" PRId64, bytesread);
+ blockbytes = bytesread;
} else {
-#if !defined(SLICE_UNIT_TEST)
- DEBUG_LOG("Block Bytes set to %" PRId64, blockbytes);
-#endif
- m_blockbytes = blockbytes;
+ ERROR_LOG("Invalid blockbytes: %s", optarg);
+ }
+ } break;
+ case 't':
+ if (0 == blockbytes) {
+ int64_t const bytesread = bytesFrom(optarg);
+ if (0 < bytesread) {
+ DEBUG_LOG("Using blockbytestest %" PRId64, bytesread);
+ blockbytes = bytesread;
+ } else {
+ ERROR_LOG("Invalid blockbytestest: %s", optarg);
+ }
+ } else {
+ DEBUG_LOG("Skipping blockbytestest in favor of blockbytes");
}
+ break;
+ case 'p': {
+ int const secsread = atoi(optarg);
+ if (0 < secsread) {
+ m_paceerrsecs = std::min(secsread, 60);
+ } else {
+ DEBUG_LOG("Ignoring pace-errlog argument");
+ }
+ } break;
+ case 'd':
+ m_paceerrsecs = -1;
+ break;
+ default:
+ break;
}
+ }
- keyvals.erase(itfind);
+ if (0 < blockbytes) {
+ DEBUG_LOG("Using configured blockbytes %" PRId64, blockbytes);
+ m_blockbytes = blockbytes;
+ } else {
+ DEBUG_LOG("Using default blockbytes %" PRId64, m_blockbytes);
}
- // bytesover unchecked range string
- itfind = keyvals.find(bostr);
- if (keyvals.end() != itfind) {
- std::string val = itfind->second;
- if (!val.empty()) {
- int64_t const bytesover = bytesFrom(val);
-
- if (bytesover <= 0) {
-#if !defined(SLICE_UNIT_TEST)
- DEBUG_LOG("Bytes Over %" PRId64 " <= 0", bytesover);
- DEBUG_LOG("Block Bytes kept at %" PRId64, m_blockbytes);
-#endif
- } else {
-#if !defined(SLICE_UNIT_TEST)
- DEBUG_LOG("Block Bytes set to %" PRId64, bytesover);
-#endif
- m_blockbytes = bytesover;
- }
- }
- keyvals.erase(itfind);
+ if (m_paceerrsecs < 0) {
+ DEBUG_LOG("Block stitching error logs disabled");
+ } else if (0 == m_paceerrsecs) {
+ DEBUG_LOG("Block stitching error logs enabled");
+ } else {
+ DEBUG_LOG("Block stitching error logs at most every %d sec(s)", m_paceerrsecs);
}
- for (std::map<std::string, std::string>::const_iterator itkv(keyvals.cbegin()); keyvals.cend() != itkv; ++itkv) {
-#if !defined(SLICE_UNIT_TEST)
- ERROR_LOG("Unhandled pparam %s", itkv->first.c_str());
-#endif
+ return true;
+}
+
+bool
+Config::canLogError()
+{
+ std::lock_guard<std::mutex> const guard(m_mutex);
+
+ if (m_paceerrsecs < 0) {
+ return false;
+ } else if (0 == m_paceerrsecs) {
+ return true;
}
+#if !defined(UNITTEST)
+ TSHRTime const timenow = TShrtime();
+ if (timenow < m_nextlogtime) {
+ return false;
+ }
+
+ m_nextlogtime = timenow + TS_HRTIME_SECONDS(m_paceerrsecs);
+#else
+ m_nextlogtime = 0; // thanks clang
+#endif
+
return true;
}
diff --git a/plugins/experimental/slice/Config.h b/plugins/experimental/slice/Config.h
index e107f3f..8c4ab24 100644
--- a/plugins/experimental/slice/Config.h
+++ b/plugins/experimental/slice/Config.h
@@ -20,21 +20,27 @@
#include "slice.h"
-#include <string>
+#include <mutex>
// Data Structures and Classes
struct Config {
- static int64_t const blockbytesmin = 1024 * 256; // 256KB
- static int64_t const blockbytesmax = 1024 * 1024 * 32; // 32MB
- static int64_t const blockbytesdefault = 1024 * 1024; // 1MB
-
- static constexpr char const *const blockbytesstr = "blockbytes";
- static constexpr char const *const bytesoverstr = "bytesover";
+ static constexpr int64_t const blockbytesmin = 1024 * 256; // 256KB
+ static constexpr int64_t const blockbytesmax = 1024 * 1024 * 32; // 32MB
+ static constexpr int64_t const blockbytesdefault = 1024 * 1024; // 1MB
int64_t m_blockbytes{blockbytesdefault};
+ int m_paceerrsecs{0}; // -1 disable logging, 0 no pacing, max 60s
+
+ // Convert optarg to bytes
+ static int64_t bytesFrom(char const *const valstr);
+
+ // Parse from args, ast one wins
+ bool fromArgs(int const argc, char const *const argv[]);
- // Last one wins
- bool fromArgs(int const argc, char const *const argv[], char *const errbuf, int const errbuf_size);
+ // Check if the error should can be logged, if sucessful may update m_nexttime
+ bool canLogError();
- static int64_t bytesFrom(std::string const &valstr);
+private:
+ TSHRTime m_nextlogtime{0}; // next time to log in ns
+ std::mutex m_mutex;
};
diff --git a/plugins/experimental/slice/Data.h b/plugins/experimental/slice/Data.h
index f770719..4538352 100644
--- a/plugins/experimental/slice/Data.h
+++ b/plugins/experimental/slice/Data.h
@@ -20,10 +20,10 @@
#include "ts/ts.h"
+#include "Config.h"
#include "HttpHeader.h"
#include "Range.h"
#include "Stage.h"
-#include "slice.h"
#include <netinet/in.h>
@@ -35,7 +35,8 @@ struct Data {
Data(Data const &) = delete;
Data &operator=(Data const &) = delete;
- int64_t const m_blockbytes_config; // configured slice block size
+ Config *const m_config;
+
sockaddr_storage m_client_ip;
// for pristine url coming in
@@ -76,8 +77,8 @@ struct Data {
TSHttpParser m_http_parser{nullptr}; //!< cached for reuse
- explicit Data(int64_t const blockbytes)
- : m_blockbytes_config(blockbytes),
+ explicit Data(Config *const config)
+ : m_config(config),
m_client_ip(),
m_urlbuffer(nullptr),
m_urlloc(nullptr),
diff --git a/plugins/experimental/slice/Makefile.inc b/plugins/experimental/slice/Makefile.inc
index 2f0f8f4..dbac02b 100644
--- a/plugins/experimental/slice/Makefile.inc
+++ b/plugins/experimental/slice/Makefile.inc
@@ -43,21 +43,21 @@ experimental_slice_slice_la_SOURCES = \
check_PROGRAMS += experimental/slice/test_content_range
-experimental_slice_test_content_range_CPPFLAGS = $(AM_CPPFLAGS) -I$(abs_top_srcdir)/tests/include -DSLICE_UNIT_TEST
+experimental_slice_test_content_range_CPPFLAGS = $(AM_CPPFLAGS) -I$(abs_top_srcdir)/tests/include -DUNITTEST
experimental_slice_test_content_range_SOURCES = \
experimental/slice/unit-tests/test_content_range.cc \
experimental/slice/ContentRange.cc
check_PROGRAMS += experimental/slice/test_range
-experimental_slice_test_range_CPPFLAGS = $(AM_CPPFLAGS) -I$(abs_top_srcdir)/tests/include -DSLICE_UNIT_TEST
+experimental_slice_test_range_CPPFLAGS = $(AM_CPPFLAGS) -I$(abs_top_srcdir)/tests/include -DUNITTEST
experimental_slice_test_range_SOURCES = \
experimental/slice/unit-tests/test_range.cc \
experimental/slice/Range.cc
check_PROGRAMS += experimental/slice/test_config
-experimental_slice_test_config_CPPFLAGS = $(AM_CPPFLAGS) -I$(abs_top_srcdir)/tests/include -DSLICE_UNIT_TEST
+experimental_slice_test_config_CPPFLAGS = $(AM_CPPFLAGS) -I$(abs_top_srcdir)/tests/include -DUNITTEST
experimental_slice_test_config_SOURCES = \
experimental/slice/unit-tests/test_config.cc \
experimental/slice/Config.cc
diff --git a/plugins/experimental/slice/README.md b/plugins/experimental/slice/README.md
index dc56997..ec3d1bc 100644
--- a/plugins/experimental/slice/README.md
+++ b/plugins/experimental/slice/README.md
@@ -18,24 +18,86 @@ To enable the plugin, specify the plugin library via @plugin at the end
of a remap line as follows (2MB slice in this example):
```
-map http://ats-cache/ http://parent/ @plugin=slice.so @pparam=blockbytes:2097152 @plugin=cache_range_requests.so
+map http://ats-cache/ http://parent/ @plugin=slice.so @pparam=--blockbytes=2M @plugin=cache_range_requests.so
+```
+
+alternatively
+
+```
+map http://ats-cache/ http://parent/ @plugin=slice.so @pparam=-b @pparam=2M @plugin=cache_range_requests.so
```
for global plugins.
```
-slice.so blockbytes:2097152
+slice.so --blockbytes=2097152
cache_range_requests.so
```
-**Note**: cache_range_requests **MUST** follow slice.so Put these plugins at the end of the plugin list
-**Note**: blockbytes is defined in bytes. 1048576 (1MB) is the default.
+alternatively:
+
+```
+slice.so -b 2M
+cache_range_requests.so
+```
+
+Options for the slice plugin (typically last one wins):
+```
+--blockbytes=<number bytes> (optional)
+ Slice block size.
+ Default is 1m or 1048576 bytes.
+ also -b <num bytes>
+ Suffix k,m,g supported.
+ Limited to 32k and 32m inclusive.
+ For backwards compatibility blockbytes:<num bytes> is also supported.
+
+--test-blockbytes=<number bytes> (optional)
+ Slice block size for testing.
+ also -t <num bytes>
+ Suffix k,m,g supported.
+ Limited to any positive number.
+ Ignored if --blockbytes is provided.
+
+--pace-errorlog=<second(s)> (optional)
+ Limit stitching error logs to every 'n' second(s)
+ Default is to log all errors (no pacing).
+ also -p <seconds>
+
+--disable-errorlog (optional)
+ Disable writing stitching errors to the error log.
+ also -d
+```
+
+**Note**: cache_range_requests **MUST** follow slice.so Put these plugins
+at the end of the plugin list
+
+**Note**: blockbytes is defined in bytes. Postfix for 'K', 'M' and 'G'
+may be used. 1048576 (1MB) is the default.
For testing purposes an unchecked value of "bytesover" is also available.
-Debug output can be enable by setting the debug tag: **slice**
+Debug output can be enable by setting the debug tag: **slice**. If debug
+is enabled all block stitch errors will log to diags.log
+
+The slice plugin is susceptible to block stitching errors caused by
+mismatched blocks. For these cases special detailed error logs are
+provided to help with debugging. Below is a sample error log entry::
-Debug messages related to object instance construction/deconstruction, see slice.h.
+```
+[Apr 19 20:26:13.639] [ET_NET 17] ERROR: [slice] 1555705573.639 reason="Non 206 internal block response" uri="http://localhost:18080/%7Ep.tex/%7Es.50M/%7Eui.20000/" uas="curl/7.29.0" req_range="bytes=1000000-" norm_range="bytes 1000000-52428799/52428800" etag_exp="%221603934496%22" lm_exp="Fri, 19 Apr 2019 18:53:20 GMT" blk_range="21000000-21999999" status_got="400" cr_got="" etag_got="" lm_got="" cc="no-store" via=""
+```
+
+Current error types logged:
+```
+ Mismatch block Etag
+ Mismatch block Last-Modified
+ Non 206 internal block response
+ Mismatch/Bad block Content-Range
+```
+
+
+With slice error logs disabled these type errors can typically be detected
+by observing crc=ERR_READ_ERROR and pscl=0 in normal logs.
At the current time only single range requests or the first part of a
multi part range request of the forms:
@@ -46,7 +108,6 @@ Range: bytes=-<last N bytes>
```
are supported as multi part range responses are non trivial to implement.
This matches with the cache_range_requests.so plugin capability.
-
---
Important things to note:
diff --git a/plugins/experimental/slice/client.cc b/plugins/experimental/slice/client.cc
index 75bbe8e..7076f6d 100644
--- a/plugins/experimental/slice/client.cc
+++ b/plugins/experimental/slice/client.cc
@@ -34,8 +34,8 @@ shutdown(TSCont const contp, Data *const data)
bool
requestBlock(TSCont contp, Data *const data)
{
- int64_t const blockbeg = (data->m_blockbytes_config * data->m_blocknum);
- Range blockbe(blockbeg, blockbeg + data->m_blockbytes_config);
+ int64_t const blockbeg = (data->m_config->m_blockbytes * data->m_blocknum);
+ Range blockbe(blockbeg, blockbeg + data->m_config->m_blockbytes);
char rangestr[1024];
int rangelen = sizeof(rangestr);
@@ -131,7 +131,7 @@ handle_client_req(TSCont contp, TSEvent event, Data *const data)
data->m_statustype = TS_HTTP_STATUS_REQUESTED_RANGE_NOT_SATISFIABLE;
// First block will give Content-Length
- rangebe = Range(0, data->m_blockbytes_config);
+ rangebe = Range(0, data->m_config->m_blockbytes);
}
} else {
DEBUG_LOG("Full content request");
@@ -143,7 +143,7 @@ handle_client_req(TSCont contp, TSEvent event, Data *const data)
}
// set to the first block in range
- data->m_blocknum = rangebe.firstBlockFor(data->m_blockbytes_config);
+ data->m_blocknum = rangebe.firstBlockFor(data->m_config->m_blockbytes);
data->m_req_range = rangebe;
// remove ATS keys to avoid 404 loop
diff --git a/plugins/experimental/slice/server.cc b/plugins/experimental/slice/server.cc
index 9371469..37016de 100644
--- a/plugins/experimental/slice/server.cc
+++ b/plugins/experimental/slice/server.cc
@@ -186,6 +186,15 @@ handleFirstServerHeader(Data *const data, TSCont const contp)
void
logSliceError(char const *const message, Data const *const data, HttpHeader const &header_resp)
{
+ Config *const config = data->m_config;
+
+ bool const logToError = config->canLogError();
+
+ // always write block stitch errors while in debug mode
+ if (!logToError && !TSIsDebugTagSet(PLUGIN_NAME)) {
+ return;
+ }
+
HttpHeader const header_req(data->m_req_hdrmgr.m_buffer, data->m_req_hdrmgr.m_lochdr);
TSHRTime const timenowus = TShrtime();
@@ -222,8 +231,8 @@ logSliceError(char const *const message, Data const *const data, HttpHeader cons
crange.toStringClosed(normstr, &normlen);
// block range request
- int64_t const blockbeg = data->m_blocknum * data->m_blockbytes_config;
- int64_t const blockend = std::min(blockbeg + data->m_blockbytes_config, data->m_contentlen);
+ int64_t const blockbeg = data->m_blocknum * data->m_config->m_blockbytes;
+ int64_t const blockend = std::min(blockbeg + data->m_config->m_blockbytes, data->m_contentlen);
// Block response data
TSHttpStatus const statusgot = header_resp.status();
@@ -261,26 +270,28 @@ logSliceError(char const *const message, Data const *const data, HttpHeader cons
size_t etaggotlen = sizeof(etaggotstr);
TSStringPercentEncode(etagstr, etaglen, etaggotstr, etaggotlen, &etaggotlen, nullptr);
- TSError("[%s] %" PRId64 ".%" PRId64 " reason=\"%s\""
- " uri=\"%.*s\""
- " uas=\"%.*s\""
- " req_range=\"%.*s\""
- " norm_range=\"%.*s\""
-
- " etag_exp=\"%.*s\""
- " lm_exp=\"%.*s\""
-
- " blk_range=\"%" PRId64 "-%" PRId64 "\""
-
- " status_got=\"%d\""
- " cr_got=\"%.*s\""
- " etag_got=\"%.*s\""
- " lm_got=\"%.*s\""
- " cc=\"%.*s\""
- " via=\"%.*s\"",
- PLUGIN_NAME, secs, ms, message, (int)urlplen, urlpstr, uaslen, uasstr, rangelen, rangestr, normlen, normstr,
- (int)etagexplen, etagexpstr, data->m_lastmodifiedlen, data->m_lastmodified, blockbeg, blockend - 1, statusgot, crlen,
- crstr, (int)etaggotlen, etaggotstr, lmlen, lmstr, cclen, ccstr, vialen, viastr);
+ DEBUG_LOG("Logging Block Stitch error");
+
+ ERROR_LOG("%" PRId64 ".%" PRId64 " reason=\"%s\""
+ " uri=\"%.*s\""
+ " uas=\"%.*s\""
+ " req_range=\"%.*s\""
+ " norm_range=\"%.*s\""
+
+ " etag_exp=\"%.*s\""
+ " lm_exp=\"%.*s\""
+
+ " blk_range=\"%" PRId64 "-%" PRId64 "\""
+
+ " status_got=\"%d\""
+ " cr_got=\"%.*s\""
+ " etag_got=\"%.*s\""
+ " lm_got=\"%.*s\""
+ " cc=\"%.*s\""
+ " via=\"%.*s\"",
+ secs, ms, message, (int)urlplen, urlpstr, uaslen, uasstr, rangelen, rangestr, normlen, normstr, (int)etagexplen,
+ etagexpstr, data->m_lastmodifiedlen, data->m_lastmodified, blockbeg, blockend - 1, statusgot, crlen, crstr,
+ (int)etaggotlen, etaggotstr, lmlen, lmstr, cclen, ccstr, vialen, viastr);
}
bool
@@ -378,7 +389,7 @@ handle_server_resp(TSCont contp, TSEvent event, Data *const data)
}
// how much to fast forward into this data block
- data->m_blockskip = data->m_req_range.skipBytesForBlock(data->m_blockbytes_config, data->m_blocknum);
+ data->m_blockskip = data->m_req_range.skipBytesForBlock(data->m_config->m_blockbytes, data->m_blocknum);
}
transfer_content_bytes(data);
@@ -424,13 +435,13 @@ handle_server_resp(TSCont contp, TSEvent event, Data *const data)
// issues a speculative request for the first block
// in that case fast forward to the real first in range block
// Btw this isn't implemented yet, to be handled
- int64_t const firstblock(data->m_req_range.firstBlockFor(data->m_blockbytes_config));
+ int64_t const firstblock(data->m_req_range.firstBlockFor(data->m_config->m_blockbytes));
if (data->m_blocknum < firstblock) {
data->m_blocknum = firstblock;
}
// done processing blocks?
- if (!data->m_req_range.blockIsInside(data->m_blockbytes_config, data->m_blocknum)) {
+ if (!data->m_req_range.blockIsInside(data->m_config->m_blockbytes, data->m_blocknum)) {
data->m_blocknum = -1; // signal value no more blocks
}
} else {
diff --git a/plugins/experimental/slice/slice.cc b/plugins/experimental/slice/slice.cc
index b0c0e1a..aab8b7d 100644
--- a/plugins/experimental/slice/slice.cc
+++ b/plugins/experimental/slice/slice.cc
@@ -33,7 +33,7 @@ namespace
Config globalConfig;
bool
-read_request(TSHttpTxn txnp, Config const *const config)
+read_request(TSHttpTxn txnp, Config *const config)
{
DEBUG_LOG("slice read_request");
TxnHdrMgr hdrmgr;
@@ -55,7 +55,8 @@ read_request(TSHttpTxn txnp, Config const *const config)
return false;
}
- Data *const data = new Data(config->m_blockbytes);
+ TSAssert(nullptr != config);
+ Data *const data = new Data(config);
// set up feedback connect
if (AF_INET == ip->sa_family) {
@@ -150,11 +151,11 @@ TSRemapOSResponse(void *ih, TSHttpTxn rh, int os_response_type)
SLICE_EXPORT
TSReturnCode
-TSRemapNewInstance(int argc, char *argv[], void **ih, char *errbuf, int errbuf_size)
+TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf */, int /* errbuf_size */)
{
Config *const config = new Config;
if (2 < argc) {
- config->fromArgs(argc - 2, argv + 2, errbuf, errbuf_size);
+ config->fromArgs(argc - 2, argv + 2);
}
*ih = static_cast<void *>(config);
return TS_SUCCESS;
@@ -195,7 +196,7 @@ TSPluginInit(int argc, char const *argv[])
}
if (1 < argc) {
- globalConfig.fromArgs(argc - 1, argv + 1, nullptr, 0);
+ globalConfig.fromArgs(argc - 1, argv + 1);
}
TSCont const contp(TSContCreate(global_read_request_hook, nullptr));
diff --git a/plugins/experimental/slice/slice.h b/plugins/experimental/slice/slice.h
index 0cb2523..682c016 100644
--- a/plugins/experimental/slice/slice.h
+++ b/plugins/experimental/slice/slice.h
@@ -30,10 +30,9 @@
#define PLUGIN_NAME "slice"
#endif
-#define __FILENAME__ (strrchr(__FILE__, '/') ? strrchr(__FILE__, '/') + 1 : __FILE__)
-
#if !defined(UNITTEST)
+#define __FILENAME__ (strrchr(__FILE__, '/') ? strrchr(__FILE__, '/') + 1 : __FILE__)
#define DEBUG_LOG(fmt, ...) \
TSDebug(PLUGIN_NAME, "[%s:%04d] %s(): " fmt, __FILENAME__, __LINE__, __func__, \
##__VA_ARGS__) /* \
@@ -48,6 +47,7 @@
TSDebug(PLUGIN_NAME, "[%s:%04d] %s(): " fmt, __FILENAME__, __LINE__, __func__, ##__VA_ARGS__)
#else
+
#define DEBUG_LOG(fmt, ...)
#define ERROR_LOG(fmt, ...)
diff --git a/plugins/experimental/slice/unit-tests/test_config.cc b/plugins/experimental/slice/unit-tests/test_config.cc
index e7467cc..fcd1b8e 100644
--- a/plugins/experimental/slice/unit-tests/test_config.cc
+++ b/plugins/experimental/slice/unit-tests/test_config.cc
@@ -25,6 +25,8 @@
#include "../Config.h"
#include "catch.hpp" /* catch unit-test framework */
+#include <array>
+
TEST_CASE("config default", "[AWS][slice][utility]")
{
Config const config;
@@ -34,9 +36,13 @@ TEST_CASE("config default", "[AWS][slice][utility]")
TEST_CASE("config bytesfrom valid parsing", "[AWS][slice][utility]")
{
- std::vector<std::string> const teststrings = {"1000", "1m", "5g", "2k", "3kb", "1z"};
+ static std::array<std::string, 6> const teststrings = {
+ "1000", "1m", "5g", "2k", "3kb", "1z",
+ };
- std::vector<int64_t> const expvals = {1000, 1024 * 1024, int64_t(1024) * 1024 * 1024 * 5, 1024 * 2, 1024 * 3, 1};
+ constexpr std::array<int64_t, 6> const expvals = {
+ 1000, 1024 * 1024, int64_t(1024) * 1024 * 1024 * 5, 1024 * 2, 1024 * 3, 1,
+ };
for (size_t index = 0; index < teststrings.size(); ++index) {
std::string const &teststr = teststrings[index];
@@ -52,12 +58,12 @@ TEST_CASE("config bytesfrom valid parsing", "[AWS][slice][utility]")
TEST_CASE("config bytesfrom invalid parsing", "[AWS][slice][utility]")
{
- std::vector<std::string> const badstrings = {
- "abc", // alpha
- "g00", // giga
- "M00", // mega
- "k00", // kilo
- "-500" // negative
+ static std::array<std::string, 5> const badstrings = {
+ "abc", // alpha
+ "g00", // giga
+ "M00", // mega
+ "k00", // kilo
+ "-500", // negative
};
for (std::string const &badstr : badstrings) {
diff --git a/tests/gold_tests/pluginTest/slice/slice.test.py b/tests/gold_tests/pluginTest/slice/slice.test.py
index a3e299a..8b57d48 100644
--- a/tests/gold_tests/pluginTest/slice/slice.test.py
+++ b/tests/gold_tests/pluginTest/slice/slice.test.py
@@ -117,7 +117,7 @@ tr = Test.AddTestRun("Load Slice plugin")
remap_config_path = ts.Disk.remap_config.Name
tr.Disk.File(remap_config_path, typename="ats:config").AddLines([
'map / http://127.0.0.1:{}'.format(server.Variables.Port) +
- ' @plugin=slice.so @pparam=bytesover:{}'.format(block_bytes)
+ ' @plugin=slice.so @pparam=--test-blockbytes={}'.format(block_bytes)
])
tr.StillRunningAfter = ts
diff --git a/tests/gold_tests/pluginTest/slice/slice_error.test.py b/tests/gold_tests/pluginTest/slice/slice_error.test.py
index f6fd4d5..53cf194 100644
--- a/tests/gold_tests/pluginTest/slice/slice_error.test.py
+++ b/tests/gold_tests/pluginTest/slice/slice_error.test.py
@@ -245,12 +245,12 @@ curl_and_args = 'sh curlsort.sh -H "Host: www.example.com"'
# set up whole asset fetch into cache
ts.Disk.remap_config.AddLine(
'map / http://127.0.0.1:{}'.format(server.Variables.Port) +
- ' @plugin=slice.so @pparam=bytesover:{}'.format(blockbytes)
+ ' @plugin=slice.so @pparam=--test-blockbytes={}'.format(blockbytes)
)
# minimal configuration
ts.Disk.records_config.update({
- 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.enabled': 0,
'proxy.config.diags.debug.tags': 'slice',
'proxy.config.http.cache.http': 0,
'proxy.config.http.wait_for_cache': 0,