[master] 7649bb1ae shard: use a configuration context per instance
Nils Goroll
nils.goroll at uplex.de
Fri Oct 9 14:48:07 UTC 2020
commit 7649bb1aed54cbb6c2a7e19f7a2b0e201e33ae61
Author: Nils Goroll <nils.goroll at uplex.de>
Date: Fri Oct 9 16:22:16 2020 +0200
shard: use a configuration context per instance
The shard director had the limitation to only support adding/removing
backends of one instance before `.reconfigure()` on that instance had
to be called, then the next instance could be changed.
This limitation came from the time when it was still unclear that
calling `VRT_priv_task()` on a per-object pointer was the accepted
interface for per-instance privs (varnish historians' recommended read:
https://github.com/varnishcache/varnish-cache/wiki/VIP1:-PRIV_*-visibility-and-lifetime-control
)
It is about time to lift that limitation now. o/ Dridi
diff --git a/bin/varnishtest/tests/d00015.vtc b/bin/varnishtest/tests/d00015.vtc
index 5050cff45..37899b829 100644
--- a/bin/varnishtest/tests/d00015.vtc
+++ b/bin/varnishtest/tests/d00015.vtc
@@ -107,6 +107,7 @@ varnish v1 -vcl+backend {
vd.add_backend(s3, "2");
vd.add_backend(s3, "3");
vd2.clear();
+ vd2.add_backend(s3, "4");
vd.add_backend(s3, "4");
vd.add_backend(s3, "5");
vd.add_backend(s3, "6");
@@ -241,7 +242,6 @@ logexpect l1 -v v1 -g raw -d 1 {
expect 0 0 Debug {^shard:.*point = 6e040182, host = 0}
expect 0 0 VCL_Log {^-- re-add some - no 2nd director$}
- expect 0 0 Error {^shard vd2: cannot change more than}
expect 0 0 Debug {^shard:.*point = 3d1fe97, host = 3}
expect 0 0 Debug {^shard:.*point = a25a43b, host = 6}
expect 0 0 Debug {^shard:.*point = 2b20d9a2, host = 1}
diff --git a/bin/varnishtest/tests/d00016.vtc b/bin/varnishtest/tests/d00016.vtc
index ac0bb7644..5452e0908 100644
--- a/bin/varnishtest/tests/d00016.vtc
+++ b/bin/varnishtest/tests/d00016.vtc
@@ -110,6 +110,7 @@ varnish v1 -vcl+backend {
vd.add_backend(s3, "2");
vd.add_backend(s3, "3");
vd2.clear();
+ vd2.add_backend(s3, "4");
vd.add_backend(s3, "4");
vd.add_backend(s3, "5");
vd.add_backend(s3, "6");
@@ -169,7 +170,6 @@ logexpect l1 -v v1 -g raw -d 1 {
expect 0 1001 VCL_Log {^-- remove s1_2 specifically$}
expect 0 1001 VCL_Log {^-- remove all instances of s1$}
expect 0 1001 VCL_Log {^-- re-add some - no 2nd director$}
- expect 0 1001 Error {^shard vd2: cannot change more than}
expect 0 1001 VCL_Log {^-- remove second-last$}
expect 0 1001 VCL_Log {^-- remove last$}
expect 0 1001 VCL_Log {^-- END$}
diff --git a/doc/changes.rst b/doc/changes.rst
index b2c192e43..a2ab4f8b1 100644
--- a/doc/changes.rst
+++ b/doc/changes.rst
@@ -37,6 +37,10 @@ Varnish Cache Next (2021-03-15)
set headers are now validated to contain only characters allowed by
RFC7230. A (runtime) VCL failure is triggered if not.
+* The shard director now supports reconfiguration (adding/removing
+ backends) of several instances without any special ordering
+ requirement.
+
================================
Varnish Cache 6.5.1 (2020-09-25)
================================
diff --git a/lib/libvmod_directors/shard_cfg.c b/lib/libvmod_directors/shard_cfg.c
index 82c51fd16..5b71ffd82 100644
--- a/lib/libvmod_directors/shard_cfg.c
+++ b/lib/libvmod_directors/shard_cfg.c
@@ -80,27 +80,22 @@ struct backend_reconfig {
*
* for backend reconfiguration, we create a change list on the VCL workspace in
* a PRIV_TASK state, which we work in reconfigure.
- *
- * for now, we allow to only reconfigure one shard director at a time.
*/
static struct shard_change *
-shard_change_get(VRT_CTX, struct vmod_priv *priv,
- const struct sharddir * const shardd)
+shard_change_get(VRT_CTX, const struct sharddir * const shardd)
{
+ struct vmod_priv *task;
struct shard_change *change;
+ const void *id = (const char *)shardd + task_off_cfg;
- if (priv->priv) {
- CAST_OBJ_NOTNULL(change, priv->priv, SHARD_CHANGE_MAGIC);
- if (change->shardd == NULL) {
- change->shardd = shardd;
- VSTAILQ_INIT(&change->tasks);
- } else if (change->shardd != shardd) {
- shard_err0(ctx, shardd,
- "cannot change more than one shard director "
- "at a time");
- return (NULL);
- }
+ CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+
+ task = VRT_priv_task(ctx, id);
+
+ if (task->priv != NULL) {
+ CAST_OBJ_NOTNULL(change, task->priv, SHARD_CHANGE_MAGIC);
+ assert (change->shardd == shardd);
return (change);
}
@@ -114,7 +109,7 @@ shard_change_get(VRT_CTX, struct vmod_priv *priv,
change->space = NULL;
change->shardd = shardd;
VSTAILQ_INIT(&change->tasks);
- priv->priv = change;
+ task->priv = change;
return (change);
}
@@ -124,7 +119,6 @@ shard_change_finish(struct shard_change *change)
{
CHECK_OBJ_NOTNULL(change, SHARD_CHANGE_MAGIC);
- change->shardd = NULL;
VSTAILQ_INIT(&change->tasks);
}
@@ -150,8 +144,7 @@ shard_change_task_add(VRT_CTX, struct shard_change *change,
}
static inline struct shard_change_task *
-shard_change_task_backend(VRT_CTX,
- struct vmod_priv *priv, const struct sharddir *shardd,
+shard_change_task_backend(VRT_CTX, const struct sharddir *shardd,
enum shard_change_task_e task_e, VCL_BACKEND be, VCL_STRING ident,
VCL_DURATION rampup)
{
@@ -161,7 +154,7 @@ shard_change_task_backend(VRT_CTX,
CHECK_OBJ_NOTNULL(shardd, SHARDDIR_MAGIC);
assert(task_e == ADD_BE || task_e == REMOVE_BE);
- change = shard_change_get(ctx, priv, shardd);
+ change = shard_change_get(ctx, shardd);
if (change == NULL)
return (NULL);
@@ -183,16 +176,15 @@ shard_change_task_backend(VRT_CTX,
* director reconfiguration tasks
*/
VCL_BOOL
-shardcfg_add_backend(VRT_CTX, struct vmod_priv *priv,
- const struct sharddir *shardd, VCL_BACKEND be, VCL_STRING ident,
- VCL_DURATION rampup, VCL_REAL weight)
+shardcfg_add_backend(VRT_CTX, const struct sharddir *shardd,
+ VCL_BACKEND be, VCL_STRING ident, VCL_DURATION rampup, VCL_REAL weight)
{
struct shard_change_task *task;
assert (weight >= 1);
AN(be);
- task = shard_change_task_backend(ctx, priv, shardd, ADD_BE,
+ task = shard_change_task_backend(ctx, shardd, ADD_BE,
be, ident, rampup);
if (task == NULL)
@@ -203,21 +195,21 @@ shardcfg_add_backend(VRT_CTX, struct vmod_priv *priv,
}
VCL_BOOL
-shardcfg_remove_backend(VRT_CTX, struct vmod_priv *priv,
- const struct sharddir *shardd, VCL_BACKEND be, VCL_STRING ident)
+shardcfg_remove_backend(VRT_CTX, const struct sharddir *shardd,
+ VCL_BACKEND be, VCL_STRING ident)
{
- return (shard_change_task_backend(ctx, priv, shardd, REMOVE_BE,
+ return (shard_change_task_backend(ctx, shardd, REMOVE_BE,
be, ident, 0) != NULL);
}
VCL_BOOL
-shardcfg_clear(VRT_CTX, struct vmod_priv *priv, const struct sharddir *shardd)
+shardcfg_clear(VRT_CTX, const struct sharddir *shardd)
{
struct shard_change *change;
CHECK_OBJ_NOTNULL(shardd, SHARDDIR_MAGIC);
- change = shard_change_get(ctx, priv, shardd);
+ change = shard_change_get(ctx, shardd);
if (change == NULL)
return (0);
@@ -609,8 +601,7 @@ shardcfg_apply_change(VRT_CTX, struct sharddir *shardd,
*/
VCL_BOOL
-shardcfg_reconfigure(VRT_CTX, struct vmod_priv *priv,
- struct sharddir *shardd, VCL_INT replicas)
+shardcfg_reconfigure(VRT_CTX, struct sharddir *shardd, VCL_INT replicas)
{
struct shard_change *change;
@@ -621,7 +612,7 @@ shardcfg_reconfigure(VRT_CTX, struct vmod_priv *priv,
return (0);
}
- change = shard_change_get(ctx, priv, shardd);
+ change = shard_change_get(ctx, shardd);
if (change == NULL)
return (0);
diff --git a/lib/libvmod_directors/shard_cfg.h b/lib/libvmod_directors/shard_cfg.h
index 0b8b8612c..5c1dc5c20 100644
--- a/lib/libvmod_directors/shard_cfg.h
+++ b/lib/libvmod_directors/shard_cfg.h
@@ -28,15 +28,11 @@
* SUCH DAMAGE.
*/
-VCL_BOOL shardcfg_add_backend(VRT_CTX, struct vmod_priv *priv,
- const struct sharddir *shardd, VCL_BACKEND be, VCL_STRING ident,
- VCL_DURATION rampup, VCL_REAL weight);
-VCL_BOOL shardcfg_remove_backend(VRT_CTX, struct vmod_priv *priv,
- const struct sharddir *shardd, VCL_BACKEND be, VCL_STRING ident);
-VCL_BOOL shardcfg_clear(VRT_CTX, struct vmod_priv *priv,
- const struct sharddir *shardd);
-VCL_BOOL shardcfg_reconfigure(VRT_CTX, struct vmod_priv *priv,
- struct sharddir *shardd, VCL_INT replicas);
+VCL_BOOL shardcfg_add_backend(VRT_CTX, const struct sharddir *shardd,
+ VCL_BACKEND be, VCL_STRING ident, VCL_DURATION rampup, VCL_REAL weight);
+VCL_BOOL shardcfg_remove_backend(VRT_CTX, const struct sharddir *shardd,
+ VCL_BACKEND be, VCL_STRING ident);
+VCL_BOOL shardcfg_clear(VRT_CTX, const struct sharddir *shardd);
+VCL_BOOL shardcfg_reconfigure(VRT_CTX, struct sharddir *shardd, VCL_INT);
VCL_VOID shardcfg_set_warmup(struct sharddir *shardd, VCL_REAL ratio);
-VCL_VOID shardcfg_set_rampup(struct sharddir *shardd,
- VCL_DURATION duration);
+VCL_VOID shardcfg_set_rampup(struct sharddir *shardd, VCL_DURATION duration);
diff --git a/lib/libvmod_directors/shard_dir.h b/lib/libvmod_directors/shard_dir.h
index 67b709aac..c2c8ddeab 100644
--- a/lib/libvmod_directors/shard_dir.h
+++ b/lib/libvmod_directors/shard_dir.h
@@ -73,6 +73,12 @@ struct sharddir {
uint32_t n_points;
};
+/* VRT_priv_task() id offsets */
+enum shard_task_off_e {
+ task_off_param = 0,
+ task_off_cfg = 1
+};
+
static inline VCL_BACKEND
sharddir_backend(const struct sharddir *shardd, unsigned id)
{
diff --git a/lib/libvmod_directors/vmod_directors.vcc b/lib/libvmod_directors/vmod_directors.vcc
index f0a32af50..f548b3f5d 100644
--- a/lib/libvmod_directors/vmod_directors.vcc
+++ b/lib/libvmod_directors/vmod_directors.vcc
@@ -376,7 +376,7 @@ association.
The association can be changed per backend request using the *param*
argument of `xshard.backend()`_.
-$Method BOOL .add_backend(PRIV_TASK, BACKEND backend,
+$Method BOOL .add_backend(BACKEND backend,
[STRING ident], [DURATION rampup], [REAL weight])
Add a backend *backend* to the director.
@@ -399,28 +399,25 @@ effect of *weight* is also capped such that the total number of
replicas does not exceed `UINT32_MAX`.
NOTE: Backend changes need to be finalized with
-`xshard.reconfigure()`_ and are only supported on one
-shard director at a time.
+`xshard.reconfigure()`_.
-$Method BOOL .remove_backend(PRIV_TASK, [BACKEND backend=0], [STRING ident=0])
+$Method BOOL .remove_backend([BACKEND backend=0], [STRING ident=0])
Remove backend(s) from the director. Either *backend* or *ident* must
be specified. *ident* removes a specific instance. If *backend* is
given without *ident*, all instances of this backend are removed.
NOTE: Backend changes need to be finalized with
-`xshard.reconfigure()`_ and are only supported on one
-shard director at a time.
+`xshard.reconfigure()`_.
-$Method BOOL .clear(PRIV_TASK)
+$Method BOOL .clear()
Remove all backends from the director.
NOTE: Backend changes need to be finalized with
-`xshard.reconfigure()`_ and are only supported on one
-shard director at a time.
+`xshard.reconfigure()`_.
-$Method BOOL .reconfigure(PRIV_TASK, INT replicas=67)
+$Method BOOL .reconfigure(INT replicas=67)
Reconfigure the consistent hashing ring to reflect backend changes.
diff --git a/lib/libvmod_directors/vmod_shard.c b/lib/libvmod_directors/vmod_shard.c
index fbcd1d132..18c3465ce 100644
--- a/lib/libvmod_directors/vmod_shard.c
+++ b/lib/libvmod_directors/vmod_shard.c
@@ -326,11 +326,10 @@ vmod_shard_add_backend(VRT_CTX, struct vmod_directors_shard *vshard,
".add_backend(weight=%f) ignored", args->weight);
}
- return shardcfg_add_backend(ctx, args->arg1,
- vshard->shardd, args->backend,
+ return (shardcfg_add_backend(ctx, vshard->shardd, args->backend,
args->valid_ident ? args->ident : NULL,
args->valid_rampup ? args->rampup : nan(""),
- weight);
+ weight));
}
VCL_BOOL v_matchproto_(td_directors_shard_remove_backend)
@@ -349,23 +348,21 @@ vmod_shard_remove_backend(VRT_CTX, struct vmod_directors_shard *vshard,
return (0);
}
- return shardcfg_remove_backend(ctx, args->arg1, vshard->shardd,
- be, ident);
+ return (shardcfg_remove_backend(ctx, vshard->shardd, be, ident));
}
VCL_BOOL v_matchproto_(td_directors_shard_clear)
-vmod_shard_clear(VRT_CTX, struct vmod_directors_shard *vshard,
- struct vmod_priv *priv)
+vmod_shard_clear(VRT_CTX, struct vmod_directors_shard *vshard)
{
CHECK_OBJ_NOTNULL(vshard, VMOD_SHARD_SHARD_MAGIC);
- return shardcfg_clear(ctx, priv, vshard->shardd);
+ return (shardcfg_clear(ctx, vshard->shardd));
}
VCL_BOOL v_matchproto_(td_directors_shard_reconfigure)
vmod_shard_reconfigure(VRT_CTX, struct vmod_directors_shard *vshard,
- struct vmod_priv *priv, VCL_INT replicas)
+ VCL_INT replicas)
{
- return shardcfg_reconfigure(ctx, priv, vshard->shardd, replicas);
+ return (shardcfg_reconfigure(ctx, vshard->shardd, replicas));
}
static inline uint32_t
@@ -878,12 +875,14 @@ shard_param_task(VRT_CTX, const void *id,
{
struct vmod_directors_shard_param *p;
struct vmod_priv *task;
+ const void *task_id;
CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
CHECK_OBJ_NOTNULL(pa, VMOD_SHARD_SHARD_PARAM_MAGIC);
assert(pa->scope > _SCOPE_INVALID);
- task = VRT_priv_task(ctx, id);
+ task_id = (const char *)id + task_off_param;
+ task = VRT_priv_task(ctx, task_id);
if (task == NULL) {
VRT_fail(ctx, "no priv_task");
More information about the varnish-commit
mailing list