From c75a811a7734221003ac2628493832acf36b12d5 Mon Sep 17 00:00:00 2001 From: Sergey Poznyakoff Date: Thu, 3 Aug 2017 22:21:03 +0300 Subject: Serialize write access. Improve API. * src/vmod_dict.c (entry): Link into a double-linked list. (entry_append, entry_remove): New functions. (max_coll): New static. (locker_t): New type. (locker_init,locker_rlock,locker_wlock) (locker_runlock,locker_wunlock): New functions. (load_entries): Use syslog for diagnostics. Abort if unable to open the file. Fix minor memory leak. (rehash,fill_table): New functions. (dict_event): Handle VCL_EVENT_DISCARD. (vmod_load): Change signature. (vmod_ci,vmod_collisions): New functions. (vmod_clear): New function. * src/vmod_dict.vcc: Update. * tests/ci.at: Accomodate the above changes. * tests/cs.at: Likewise. --- src/vmod_dict.c | 271 ++++++++++++++++++++++++++++++++++++++++++++++-------- src/vmod_dict.vcc | 30 ++++-- tests/ci.at | 4 +- tests/cs.at | 3 +- 4 files changed, 259 insertions(+), 49 deletions(-) diff --git a/src/vmod_dict.c b/src/vmod_dict.c index 03d116f..5875abd 100644 --- a/src/vmod_dict.c +++ b/src/vmod_dict.c @@ -21,6 +21,7 @@ #include #include #include +#include #include "vcl.h" #include "vrt.h" #include "vas.h" @@ -32,15 +33,116 @@ struct entry char *key; char *val; size_t hash; - struct entry *next; + struct entry *next, *prev; }; static struct entry *ent_head, *ent_tail; static size_t ent_count; +static void +entry_append(struct entry *ent) +{ + ent->next = NULL; + ent->prev = ent_tail; + if (ent_tail) + ent_tail->next = ent; + else + ent_head = ent; + ent_tail = ent; + ent_count++; +} + +static void +entry_remove(struct entry *ent) +{ + struct entry *p; + + if ((p = ent->prev) != NULL) + p->next = ent->next; + else + ent_head = ent->next; + if ((p = ent->next) != NULL) + p->prev = ent->prev; + else + ent_tail = ent->prev; + ent_count--; + free(ent); +} + + static size_t hash_size; -static struct entry const **hash_table; +static struct entry **hash_table; static size_t max_hash_size = 8192; +static ssize_t max_coll = -1; /* Max. number of collisions allowed */ + +typedef struct { + size_t readers; + size_t writer; + pthread_mutex_t mutex; + pthread_cond_t lock_free; +} locker_t; + +static locker_t rwlock; + +static void +locker_init(locker_t *rdwr) +{ + rdwr->readers = 0; + rdwr->writer = 0; + pthread_mutex_init(&rdwr->mutex, NULL); + pthread_cond_init(&rdwr->lock_free, NULL); +} + +static void +locker_rlock(locker_t *rdwr) +{ + pthread_mutex_lock(&rdwr->mutex); + while (rdwr->writer) + pthread_cond_wait(&rdwr->lock_free, &rdwr->mutex); + rdwr->readers++; + pthread_mutex_unlock(&rdwr->mutex); +} + +static void +locker_wlock(locker_t *rdwr) +{ + pthread_mutex_lock(&rdwr->mutex); + while (rdwr->writer || rdwr->readers) + pthread_cond_wait(&rdwr->lock_free, &rdwr->mutex); + rdwr->writer++; + pthread_mutex_unlock(&rdwr->mutex); +} + +static int +locker_runlock(locker_t *rdwr) +{ + pthread_mutex_lock(&rdwr->mutex); + if (rdwr->readers == 0) { + pthread_mutex_unlock(&rdwr->mutex); + return -1; + } else { + rdwr->readers--; + if (rdwr->readers == 0) + pthread_cond_signal(&rdwr->lock_free); + pthread_mutex_unlock(&rdwr->mutex); + return 0; + } +} + +static int +locker_wunlock(locker_t *rdwr) +{ + pthread_mutex_lock(&rdwr->mutex); + if (rdwr->writer == 0) { + pthread_mutex_unlock(&rdwr->mutex); + return -1; + } else { + rdwr->writer = 0; + pthread_cond_signal(&rdwr->lock_free); + pthread_mutex_unlock(&rdwr->mutex); + return 0; + } +} static int cs_coll[] = { 0, 1, 2, 3, 4, 5, 6, 7, @@ -235,9 +337,10 @@ load_entries(char const *file) fp = fopen(file, "r"); if (!fp) { - fprintf(stderr, - "lookup.init: can't load \"%s\": %s\n", - file, strerror(errno)); + syslog(LOG_DAEMON|LOG_ERR, + "dict.init: can't load \"%s\": %s", + file, strerror(errno)); + abort(); } line = 0; while ((n = my_getline(fp, &buf, &size)) > 0) { @@ -256,7 +359,8 @@ load_entries(char const *file) p[l] = 0; if (l == n) { - fprintf(stderr, "%s:%u: malformed line\n", file, line); + syslog(LOG_DAEMON|LOG_ERR, + "%s:%u: malformed line", file, line); continue; } @@ -271,35 +375,35 @@ load_entries(char const *file) strcpy(ent->key, p); strcpy(ent->val, val); ent->hash = string_hash(ent->key); - ent->next = NULL; - if (ent_tail) - ent_tail->next = ent; - else - ent_head = ent; - ent_tail = ent; - ent_count++; + entry_append(ent); } + free(buf); assert(n != -1); fclose(fp); } - -VCL_VOID -vmod_load(VRT_CTX, VCL_STRING file, VCL_BOOL ci, VCL_INT coll) + +static void +rehash(void) +{ + struct entry *ent; + for (ent = ent_head; ent; ent = ent->next) + ent->hash = string_hash(ent->key); +} + +static void +fill_table(void) { struct entry *ent; size_t next_size; - collation = ci ? ci_coll : cs_coll; - - load_entries(file); - - next_size = ent_count * 2; - if (ent_count < max_hash_size) + next_size = ent_count * 2 + 1; + if (next_size > max_hash_size) max_hash_size = next_size; + do { size_t cn = 0; size_t hs; - + hash_size = next_size; hs = sizeof(hash_table[0]) * hash_size; hash_table = realloc(hash_table, hs); @@ -311,6 +415,10 @@ vmod_load(VRT_CTX, VCL_STRING file, VCL_BOOL ci, VCL_INT coll) size_t n = 0; while (hash_table[i]) { + if (streq(hash_table[i]->key, ent->key)) { + entry_remove(hash_table[i]); + break; + } i = (i + 1) % hash_size; assert(i != h); n++; @@ -320,33 +428,116 @@ vmod_load(VRT_CTX, VCL_STRING file, VCL_BOOL ci, VCL_INT coll) hash_table[i] = ent; } - if (coll <= 0 || cn < coll) + if (max_coll <= 0 || cn < max_coll) break; - next_size *= 2; + next_size = next_size * 2 + 1; } while (next_size < max_hash_size); +} + +int +dict_event(VRT_CTX, struct vmod_priv *priv, enum vcl_event_e e) +{ + switch (e) { + case VCL_EVENT_LOAD: + locker_init(&rwlock); + break; + + case VCL_EVENT_DISCARD: + locker_wlock(&rwlock); + while (ent_head) + entry_remove(ent_head); + free(hash_table); + hash_table = NULL; + hash_size = 0; + locker_wunlock(&rwlock); + break; + + case VCL_EVENT_WARM: + case VCL_EVENT_COLD: + break; + } + return 0; } +VCL_VOID +vmod_ci(VRT_CTX, VCL_BOOL ci) +{ + int *cp; + locker_wlock(&rwlock); + cp = ci ? ci_coll : cs_coll; + if (cp != collation) { + collation = cp; + rehash(); + fill_table(); + } + locker_wunlock(&rwlock); +} + +VCL_VOID +vmod_collisions(VRT_CTX, VCL_INT coll) +{ + max_coll = coll; +} + +VCL_VOID +vmod_load(VRT_CTX, VCL_STRING file) +{ + locker_wlock(&rwlock); + load_entries(file); + fill_table(); + locker_wunlock(&rwlock); +} + +VCL_VOID +vmod_clear(VRT_CTX) +{ + size_t i; + + locker_wlock(&rwlock); + for (i = 0; i < hash_size; i++) + hash_table[i] = NULL; + while (ent_head) + entry_remove(ent_head); + locker_wunlock(&rwlock); +} + +static char const * +lookup_unlocked(char const *key) +{ + size_t i, h; + char const *s = NULL; + + if (hash_size == 0) + return NULL; + + h = string_hash(key) % hash_size; + i = h; + while (hash_table[i]) { + if (streq(hash_table[i]->key, key)) { + s = hash_table[i]->val; + break; + } + i = (i + 1) % hash_size; + if (i == h) + break; + } + return s; +} + VCL_STRING vmod_lookup(VRT_CTX, VCL_STRING key) { + char *s = NULL; if (key) { - size_t i, h; - - h = string_hash(key) % hash_size; - i = h; - while (hash_table[i]) { - if (streq(hash_table[i]->key, key)) { - char *s = WS_Copy(ctx->ws, hash_table[i]->val, - -1); - AN(s); - return s; - } - i = (i + 1) % hash_size; - if (i == h) - break; + locker_rlock(&rwlock); + char const *p = lookup_unlocked(key); + if (p) { + s = WS_Copy(ctx->ws, p, -1); + AN(s); } + locker_runlock(&rwlock); } - return NULL; + return s; } diff --git a/src/vmod_dict.vcc b/src/vmod_dict.vcc index 76c4267..5e80a02 100644 --- a/src/vmod_dict.vcc +++ b/src/vmod_dict.vcc @@ -9,17 +9,33 @@ or a pair of keyword - value separated by one or more whitespace characters. Leading and trailing whitespace is discarded. Comments are introduced by a hash sign at the beginning of the line. Empty lines and comments are ignored. -$Function VOID load(STRING file, BOOL ci, INT ncls) +$Event dict_event +$Function VOID ci(BOOL v) Description - Loads key/value dictionary from **file** into memory. The **ci** - parameter controls whether key lookups should be case-insensitive. - The **ncls** parameter, if positive, gives the maximum allowable length - of collision chain in the hash table. The module will adjust the hash - load factor to ensure thes number of collisions doesn't exceed this - value. + If **v** is **true**, sets case-insensitive string comparison. Default is + case-sensitive comparison. + +$Function VOID collisions(INT n) + +Description + Sets the maximum allowable length of collision chain in the hash table. + Negative value of **n** means unlimited length. Otherwise, the module will + adjust the hash table load factor to ensure the number of collisions + doesn't exceed **n**. + +$Function VOID load(STRING file) + +Description + Loads key/value dictionary from **file** into memory. The **file** must + exist and be readable for the user varnish runs as. This function is normally called from **vcl_init**. + +$Function VOID clear() + +Description + Clears entire dictionary. $Function STRING lookup(STRING key) diff --git a/tests/ci.at b/tests/ci.at index 6554e53..d486188 100644 --- a/tests/ci.at +++ b/tests/ci.at @@ -2,7 +2,9 @@ AT_SETUP([case-insensitive]) AT_KEYWORDS([case-insensitive]) AT_VARNISHTEST([dict],[ sub vcl_init { - dict.load("\${vmod_topsrc}/tests/num.dict", true, 0); + dict.ci(true); + dict.collisions(0); + dict.load("\${vmod_topsrc}/tests/num.dict"); } sub vcl_recv { set req.http.X-result = dict.lookup(regsub(req.url, "^/(.*)", "\1")); diff --git a/tests/cs.at b/tests/cs.at index f847773..0333943 100644 --- a/tests/cs.at +++ b/tests/cs.at @@ -2,7 +2,8 @@ AT_SETUP([case-sensitive]) AT_KEYWORDS([case-sensitive]) AT_VARNISHTEST([dict],[ sub vcl_init { - dict.load("\${vmod_topsrc}/tests/num.dict", false, 0); + dict.collisions(0); + dict.load("\${vmod_topsrc}/tests/num.dict"); } sub vcl_recv { set req.http.X-result = dict.lookup(regsub(req.url, "^/(.*)", "\1")); -- cgit v1.2.1