BBlack has uploaded a new change for review.
https://gerrit.wikimedia.org/r/71806
Change subject: Multiple DBs per VCL; removed fatal startup errors
......................................................................
Multiple DBs per VCL; removed fatal startup errors
Change-Id: I330550e1e54e07a057d348e31770edc2fd2af924
---
M README.rst
M src/tests/test01.vtc
R src/tests/test01a.json
A src/tests/test01b.json
A src/tests/test01c.json
M src/vmod_netmapper.c
M src/vmod_netmapper.vcc
7 files changed, 158 insertions(+), 91 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/operations/software/varnish/libvmod-netmapper
refs/changes/06/71806/1
diff --git a/README.rst b/README.rst
index 087a2c1..5802bee 100644
--- a/README.rst
+++ b/README.rst
@@ -7,8 +7,8 @@
-------------------------------------------------
:Author: Brandon Black
-:Date: 2013-06-24
-:Version: 1.0
+:Date: 2013-07-02
+:Version: 1.1
:Manual section: 3
SYNOPSIS
@@ -19,11 +19,13 @@
import netmapper;
sub vcl_init {
- netmapper.init("/path/to/netmap.json", 42);
+ netmapper.init("mydb", "/path/to/mydb.json", 42);
+ netmapper.init("odb", "/path/to/odb.json", 42);
}
sub vcl_recv {
- set req.http.X-Foo = netmapper.map("" + client.ip);
+ set req.http.X-Foo = netmapper.map("mydb", "" + client.ip);
+ set req.http.X-Bar = netmapper.map("odb", "" + client.ip);
}
DESCRIPTION
@@ -40,19 +42,20 @@
-----
Prototype
- ``init(STRING DatabaseFile, INT CheckInterval)``
+ ``init(STRING Label, STRING DatabaseFile, INT CheckInterval)``
Return value
VOID
Description
- Initializes netmapper with a given JSON database and reload check
+ Loads a given JSON database with the given Label and reload check
interval (in seconds), for this VCL. The database is checked via
stat(2) for changes every check interval, and reloaded on the fly
- when altered.
+ when altered. The Label is used to differentiate multiple databases
+ during runtime map() calls.
Example
::
sub vcl_init {
- netmapper.init("/path/to/netmap.json", 42);
+ netmapper.init("mydb", "/path/to/mydb.json", 42);
}
@@ -60,19 +63,19 @@
-----
Prototype
- ``map(STRING IPAddr)``
+ ``map(STRING Label, STRING IPAddr)``
Return value
String, could be undefined if no match.
Description
Maps the passed client IP address (in string form) against the
- data loaded from the JSON database, returning the string key
- of the set this address belongs to, or NULL (undefined) if
- not matched by any set.
+ data loaded from the JSON database identified by Label, returning
+ the string key of the set this address belongs to, or NULL (undefined)
+ if not matched by any set.
Example
::
sub vcl_recv {
- set req.http.X-Foo = netmapper.map("" + client.ip);
+ set req.http.X-Foo = netmapper.map("mydb", "" + client.ip);
}
@@ -153,20 +156,21 @@
Usage::
- ./configure VARNISHSRC=DIR [VMODDIR=DIR]
+ ./configure VARNISHSRC=DIR [VARNISHTEST=FILE] [VARNISHD=FILE]
`VARNISHSRC` is the directory of the Varnish source tree for which to
compile your vmod. Both the `VARNISHSRC` and `VARNISHSRC/include`
will be added to the include search paths for your module.
-Optionally you can also set the vmod install directory by adding
-`VMODDIR=DIR` (defaults to the pkg-config discovered directory from your
-Varnish installation).
+By default the varnish source tree must be pre-built, and the varnishd
+and varnishtest binaries will be used directly from it. Optionally,
+you can specify external binaries for these and build against an
+unbuilt `VARNISHRC`. Be sure that the source and binaries match!
Make targets:
* make - builds the vmod
-* make install - installs your vmod in `VMODDIR`
+* make install - installs your vmod in $libdir/varnish/vmods/
* make check - runs the unit tests in ``src/tests/*.vtc``
HISTORY
diff --git a/src/tests/test01.vtc b/src/tests/test01.vtc
index c88b5bd..a41d4d8 100644
--- a/src/tests/test01.vtc
+++ b/src/tests/test01.vtc
@@ -5,13 +5,18 @@
expect req.http.X-CS == "localhosty"
expect req.http.X-CS-T1 == "localhosty"
expect req.http.X-CS-T2 == "localhosty"
- expect req.http.X-CS-T3 == "Carrier Foo"
- expect req.http.X-CS-T4 == "Carrier Bar"
- expect req.http.X-CS-T5 == "Carrier Foo"
- expect req.http.X-CS-T6 == "Carrier Foo"
- expect req.http.X-CS-T7 == "Carrier Bar"
- expect req.http.X-CS-T8 == <undef>
- expect req.http.X-CS-T9 == "Carrier Bar"
+ expect req.http.X-CS-T3 == "ZZZ"
+ expect req.http.X-CS-T4 == "Carrier Foo"
+ expect req.http.X-CS-T5 == "Carrier Bar"
+ expect req.http.X-CS-T6 == "XYZZY"
+ expect req.http.X-CS-T7 == <undef>
+ expect req.http.X-CS-T8 == "Carrier Foo"
+ expect req.http.X-CS-T9 == <undef>
+ expect req.http.X-CS-TA == "Carrier Foo"
+ expect req.http.X-CS-TB == "Carrier Bar"
+ expect req.http.X-CS-TC == <undef>
+ expect req.http.X-CS-TD == <undef>
+ expect req.http.X-CS-TE == "Carrier Bar"
txresp
} -start
@@ -19,20 +24,27 @@
import netmapper from "${vmod_topbuild}/src/.libs/libvmod_netmapper.so";
sub vcl_init {
- netmapper.init("${vmod_topsrc}/src/tests/test01.json", 1);
+ netmapper.init("aaa", "${vmod_topsrc}/src/tests/test01a.json", 1);
+ netmapper.init("bbb", "${vmod_topsrc}/src/tests/test01b.json", 1);
+ netmapper.init("ccc", "${vmod_topsrc}/src/tests/test01c.json", 1);
}
sub vcl_recv {
- set req.http.X-CS = netmapper.map("" + client.ip);
- set req.http.X-CS-T1 = netmapper.map("127.1.2.3");
- set req.http.X-CS-T2 = netmapper.map("::1");
- set req.http.X-CS-T3 = netmapper.map("192.0.2.75");
- set req.http.X-CS-T4 = netmapper.map("192.0.2.175");
- set req.http.X-CS-T5 = netmapper.map("2001:db8:1234::abcd");
- set req.http.X-CS-T6 = netmapper.map("10.200.100.10");
- set req.http.X-CS-T7 = netmapper.map("172.16.123.123");
- set req.http.X-CS-T8 = netmapper.map("2001:db8:4230::abcd");
- set req.http.X-CS-T9 = netmapper.map("2001:db8:4231::abcd");
+ set req.http.X-CS = netmapper.map("aaa", "" + client.ip);
+ set req.http.X-CS-T1 = netmapper.map("aaa", "127.1.2.3");
+ set req.http.X-CS-T2 = netmapper.map("aaa", "::1");
+ set req.http.X-CS-T3 = netmapper.map("ccc", "123.123.123.123");
+ set req.http.X-CS-T4 = netmapper.map("aaa", "192.0.2.75");
+ set req.http.X-CS-T5 = netmapper.map("aaa", "192.0.2.175");
+ set req.http.X-CS-T6 = netmapper.map("bbb", "192.255.1.42");
+ set req.http.X-CS-T7 = netmapper.map("nxnxnx", "1.2.3.4");
+ set req.http.X-CS-T8 = netmapper.map("aaa", "2001:db8:1234::abcd");
+ set req.http.X-CS-T9 = netmapper.map("bbb", "192.254.1.42");
+ set req.http.X-CS-TA = netmapper.map("aaa", "10.200.100.10");
+ set req.http.X-CS-TB = netmapper.map("aaa", "172.16.123.123");
+ set req.http.X-CS-TC = netmapper.map("ccc", "129.129.129.129");
+ set req.http.X-CS-TD = netmapper.map("aaa", "2001:db8:4230::abcd");
+ set req.http.X-CS-TE = netmapper.map("aaa", "2001:db8:4231::abcd");
return (pass);
}
} -start
diff --git a/src/tests/test01.json b/src/tests/test01a.json
similarity index 100%
rename from src/tests/test01.json
rename to src/tests/test01a.json
diff --git a/src/tests/test01b.json b/src/tests/test01b.json
new file mode 100644
index 0000000..5697b33
--- /dev/null
+++ b/src/tests/test01b.json
@@ -0,0 +1 @@
+{ "XYZZY": [ "192.255.0.0/16" ] }
diff --git a/src/tests/test01c.json b/src/tests/test01c.json
new file mode 100644
index 0000000..118e9f8
--- /dev/null
+++ b/src/tests/test01c.json
@@ -0,0 +1 @@
+{ "ZZZ": [ "0.0.0.0/1" ] }
diff --git a/src/vmod_netmapper.c b/src/vmod_netmapper.c
index 63d21e7..ec8db1a 100644
--- a/src/vmod_netmapper.c
+++ b/src/vmod_netmapper.c
@@ -35,12 +35,23 @@
#include "vnm.h"
+// note, the set of databases is indexed at runtime by a text
+// label, and we just iterate strcmp to look them up. If anyone
+// actually has a lot of databases and cares, please submit a patch that
+// implements a hashtable for them!
+
typedef struct {
unsigned reload_check_interval;
+ char* label;
char* fn;
vnm_db_t* db;
pthread_t updater;
struct stat db_stat;
+} vnm_db_file_t;
+
+typedef struct {
+ unsigned db_count;
+ vnm_db_file_t* dbs;
} vnm_priv_t;
// Copy a str_t*'s data to a const char* in the session workspace,
@@ -64,32 +75,33 @@
return rv;
}
-static void* updater_start(void* vp_asvoid) {
- vnm_priv_t* vp = vp_asvoid;
+static void* updater_start(void* dbf_asvoid) {
+ vnm_db_file_t* dbf = dbf_asvoid;
struct stat check_stat;
while(1) {
- sleep(vp->reload_check_interval);
- if(stat(vp->fn, &check_stat)) {
- VSL(SLT_Error, 0, "vmod_netmapper: Failed to stat JSON database
'%s' for reload check", vp->fn);
+ sleep(dbf->reload_check_interval);
+ if(stat(dbf->fn, &check_stat)) {
+ VSL(SLT_Error, 0, "vmod_netmapper: Failed to stat JSON database
'%s' for reload check", dbf->fn);
continue;
}
- if( check_stat.st_mtime != vp->db_stat.st_mtime
- || check_stat.st_ctime != vp->db_stat.st_ctime
- || check_stat.st_ino != vp->db_stat.st_ino
- || check_stat.st_dev != vp->db_stat.st_dev) {
+ if( check_stat.st_mtime != dbf->db_stat.st_mtime
+ || check_stat.st_ctime != dbf->db_stat.st_ctime
+ || check_stat.st_ino != dbf->db_stat.st_ino
+ || check_stat.st_dev != dbf->db_stat.st_dev) {
- vnm_db_t* new_db = vnm_db_parse(vp->fn, &vp->db_stat);
+ vnm_db_t* new_db = vnm_db_parse(dbf->fn, &dbf->db_stat);
if(new_db) {
- vnm_db_t* old_db = vp->db;
- rcu_assign_pointer(vp->db, new_db);
+ vnm_db_t* old_db = dbf->db;
+ rcu_assign_pointer(dbf->db, new_db);
synchronize_rcu();
- vnm_db_destruct(old_db);
- VSL(SLT_CLI, 0, "vmod_netmapper: JSON database '%s' reloaded
with new data", vp->fn); // CLI??
+ if(old_db)
+ vnm_db_destruct(old_db);
+ VSL(SLT_CLI, 0, "vmod_netmapper: JSON database '%s'
(re-)loaded with new data", dbf->fn); // CLI??
}
else {
- VSL(SLT_Error, 0, "vmod_netmapper: JSON database '%s' reload
failed, continuing with old data", vp->fn);
+ VSL(SLT_Error, 0, "vmod_netmapper: JSON database '%s' reload
failed, continuing with old data", dbf->fn);
}
}
}
@@ -100,40 +112,57 @@
static void per_vcl_fini(void* vp_asvoid) {
vnm_priv_t* vp = vp_asvoid;
- // clean up the updater thread
- pthread_cancel(vp->updater);
- pthread_join(vp->updater, NULL);
+ for(unsigned i = 0; i < vp->db_count; i++) {
+ // clean up the updater thread
+ pthread_cancel(vp->dbs[i].updater);
+ pthread_join(vp->dbs[i].updater, NULL);
- // free the most-recent data
- vnm_db_destruct(vp->db);
- free(vp->fn);
+ // free the most-recent data
+ vnm_db_destruct(vp->dbs[i].db);
+ free(vp->dbs[i].fn);
+ }
+
+ free(vp->dbs);
}
/*****************************
* Actual VMOD/VCL/VRT Hooks *
*****************************/
-void vmod_init(struct sess *sp, struct vmod_priv *priv, const char* json_path,
const int reload_interval) {
- if(priv->priv) {
- // I think this would be a bug, but it's remotely possible
- // this is a situation that has to be handled manually
- // during some reload by destructing ourselves, or that
- // multiple threads can run vcl_init() for one VCL?
- WSP(sp, SLT_Error, "vmod_netmapper: per-VCL double-init! Bug?");
- abort();
+// we serialize all calls to vmod_init() for this vmod globally, because
+// (a) it's not a perf hotspot anyways and
+// (b) it is critical that this executes serially for all .init() calls
+// at *least* within a given VCL (PRIV_VCL context).
+// (c) I'm not 100% sure that varnish will never call this from two threads
+// in parallel, either now or in the future, during startup with multiple
+// VCLs using the module, etc.
+pthread_mutex_t serial_init = PTHREAD_MUTEX_INITIALIZER;
+
+void vmod_init(struct sess *sp, struct vmod_priv *priv, const char* db_label,
const char* json_path, const int reload_interval) {
+
+ pthread_mutex_lock(&serial_init);
+
+ vnm_priv_t* vp = priv->priv;
+
+ if(!vp) {
+ priv->priv = vp = calloc(1, sizeof(vnm_priv_t));
+ priv->free = per_vcl_fini;
}
- vnm_priv_t* vp = malloc(sizeof(vnm_priv_t));
- vp->reload_check_interval = reload_interval;
- vp->fn = strdup(json_path);
- vp->db = vnm_db_parse(vp->fn, &vp->db_stat);
- if(!vp->db) {
- VSL(SLT_Error, 0, "vmod_netmapper: Failed initial load of JSON
netmapper database %s", vp->fn);
- abort();
- }
- pthread_create(&vp->updater, NULL, updater_start, vp);
- priv->priv = vp;
- priv->free = per_vcl_fini;
+ const unsigned db_idx = vp->db_count++;
+ vp->dbs = realloc(vp->dbs, vp->db_count * sizeof(vnm_db_file_t));
+ vnm_db_file_t* dbf = &vp->dbs[db_idx];
+
+ dbf->reload_check_interval = reload_interval;
+ dbf->fn = strdup(json_path);
+ dbf->label = strdup(db_label);
+ memset(&dbf->db_stat, 0, sizeof(struct stat));
+ dbf->db = vnm_db_parse(dbf->fn, &dbf->db_stat);
+ if(!dbf->db)
+ VSL(SLT_Error, 0, "vmod_netmapper: Failed initial load of JSON
netmapper database %s (will keep trying periodically)", dbf->fn);
+
+ pthread_mutex_unlock(&serial_init);
+ pthread_create(&dbf->updater, NULL, updater_start, dbf);
}
// Crazy hack to get per-thread rcu register/unregister, even though
@@ -147,7 +176,7 @@
static void destruct_rcu(void* x) { pthread_setspecific(unreg_hack, NULL);
rcu_unregister_thread(); }
static void make_unreg_hack(void) { pthread_key_create(&unreg_hack,
destruct_rcu); }
-const char* vmod_map(struct sess *sp, struct vmod_priv* priv, const char*
ip_string) {
+const char* vmod_map(struct sess *sp, struct vmod_priv* priv, const char*
db_label, const char* ip_string) {
assert(sp); assert(priv); assert(priv->priv); assert(ip_string);
CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
@@ -160,22 +189,42 @@
rcu_registered = true;
}
+ // static database index, no thread concerns during runtime...
+ const vnm_priv_t* vp = priv->priv;
+ const vnm_db_file_t* dbf = NULL;
+ for(unsigned i = 0; i < vp->db_count; i++) {
+ if(!strcmp(db_label, vp->dbs[i].label)) {
+ dbf = &vp->dbs[i];
+ break;
+ }
+ }
+
const char* rv = NULL;
- // normal rcu reader stuff
- rcu_thread_online();
- rcu_read_lock();
+ if(!dbf) {
+ VSL(SLT_Error, 0, "vmod_netmapper: JSON database label '%s' is not
configured!", db_label);
+ }
+ else {
+ // normal rcu reader stuff
+ rcu_thread_online();
+ rcu_read_lock();
- // search net database. if match, convert
- // string to a vcl string and return it...
- const vnm_priv_t* vp = priv->priv;
- const vnm_str_t* str = vnm_lookup(rcu_dereference(vp->db), ip_string);
- if(str)
- rv = vnm_str_to_vcl(sp, str);
+ const vnm_db_t* dbptr = rcu_dereference(dbf->db);
+ if(dbptr) {
+ // search net database. if match, convert
+ // string to a vcl string and return it...
+ const vnm_str_t* str = vnm_lookup(dbptr, ip_string);
+ if(str)
+ rv = vnm_str_to_vcl(sp, str);
+ }
+ else {
+ VSL(SLT_Error, 0, "vmod_netmapper: JSON database label '%s' was
never succesfully loaded!", db_label);
+ }
- // normal rcu reader stuff
- rcu_read_unlock();
- rcu_thread_offline();
+ // normal rcu reader stuff
+ rcu_read_unlock();
+ rcu_thread_offline();
+ }
return rv;
}
diff --git a/src/vmod_netmapper.vcc b/src/vmod_netmapper.vcc
index 5ed3c71..1f33ea9 100644
--- a/src/vmod_netmapper.vcc
+++ b/src/vmod_netmapper.vcc
@@ -1,3 +1,3 @@
Module netmapper
-Function VOID init(PRIV_VCL, STRING, INT)
-Function STRING map(PRIV_VCL, STRING)
+Function VOID init(PRIV_VCL, STRING, STRING, INT)
+Function STRING map(PRIV_VCL, STRING, STRING)
--
To view, visit https://gerrit.wikimedia.org/r/71806
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I330550e1e54e07a057d348e31770edc2fd2af924
Gerrit-PatchSet: 1
Gerrit-Project: operations/software/varnish/libvmod-netmapper
Gerrit-Branch: master
Gerrit-Owner: BBlack <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
