Elukey has uploaded a new change for review.
https://gerrit.wikimedia.org/r/291870
Change subject: Clarify the use of TAG_F_NOVARMATCH within the context of
%{<strftime-format>}t.
......................................................................
Clarify the use of TAG_F_NOVARMATCH within the context of %{<strftime-format>}t.
While investigating some Timestamp inconsistencies between the old and new
Varnishkafka,
I had to spend a bit of time trying to figure out the semantic of
TAG_F_NOVARMATCH. Its
behavior was leading to a bullet proof result with the Varnish 3 tags since all
the
timestamps had a separate and specific VSL tag, meanwhile Varnish 4 offers a
more
generic SLT_Timestamp that carries the request phase (for example Start, Resp,
etc..)
in its payload (together with various timings). The solution that we currently
have is
working fine but it is not robust since it forces Varnishkafka to pick the
first timestamp
encountered with TAG_F_NOVARMATCH, not a specific one that we set. With the
current configuration,
namely grouping by Client request and discarding backend tags it works fine
because the
"Start" timestamp is indeed the first one encountered, but it might need to be
changed in
the future. I added a lot of comments and removed a confusing field in
format_parse to
facilitate the work of whoever will refactor the code in the future. I don't
see the use
case at the moment to add more code and fix this for good, but I'd really like
to get
everybody's opinion to discuss what is best.
Bug: T136314
Change-Id: Icbeb5dede1cefe23a99755a68c2f0820c6c9f3e5
---
M varnishkafka.c
1 file changed, 35 insertions(+), 3 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/operations/software/varnish/varnishkafka
refs/changes/70/291870/1
diff --git a/varnishkafka.c b/varnishkafka.c
index 88371da..7dc8541 100644
--- a/varnishkafka.c
+++ b/varnishkafka.c
@@ -845,10 +845,18 @@
.tag_flags = TAG_F_LAST }
} },
['t'] = { {
- /* Time the request was received */
+ /* Time the request was received.
+ * TAG_F_NOVARMATCH forces varnishkafka to avoid
+ * any sort of var comparison so the .var field
is
+ * not usable for this option. This flag is
needed
+ * since 'var' in %{<strftime-format>}t gets
+ * the '<strftime-format>' value, that is not
+ * usable for tag match operations.
+ * The only option available is:
+ * - Start timestamp (TAG_F_NOVARMATCH needs to
be set)
+ */
{ VSL_CLIENTMARKER, SLT_Timestamp,
.col = 2,
- .var = "Start",
.parser = parse_t,
.tag_flags = TAG_F_NOVARMATCH }
} },
@@ -1473,13 +1481,37 @@
if (!(tag->spec & spec))
continue;
+ /*
+ * TAG_F_NOVARMATCH is a special flag used for format tags like
+ * %{<strftime-format>}t because the related tag->var value
+ * is not usable.
+ * The only use case at the moment is represented by the
SLT_Timestamp
+ * tag that occurs multiple times during the request processing.
+ * The format is always the same, for example:
+ * Start: 1464628135.916033 0.000885 0.000084
+ * TAG_F_NOVARMATCH avoids the match between the tag 't' var
+ * specified in format_parse and the content of ptr. This means
that
+ * at the moment the only possible matche for 't' is:
+ * - Start timestamp if only TAG_F_NOVARMATCH is set.
+ * This feature is not really flexible and it would need to be
improved
+ * in the future to let the users to specify the desired
Timestamp in
+ * their config files, rather than forcing it in the code.
+ */
if ((tag->var) && !(tag->flags & TAG_F_NOVARMATCH)) {
const char *t;
- /* Variable match ("Varname: value") */
+ /* Get the occurence of ":" in ("Varname: value") */
if (!(t = strnchr(ptr, len, ':')))
continue;
+ /*
+ * Variable match ("Varname: value") checks:
+ * 1) the len of the substring before the ':' (Varname)
needs
+ * to match the len of the tag requested.
+ * 2) strncasecmp between ptr (up to tag->varlen chars)
and
+ * the current tag candidate for the match
+ * must be 0 (so equal strings).
+ */
if (tag->varlen != (int)(t-ptr) ||
strncasecmp(ptr, tag->var, tag->varlen))
continue;
--
To view, visit https://gerrit.wikimedia.org/r/291870
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icbeb5dede1cefe23a99755a68c2f0820c6c9f3e5
Gerrit-PatchSet: 1
Gerrit-Project: operations/software/varnish/varnishkafka
Gerrit-Branch: master
Gerrit-Owner: Elukey <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
