You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2013/09/09 21:39:57 UTC
[1/3] git commit: TS-2188 Never consider a simple / path as a
healthcheck
Updated Branches:
refs/heads/master a27b9e73a -> f1bffdf25
TS-2188 Never consider a simple / path as a healthcheck
This is an optimization, and a safety net, such that a simple
/ path can never be considered for a healthcheck.
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/9e82415f
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/9e82415f
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/9e82415f
Branch: refs/heads/master
Commit: 9e82415fc2c40a628589f4053ad1103c5730bdb8
Parents: 49d00ff
Author: Scott Harris <sc...@harrisnet.id.au>
Authored: Mon Sep 9 13:36:22 2013 -0600
Committer: Leif Hedstrom <zw...@apache.org>
Committed: Mon Sep 9 13:37:28 2013 -0600
----------------------------------------------------------------------
CHANGES | 3 +++
plugins/experimental/healthchecks/README | 4 +++-
plugins/experimental/healthchecks/healthchecks.c | 4 ++++
3 files changed, 10 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9e82415f/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 878b55b..6734ed8 100644
--- a/CHANGES
+++ b/CHANGES
@@ -3,6 +3,9 @@ Changes with Apache Traffic Server 4.1.0
*) [TS-2191] not reschedule http_sm when the sm_list`s lock is not acquired.
+ *) [TS-2188] Fixes to make healthcheck plugin not segfault, and parse
+ the log files properly. Author: Scott Harris <sc...@harrisnet.id.au>.
+
*) [TS-1086] Avoid edge case returning 304 to an unconditional request.
Diagnosis and patch by Mohamad Khateeb.
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9e82415f/plugins/experimental/healthchecks/README
----------------------------------------------------------------------
diff --git a/plugins/experimental/healthchecks/README b/plugins/experimental/healthchecks/README
index 920dbe7..ab156ac 100644
--- a/plugins/experimental/healthchecks/README
+++ b/plugins/experimental/healthchecks/README
@@ -6,8 +6,10 @@ This configuration contains one, or several, lines of the format
<URI-path> <file-path> <mime> <file-exists-code> <file-missing-code>
+The URI-path can *not* be "/" only.
-For example,
+
+Examples:
/__hc /var/run/ts-alive text/plain 200 403
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9e82415f/plugins/experimental/healthchecks/healthchecks.c
----------------------------------------------------------------------
diff --git a/plugins/experimental/healthchecks/healthchecks.c b/plugins/experimental/healthchecks/healthchecks.c
index 47b4f7b..c7ae166 100644
--- a/plugins/experimental/healthchecks/healthchecks.c
+++ b/plugins/experimental/healthchecks/healthchecks.c
@@ -500,6 +500,10 @@ health_check_origin(TSCont contp ATS_UNUSED, TSEvent event ATS_UNUSED, void *eda
int path_len = 0;
const char* path = TSUrlPathGet(reqp, url_loc, &path_len);
+ /* Short circuit the / path, common case, and we won't allow healthecks on / */
+ if (!path || !path_len)
+ goto cleanup;
+
while (info) {
if (info->p_len == path_len && !memcmp(info->path, path, path_len)) {
TSDebug(PLUGIN_NAME, "Found match for /%.*s", path_len, path);
[3/3] git commit: Updated text / format.
Posted by zw...@apache.org.
Updated text / format.
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/f1bffdf2
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/f1bffdf2
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/f1bffdf2
Branch: refs/heads/master
Commit: f1bffdf25d30c6c4d96c6ebc49c7e059f77e9ed1
Parents: 9e82415
Author: Leif Hedstrom <zw...@apache.org>
Authored: Mon Sep 9 13:39:50 2013 -0600
Committer: Leif Hedstrom <zw...@apache.org>
Committed: Mon Sep 9 13:39:50 2013 -0600
----------------------------------------------------------------------
CHANGES | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/f1bffdf2/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 6734ed8..f63e647 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,10 +1,12 @@
-*- coding: utf-8 -*-
Changes with Apache Traffic Server 4.1.0
- *) [TS-2191] not reschedule http_sm when the sm_list`s lock is not acquired.
+ *) [TS-2191] Do not reschedule http_sm when the sm_list`s lock is not
+ acquired. This can lose items for the WebUI, which will be fixed in a
+ separate bug.
- *) [TS-2188] Fixes to make healthcheck plugin not segfault, and parse
- the log files properly. Author: Scott Harris <sc...@harrisnet.id.au>.
+ *) [TS-2188] Fixes to make healthcheck plugin not segfault, and parse the
+ log files properly. Author: Scott Harris <sc...@harrisnet.id.au>.
*) [TS-1086] Avoid edge case returning 304 to an unconditional request.
Diagnosis and patch by Mohamad Khateeb.
Re: [1/3] git commit: TS-2188 Never consider a simple / path as a healthcheck
Posted by Leif Hedstrom <zw...@apache.org>.
On Sep 10, 2013, at 6:54 AM, Igor Galić <i....@brainsware.org> wrote:
>
>> + /* Short circuit the / path, common case, and we won't allow healthecks
>> on / */
>> + if (!path || !path_len)
>
> shouldn't this error out before going to cleanup?
No, it's not an error case, just an "optimization" such that on requests for "/", we don't even try to match up against possible health check URLs. The original bug reported used this as a bug fix, but that only hides the real bug (which is fixed in a separate commit).
-- leif
Re: [1/3] git commit: TS-2188 Never consider a simple / path as a
healthcheck
Posted by Igor Galić <i....@brainsware.org>.
----- Original Message -----
> Updated Branches:
> refs/heads/master a27b9e73a -> f1bffdf25
>
>
> TS-2188 Never consider a simple / path as a healthcheck
>
> This is an optimization, and a safety net, such that a simple
> / path can never be considered for a healthcheck.
>
>
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/9e82415f
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/9e82415f
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/9e82415f
>
> Branch: refs/heads/master
> Commit: 9e82415fc2c40a628589f4053ad1103c5730bdb8
> Parents: 49d00ff
> Author: Scott Harris <sc...@harrisnet.id.au>
> Authored: Mon Sep 9 13:36:22 2013 -0600
> Committer: Leif Hedstrom <zw...@apache.org>
> Committed: Mon Sep 9 13:37:28 2013 -0600
>
> ----------------------------------------------------------------------
> CHANGES | 3 +++
> plugins/experimental/healthchecks/README | 4 +++-
> plugins/experimental/healthchecks/healthchecks.c | 4 ++++
> 3 files changed, 10 insertions(+), 1 deletion(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9e82415f/CHANGES
> ----------------------------------------------------------------------
> diff --git a/CHANGES b/CHANGES
> index 878b55b..6734ed8 100644
> --- a/CHANGES
> +++ b/CHANGES
> @@ -3,6 +3,9 @@ Changes with Apache Traffic Server 4.1.0
>
> *) [TS-2191] not reschedule http_sm when the sm_list`s lock is not
> acquired.
>
> + *) [TS-2188] Fixes to make healthcheck plugin not segfault, and parse
> + the log files properly. Author: Scott Harris <sc...@harrisnet.id.au>.
> +
> *) [TS-1086] Avoid edge case returning 304 to an unconditional request.
> Diagnosis and patch by Mohamad Khateeb.
>
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9e82415f/plugins/experimental/healthchecks/README
> ----------------------------------------------------------------------
> diff --git a/plugins/experimental/healthchecks/README
> b/plugins/experimental/healthchecks/README
> index 920dbe7..ab156ac 100644
> --- a/plugins/experimental/healthchecks/README
> +++ b/plugins/experimental/healthchecks/README
> @@ -6,8 +6,10 @@ This configuration contains one, or several, lines of the
> format
>
> <URI-path> <file-path> <mime> <file-exists-code> <file-missing-code>
>
> +The URI-path can *not* be "/" only.
>
> -For example,
> +
> +Examples:
>
> /__hc /var/run/ts-alive text/plain 200 403
>
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9e82415f/plugins/experimental/healthchecks/healthchecks.c
> ----------------------------------------------------------------------
> diff --git a/plugins/experimental/healthchecks/healthchecks.c
> b/plugins/experimental/healthchecks/healthchecks.c
> index 47b4f7b..c7ae166 100644
> --- a/plugins/experimental/healthchecks/healthchecks.c
> +++ b/plugins/experimental/healthchecks/healthchecks.c
> @@ -500,6 +500,10 @@ health_check_origin(TSCont contp ATS_UNUSED, TSEvent
> event ATS_UNUSED, void *eda
> int path_len = 0;
> const char* path = TSUrlPathGet(reqp, url_loc, &path_len);
>
> + /* Short circuit the / path, common case, and we won't allow healthecks
> on / */
> + if (!path || !path_len)
shouldn't this error out before going to cleanup?
> + goto cleanup;
> +
> while (info) {
> if (info->p_len == path_len && !memcmp(info->path, path, path_len)) {
> TSDebug(PLUGIN_NAME, "Found match for /%.*s", path_len, path);
>
>
--
Igor Galić
Tel: +43 (0) 664 886 22 883
Mail: i.galic@brainsware.org
URL: http://brainsware.org/
GPG: 6880 4155 74BD FD7C B515 2EA5 4B1D 9E08 A097 C9AE
[2/3] git commit: TS-2188 healthcheck plugin doesn't parse config
files properly
Posted by zw...@apache.org.
TS-2188 healthcheck plugin doesn't parse config files properly
This is part of the fixes to the healthcheck plugins, such that
we don't segfault. This part of the fix actually makes sure we
properly parse the configuration file as was expected.
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/49d00ff1
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/49d00ff1
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/49d00ff1
Branch: refs/heads/master
Commit: 49d00ff1fe2d8097952aa857d1b4d79103825043
Parents: a27b9e7
Author: Leif Hedstrom <zw...@apache.org>
Authored: Mon Sep 9 13:29:28 2013 -0600
Committer: Leif Hedstrom <zw...@apache.org>
Committed: Mon Sep 9 13:37:28 2013 -0600
----------------------------------------------------------------------
.../experimental/healthchecks/healthchecks.c | 42 +++++++++++++-------
1 file changed, 27 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/49d00ff1/plugins/experimental/healthchecks/healthchecks.c
----------------------------------------------------------------------
diff --git a/plugins/experimental/healthchecks/healthchecks.c b/plugins/experimental/healthchecks/healthchecks.c
index 3c550b7..47b4f7b 100644
--- a/plugins/experimental/healthchecks/healthchecks.c
+++ b/plugins/experimental/healthchecks/healthchecks.c
@@ -312,14 +312,11 @@ parse_configs(const char* fname)
int state = 0;
char *ok=NULL, *miss=NULL, *mime=NULL;
- prev_finfo = finfo;
- finfo = TSmalloc(sizeof(HCFileInfo));
- memset(finfo, 0, sizeof(HCFileInfo));
-
- if (NULL == head_finfo)
- head_finfo = finfo;
- if (prev_finfo)
- prev_finfo->_next = finfo;
+ /* Allocate a new config-info, if we don't have one already */
+ if (!finfo) {
+ finfo = TSmalloc(sizeof(HCFileInfo));
+ memset(finfo, 0, sizeof(HCFileInfo));
+ }
if (fgets(buf, sizeof(buf) - 1, fd)) {
str = strtok_r(buf, SEPARATORS, &save);
@@ -353,12 +350,25 @@ parse_configs(const char* fname)
str = strtok_r(NULL, SEPARATORS, &save);
}
- finfo->ok = gen_header(ok, mime, &finfo->o_len);
- finfo->miss = gen_header(miss, mime, &finfo->m_len);
- finfo->data = TSmalloc(sizeof(HCFileData));
- memset(finfo->data, 0, sizeof(HCFileData));
- reload_status_file(finfo, finfo->data);
- TSDebug(PLUGIN_NAME, "Parsed: %s %s %s %s %s", finfo->path, finfo->fname, mime, ok, miss);
+ /* Fill in the info if everything was ok */
+ if (state > 4) {
+ TSDebug(PLUGIN_NAME, "Parsed: %s %s %s %s %s", finfo->path, finfo->fname, mime, ok, miss);
+ finfo->ok = gen_header(ok, mime, &finfo->o_len);
+ finfo->miss = gen_header(miss, mime, &finfo->m_len);
+ finfo->data = TSmalloc(sizeof(HCFileData));
+ memset(finfo->data, 0, sizeof(HCFileData));
+ reload_status_file(finfo, finfo->data);
+
+ /* Add it the linked list */
+ TSDebug(PLUGIN_NAME, "Adding path=%s to linked list", finfo->path);
+ if (NULL == head_finfo) {
+ head_finfo = finfo;
+ } else {
+ prev_finfo->_next = finfo;
+ }
+ prev_finfo = finfo;
+ finfo = NULL; /* We used this one up, get a new one next iteration */
+ }
}
}
fclose(fd);
@@ -491,8 +501,10 @@ health_check_origin(TSCont contp ATS_UNUSED, TSEvent event ATS_UNUSED, void *eda
const char* path = TSUrlPathGet(reqp, url_loc, &path_len);
while (info) {
- if (info->p_len == path_len && !memcmp(info->path, path, path_len))
+ if (info->p_len == path_len && !memcmp(info->path, path, path_len)) {
+ TSDebug(PLUGIN_NAME, "Found match for /%.*s", path_len, path);
break;
+ }
info = info->_next;
}