[master] b5443cc8a Make VXID's 64 bit and VRT_INTEGER limited.
Poul-Henning Kamp
phk at FreeBSD.org
Mon Jan 23 11:45:13 UTC 2023
commit b5443cc8a2aa1550ca3e36766ce2d2ceba8a42a7
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Mon Jan 23 11:44:28 2023 +0000
Make VXID's 64 bit and VRT_INTEGER limited.
diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index a0e141a48..55a86960a 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -65,8 +65,8 @@ typedef struct vxids vxid_t;
#define NO_VXID ((struct vxids){0})
#define IS_NO_VXID(x) ((x).vxid == 0)
-#define VXID_TAG(x) ((x).vxid & (VSL_CLIENTMARKER|VSL_BACKENDMARKER))
-#define VXID(u) ((uintmax_t)(u.vxid) & VSL_IDENTMASK)
+#define VXID_TAG(x) ((uintmax_t)((x).vxid & (VSL_CLIENTMARKER|VSL_BACKENDMARKER)))
+#define VXID(u) ((uintmax_t)((u.vxid) & VSL_IDENTMASK))
#define IS_SAME_VXID(x, y) ((x).vxid == (y).vxid)
/*--------------------------------------------------------------------*/
@@ -705,10 +705,6 @@ extern const char H__Reason[];
// rfc7233,l,1207,1208
#define http_range_at(str, tok, l) http_tok_at(str, #tok, l)
-/* cache_main.c */
-vxid_t VXID_Get(const struct worker *, uint32_t marker);
-extern pthread_key_t witness_key;
-
/* cache_lck.c */
/* Internal functions, call only through macros below */
diff --git a/bin/varnishd/cache/cache_main.c b/bin/varnishd/cache/cache_main.c
index ae414bc01..370b8c741 100644
--- a/bin/varnishd/cache/cache_main.c
+++ b/bin/varnishd/cache/cache_main.c
@@ -184,7 +184,7 @@ static uint32_t vxid_chunk = 32768;
static struct lock vxid_lock;
vxid_t
-VXID_Get(const struct worker *wrk, uint32_t mask)
+VXID_Get(const struct worker *wrk, uint64_t mask)
{
struct vxid_pool *v;
vxid_t retval;
@@ -193,12 +193,12 @@ VXID_Get(const struct worker *wrk, uint32_t mask)
CHECK_OBJ_NOTNULL(wrk->wpriv, WORKER_PRIV_MAGIC);
v = wrk->wpriv->vxid_pool;
AZ(mask & VSL_IDENTMASK);
- while (v->count == 0 || v->next >= VSL_CLIENTMARKER) {
+ while (v->count == 0 || v->next >= VRT_INTEGER_MAX) {
Lck_Lock(&vxid_lock);
v->next = vxid_base;
v->count = vxid_chunk;
vxid_base += v->count;
- if (vxid_base >= VSL_CLIENTMARKER)
+ if (vxid_base >= VRT_INTEGER_MAX)
vxid_base = 1;
Lck_Unlock(&vxid_lock);
}
diff --git a/bin/varnishd/cache/cache_shmlog.c b/bin/varnishd/cache/cache_shmlog.c
index 7a217d6f8..b3b2e213d 100644
--- a/bin/varnishd/cache/cache_shmlog.c
+++ b/bin/varnishd/cache/cache_shmlog.c
@@ -143,7 +143,7 @@ vsl_hdr(enum VSL_tag_e tag, uint32_t *p, unsigned len, vxid_t vxid)
assert(tag < SLT__Reserved);
AZ(len & ~VSL_LENMASK);
- p[2] = 0;
+ p[2] = vxid.vxid >> 32;
p[1] = vxid.vxid;
p[0] = (((unsigned)tag & VSL_IDMASK) << VSL_IDSHIFT) |
(VSL_VERSION_3 << VSL_VERSHIFT) |
diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h
index 20077e182..e6dc7e697 100644
--- a/bin/varnishd/cache/cache_varnishd.h
+++ b/bin/varnishd/cache/cache_varnishd.h
@@ -302,6 +302,9 @@ uint16_t HTTP1_DissectResponse(struct http_conn *, struct http *resp,
unsigned HTTP1_Write(const struct worker *w, const struct http *hp, const int*);
/* cache_main.c */
+vxid_t VXID_Get(const struct worker *, uint64_t marker);
+extern pthread_key_t witness_key;
+
void THR_SetName(const char *name);
const char* THR_GetName(void);
void THR_SetBusyobj(const struct busyobj *);
diff --git a/bin/varnishtest/tests/c00122.vtc b/bin/varnishtest/tests/c00122.vtc
new file mode 100644
index 000000000..d624df224
--- /dev/null
+++ b/bin/varnishtest/tests/c00122.vtc
@@ -0,0 +1,107 @@
+varnishtest "test 64 bit vxid rollover"
+
+server s1 {
+ rxreq
+ txresp
+
+ accept
+ rxreq
+ delay 5
+ txresp
+} -start
+
+varnish v1 -vcl+backend {
+ sub vcl_backend_response {
+ if (bereq.url == "/retry" &&
+ bereq.retries == 0) {
+ return (retry);
+ }
+ }
+ sub vcl_deliver {
+ if (req.url == "/restart" &&
+ req.restarts == 0) {
+ return (restart);
+ }
+ }
+
+} -start
+
+process p1 -winsz 100 200 {exec varnishlog -n ${v1_name} -g raw} -start
+process p1 -expect-text 0 0 CLI
+
+varnish v1 -cliok "param.set debug +syncvsl"
+varnish v1 -cliok "debug.xid 999999999999998"
+
+logexpect l1 -v v1 -g request -T 2 {
+ expect 0 1 Begin "req 999999999999998"
+ expect * = ReqStart
+ expect 0 = ReqMethod GET
+ expect 0 = ReqURL /
+ expect 0 = ReqProtocol HTTP/1.1
+ expect * = ReqHeader "Foo: bar"
+ expect * = Link "bereq 2 fetch"
+ expect * = VSL "timeout"
+ expect * = End "synth"
+
+ expect 0 2 Begin "bereq 1"
+ expect * 2 Link "bereq 3 retry"
+ expect * = End
+
+ expect 0 3 Begin "bereq 2 retry"
+ expect * = End
+} -start
+
+client c1 {
+ txreq -url "/retry" -hdr "Foo: bar"
+ rxresp
+ expect resp.status == 200
+} -run
+
+varnish v1 -vsl_catchup
+
+logexpect l1 -wait
+process p1 -expect-text 0 1 "999999999999998 SessClose"
+process p1 -screen_dump
+
+
+################################################################################
+
+server s1 {
+ rxreq
+ txresp
+} -start
+
+varnish v1 -cliok "param.set debug +syncvsl"
+varnish v1 -cliok "debug.xid 999999999999997"
+
+logexpect l1 -v v1 -g request {
+ expect 0 999999999999998 Begin "req 999999999999997"
+ expect * = ReqStart
+ expect 0 = ReqMethod GET
+ expect 0 = ReqURL /
+ expect 0 = ReqProtocol HTTP/1.1
+ expect * = ReqHeader "Foo: bar"
+ expect * = Link "bereq 1 fetch"
+ expect * = Link "req 2 restart"
+ expect * = End
+
+ expect 0 1 Begin "bereq 999999999999998"
+ expect * = End
+
+ expect 0 2 Begin "req 999999999999998 restart"
+ expect * = End
+} -start
+
+client c1 {
+ txreq -url "/restart" -hdr "Foo: bar"
+ rxresp
+ expect resp.status == 200
+} -run
+
+varnish v1 -vsl_catchup
+logexpect l1 -wait
+
+process p1 -expect-text 0 1 "999999999999998 Link c req 2 restart"
+process p1 -expect-text 0 1 "999999999999997 SessClose"
+process p1 -screen_dump
+process p1 -stop
diff --git a/bin/varnishtest/tests/r01762.vtc b/bin/varnishtest/tests/r01762.vtc
index 5c5b39f2b..d0f96d9bb 100644
--- a/bin/varnishtest/tests/r01762.vtc
+++ b/bin/varnishtest/tests/r01762.vtc
@@ -1,5 +1,7 @@
varnishtest "test vsl api handling of incomplete vtxes combined with bad vxids"
+feature cmd false
+
server s1 {
rxreq
txresp
diff --git a/bin/varnishtest/vtc_logexp.c b/bin/varnishtest/vtc_logexp.c
index 88cb5049f..7eede8a7e 100644
--- a/bin/varnishtest/vtc_logexp.c
+++ b/bin/varnishtest/vtc_logexp.c
@@ -661,7 +661,7 @@ cmd_logexp_common(struct logexp *le, struct vtclog *vl,
else if (!strcmp(av[2], "="))
vxid = LE_LAST;
else {
- vxid = (int)strtoll(av[2], &end, 10);
+ vxid = strtoll(av[2], &end, 10);
if (*end != '\0' || vxid < 0)
vtc_fatal(vl, "Not a positive integer: '%s'", av[2]);
}
diff --git a/doc/changes.rst b/doc/changes.rst
index 3e47502bf..8d16a3988 100644
--- a/doc/changes.rst
+++ b/doc/changes.rst
@@ -35,6 +35,24 @@ release process.
Varnish Cache NEXT (2023-03-15)
===============================
+* VXIDs are 64 bit now and the binary format of SHM and raw saved
+ VSL files has changed as a consequence.
+
+ The actual valid range for VXIDs is [1…999999999999999], so it
+ fits in a VRT_INTEGER.
+
+ At one million cache-missing single request sessions per second
+ VXIDs will roll over in a little over ten years::
+
+ (1e15-1) / (3 * 1e6 * 86400 * 365) = 10.57
+
+ That should be enough for everybody™.
+
+ You can test if your downstream log-chewing pipeline handle the
+ larger VXIDs correctly using the CLI command::
+
+ ``debug.xid 20000000000``
+
* The ``debug.xid`` CLI command now sets the next XID to be used,
rather than "one less than the next XID to be used"
diff --git a/include/vapi/vsl_int.h b/include/vapi/vsl_int.h
index faeb9d111..a40158e04 100644
--- a/include/vapi/vsl_int.h
+++ b/include/vapi/vsl_int.h
@@ -62,9 +62,9 @@
* changing corresponding magic numbers in varnishd/cache/cache_shmlog.c
*/
-#define VSL_CLIENTMARKER (1U<<30)
-#define VSL_BACKENDMARKER (1U<<31)
-#define VSL_IDENTMASK (~(3U<<30))
+#define VSL_CLIENTMARKER (1ULL<<62)
+#define VSL_BACKENDMARKER (1ULL<<63)
+#define VSL_IDENTMASK ((1ULL<<51)-1)
#define VSL_LENMASK 0xffff
#define VSL_VERMASK 0x3
@@ -81,9 +81,10 @@
#define VSL_LEN(ptr) ((ptr)[0] & VSL_LENMASK)
#define VSL_VER(ptr) (((ptr)[0] & VSL_VERMASK) >> VSL_VERSHIFT)
#define VSL_TAG(ptr) ((enum VSL_tag_e)((ptr)[0] >> VSL_IDSHIFT))
-#define VSL_ID(ptr) (((ptr)[1]) & VSL_IDENTMASK)
-#define VSL_CLIENT(ptr) (((ptr)[1]) & VSL_CLIENTMARKER)
-#define VSL_BACKEND(ptr) (((ptr)[1]) & VSL_BACKENDMARKER)
+#define VSL_ID64(ptr) (((uint64_t)((ptr)[2])<<32) | ((ptr)[1]))
+#define VSL_ID(ptr) (VSL_ID64(ptr) & VSL_IDENTMASK)
+#define VSL_CLIENT(ptr) (((ptr)[2]) & (VSL_CLIENTMARKER >> 32))
+#define VSL_BACKEND(ptr) (((ptr)[2]) & (VSL_BACKENDMARKER >> 32))
#define VSL_DATA(ptr) ((char*)((ptr)+VSL_OVERHEAD))
#define VSL_CDATA(ptr) ((const char*)((ptr)+VSL_OVERHEAD))
#define VSL_BATCHLEN(ptr) ((ptr)[1])
diff --git a/include/vrt.h b/include/vrt.h
index e65e59737..e9ca4f087 100644
--- a/include/vrt.h
+++ b/include/vrt.h
@@ -58,6 +58,7 @@
* binary/load-time compatible, increment MAJOR version
*
* NEXT (2023-03-15)
+ * VXID is 64 bit
* [cache.h] http_GetRange() changed
* 16.0 (2022-09-15)
* VMOD C-prototypes moved into JSON
diff --git a/lib/libvarnishapi/vsl_dispatch.c b/lib/libvarnishapi/vsl_dispatch.c
index 34b8af199..3e990fb36 100644
--- a/lib/libvarnishapi/vsl_dispatch.c
+++ b/lib/libvarnishapi/vsl_dispatch.c
@@ -1033,6 +1033,7 @@ vtx_synth_rec(struct vtx *vtx, unsigned tag, const char *fmt, ...)
va_list ap;
char *buf;
int l, buflen;
+ uint64_t vxid;
ALLOC_OBJ(synth, SYNTH_MAGIC);
AN(synth);
@@ -1046,18 +1047,21 @@ vtx_synth_rec(struct vtx *vtx, unsigned tag, const char *fmt, ...)
if (l > buflen - 1)
l = buflen - 1;
buf[l++] = '\0'; /* NUL-terminated */
- synth->data[1] = vtx->key.vxid;
+ vxid = vtx->key.vxid;
switch (vtx->type) {
case VSL_t_req:
- synth->data[1] |= VSL_CLIENTMARKER;
+ vxid |= VSL_CLIENTMARKER;
break;
case VSL_t_bereq:
- synth->data[1] |= VSL_BACKENDMARKER;
+ vxid |= VSL_BACKENDMARKER;
break;
default:
break;
}
- synth->data[0] = (((tag & 0xff) << 24) | l);
+ synth->data[2] = vxid >> 32;
+ synth->data[1] = vxid;
+ synth->data[0] = (((tag & VSL_IDMASK) << VSL_IDSHIFT) |
+ (VSL_VERSION_3 << VSL_VERSHIFT) | l);
synth->offset = vtx->c.offset;
VTAILQ_FOREACH_REVERSE(it, &vtx->synth, synthhead, list) {
diff --git a/vmod/vmod_vtc.c b/vmod/vmod_vtc.c
index aa1c2b9ad..6c38db3ae 100644
--- a/vmod/vmod_vtc.c
+++ b/vmod/vmod_vtc.c
@@ -422,7 +422,7 @@ vmod_vsl(VRT_CTX, VCL_INT id, VCL_STRING tag_s, VCL_ENUM side, VCL_STRANDS s)
return;
}
- if (id < 0 || id > VSL_IDENTMASK) {
+ if (id < 0 || id > VRT_INTEGER_MAX) {
VRT_fail(ctx, "id out of bounds");
return;
}
More information about the varnish-commit
mailing list