Hi Patrick,
On Sun, Aug 11, 2013 at 03:45:36PM -0400, Patrick Hemmer wrote:
> I'm using the %rt field in the "unique-id-format" config parameter (the
> full value is "%{+X}o%pid-%rt"), and am getting lots of duplicates. In
> one specific case, haproxy added the same http_request_counter value to
> 70 different http requests within a span of 61 seconds (from various
> client hosts too). Does the http_request_counter only increment under
> certain conditions, or is this a bug?
Wow, congrats, you found a nice ugly bug! Here's how the counter is
retrieved at the moment of logging :
iret = snprintf(tmplog, dst + maxsize - tmplog, "%04X", global.req_count);
As you can see, it uses a global variable which holds the global number of
requests seen at the moment of logging (or assigning the header) instead of
a unique value assigned to each request!
So all the requests that are logged in the same time frame between two
new requests get the same ID :-(
The counter should be auto-incrementing so that each retrieval is unique.
Please try with the attached patch.
Thanks,
Willy
>From 9f09521f2d2deacfb4b1b10b23eb5525b9941c62 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w...@1wt.eu>
Date: Tue, 13 Aug 2013 17:51:07 +0200
Subject: BUG/MEDIUM: unique_id: HTTP request counter must be unique!
The HTTP request counter is incremented non atomically, which means that
many requests can log the same ID. Let's increment it when it is consumed
so that we avoid this case.
This bug was reported by Patrick Hemmer. It's 1.5-specific and does not
need to be backported.
---
include/types/global.h | 2 +-
src/log.c | 4 ++--
src/proto_http.c | 2 --
3 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/include/types/global.h b/include/types/global.h
index 41cd67f..cfc3d23 100644
--- a/include/types/global.h
+++ b/include/types/global.h
@@ -90,7 +90,7 @@ struct global {
int rlimit_memmax; /* default ulimit-d in megs value : 0=unset */
long maxzlibmem; /* max RAM for zlib in bytes */
int mode;
- unsigned int req_count; /* HTTP request counter */
+ unsigned int req_count; /* HTTP request counter for logs and unique_id
*/
int last_checks;
int spread_checks;
char *chroot;
diff --git a/src/log.c b/src/log.c
index 8f8fd8f..369dc34 100644
--- a/src/log.c
+++ b/src/log.c
@@ -1448,13 +1448,13 @@ int build_logline(struct session *s, char *dst, size_t
maxsize, struct list *lis
case LOG_FMT_COUNTER: // %rt
if (tmp->options & LOG_OPT_HEXA) {
- iret = snprintf(tmplog, dst + maxsize -
tmplog, "%04X", global.req_count);
+ iret = snprintf(tmplog, dst + maxsize -
tmplog, "%04X", global.req_count++);
if (iret < 0 || iret > dst + maxsize -
tmplog)
goto out;
last_isspace = 0;
tmplog += iret;
} else {
- ret = ltoa_o(global.req_count, tmplog,
dst + maxsize - tmplog);
+ ret = ltoa_o(global.req_count++,
tmplog, dst + maxsize - tmplog);
if (ret == NULL)
goto out;
tmplog = ret;
diff --git a/src/proto_http.c b/src/proto_http.c
index 3ef6472..8d6eaf5 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -8289,8 +8289,6 @@ void http_init_txn(struct session *s)
txn->flags = 0;
txn->status = -1;
- global.req_count++;
-
txn->cookie_first_date = 0;
txn->cookie_last_date = 0;
--
1.7.12.2.21.g234cd45.dirty
