[master] 895045a93 vbp: Introduce a backend probe state & document fsm

Nils Goroll nils.goroll at uplex.de
Mon Jul 15 18:29:03 UTC 2024


commit 895045a932ae67017b6516586b6681f69cff9e80
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Mon Jun 3 17:42:33 2024 +0200

    vbp: Introduce a backend probe state & document fsm
    
    The state is not used yet other than for assertions.

diff --git a/bin/varnishd/cache/cache_backend_probe.c b/bin/varnishd/cache/cache_backend_probe.c
index 6de4eeb89..a0b47c4a3 100644
--- a/bin/varnishd/cache/cache_backend_probe.c
+++ b/bin/varnishd/cache/cache_backend_probe.c
@@ -56,6 +56,17 @@
 
 #include "VSC_vbe.h"
 
+struct vbp_state {
+	const char			*name;
+};
+
+#define VBP_STATE(n) const struct vbp_state vbp_state_ ## n [1] = {{ .name = #n }}
+VBP_STATE(scheduled);
+VBP_STATE(running);
+VBP_STATE(cold);
+VBP_STATE(deleted);
+#undef VBP_STATE
+
 /* Default averaging rate, we want something pretty responsive */
 #define AVG_RATE			4
 
@@ -83,6 +94,7 @@ struct vbp_target {
 	double				rate;
 
 	vtim_real			due;
+	const struct vbp_state		*state;
 	int				running;
 	int				heap_idx;
 	struct pool_task		task[1];
@@ -449,15 +461,18 @@ vbp_task(struct worker *wrk, void *priv)
 
 	Lck_Lock(&vbp_mtx);
 	if (vt->running < 0) {
+		assert(vt->state == vbp_state_deleted);
 		assert(vt->heap_idx == VBH_NOIDX);
 		vbp_delete(vt);
 	} else {
+		assert(vt->state == vbp_state_running);
 		vt->running = 0;
 		if (vt->heap_idx != VBH_NOIDX) {
 			vt->due = VTIM_real() + vt->interval;
 			VBH_delete(vbp_heap, vt->heap_idx);
 			vbp_heap_insert(vt);
 		}
+		vt->state = vbp_state_scheduled;
 	}
 	Lck_Unlock(&vbp_mtx);
 }
@@ -490,15 +505,22 @@ vbp_thread(struct worker *wrk, void *priv)
 			vt->due = now + vt->interval;
 			VBH_insert(vbp_heap, vt);
 			if (!vt->running) {
+				assert(vt->state == vbp_state_scheduled);
+				vt->state = vbp_state_running;
 				vt->running = 1;
 				vt->task->func = vbp_task;
 				vt->task->priv = vt;
 				Lck_Unlock(&vbp_mtx);
 				r = Pool_Task_Any(vt->task, TASK_QUEUE_REQ);
 				Lck_Lock(&vbp_mtx);
-				if (r)
+				if (r) {
 					vt->running = 0;
+					vt->state = vbp_state_scheduled;
+				}
 			}
+			else
+				assert(vt->state == vbp_state_running);
+
 		}
 	}
 	NEEDLESS(Lck_Unlock(&vbp_mtx));
@@ -658,12 +680,18 @@ VBP_Control(const struct backend *be, int enable)
 
 	Lck_Lock(&vbp_mtx);
 	if (enable) {
+		// XXX next two assertions are WRONG, see #4108 - WIP
+		assert(vt->state == vbp_state_cold);
 		assert(vt->heap_idx == VBH_NOIDX);
 		vt->due = VTIM_real();
 		vbp_heap_insert(vt);
+		vt->state = vbp_state_scheduled;
 	} else {
+		assert(vt->state == vbp_state_scheduled ||
+		    vt->state == vbp_state_running);
 		assert(vt->heap_idx != VBH_NOIDX);
 		VBH_delete(vbp_heap, vt->heap_idx);
+		vt->state = vbp_state_cold;
 	}
 	Lck_Unlock(&vbp_mtx);
 }
@@ -686,6 +714,7 @@ VBP_Insert(struct backend *b, const struct vrt_backend_probe *vp,
 	ALLOC_OBJ(vt, VBP_TARGET_MAGIC);
 	XXXAN(vt);
 
+	vt->state = vbp_state_cold;
 	vt->conn_pool = tp;
 	VCP_AddRef(vt->conn_pool);
 	vt->backend = b;
@@ -711,13 +740,17 @@ VBP_Remove(struct backend *be)
 	be->probe = NULL;
 	vt->backend = NULL;
 	if (vt->running) {
+		assert(vt->state == vbp_state_running);
 		// task scheduled, it calls vbp_delete()
 		vt->running = -1;
 		vt = NULL;
+		vt->state = vbp_state_deleted;
 	} else if (vt->heap_idx != VBH_NOIDX) {
+		assert(vt->state == vbp_state_scheduled);
 		// task done, not yet rescheduled
 		VBH_delete(vbp_heap, vt->heap_idx);
-	}
+	} else
+		assert(vt->state == vbp_state_cold);
 	Lck_Unlock(&vbp_mtx);
 	if (vt != NULL) {
 		assert(vt->heap_idx == VBH_NOIDX);
diff --git a/doc/graphviz/cache_backend_probe.dot b/doc/graphviz/cache_backend_probe.dot
new file mode 100644
index 000000000..937eb6c0f
--- /dev/null
+++ b/doc/graphviz/cache_backend_probe.dot
@@ -0,0 +1,35 @@
+# cache_backend_probe struct vbp_state
+
+digraph cache_backend_probe {
+	ALLOC
+	scheduled
+	running
+	cold
+	deleted
+	FREE
+
+	edge [fontname=Courier]
+
+	edge [label="vbp_task()"]
+	deleted -> FREE
+	running -> scheduled
+
+	edge [label="vbp_thread()"]
+	scheduled -> running
+
+	edge [label="vbp_thread() error"]
+	scheduled -> scheduled
+
+	edge [label="VBP_Control()"]
+	cold -> scheduled
+	scheduled -> cold
+	running -> cold
+
+	edge [label="VBP_Insert()"]
+	ALLOC -> cold
+
+	edge [label="VBP_Remove()"]
+	running -> deleted # should not happen. we should go through some cool first
+	scheduled -> FREE # This should not happen. VBP_Control should have set cold
+	cold -> FREE
+}


More information about the varnish-commit mailing list