From 3431181be1dd0673508cd130bf5d7664044d6f25 Mon Sep 17 00:00:00 2001 From: Eduardo Silva Date: Fri, 17 Nov 2017 14:31:21 -0800 Subject: [PATCH 1/5] hash_table: new structure to support counters and eviction modes Signed-off-by: Eduardo Silva --- include/fluent-bit/flb_hash.h | 16 ++++++++++++++-- src/flb_hash.c | 27 ++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/include/fluent-bit/flb_hash.h b/include/fluent-bit/flb_hash.h index d8f3b4c6961..ecc6b861af5 100644 --- a/include/fluent-bit/flb_hash.h +++ b/include/fluent-bit/flb_hash.h @@ -26,12 +26,20 @@ #include #include +/* Eviction modes when the table reach full capacity (if any) */ +#define FLB_HASH_EVICT_NONE 0 +#define FLB_HASH_EVICT_OLDER 1 +#define FLB_HASH_EVICT_LESS_USED 2 + struct flb_hash_entry { + time_t created; + uint64_t hits; char *key; size_t key_len; char *val; size_t val_size; - struct mk_list _head; + struct mk_list _head; /* link to flb_hash_table->chains */ + struct mk_list _head_parent; /* link to flb_hash->entries */ }; struct flb_hash_table { @@ -40,11 +48,15 @@ struct flb_hash_table { }; struct flb_hash { + int evict_mode; + int max_entries; + int total_count; size_t size; + struct mk_list entries; struct flb_hash_table *table; }; -struct flb_hash *flb_hash_create(size_t size); +struct flb_hash *flb_hash_create(int evict_mode, size_t size, int max_entries); void flb_hash_destroy(struct flb_hash *ht); int flb_hash_add(struct flb_hash *ht, char *key, int key_len, diff --git a/src/flb_hash.c b/src/flb_hash.c index a3124fe598b..68591afbc61 100644 --- a/src/flb_hash.c +++ b/src/flb_hash.c @@ -87,12 +87,13 @@ static unsigned int gen_hash(const void *key, int len) static inline void flb_hash_entry_free(struct flb_hash_entry *entry) { mk_list_del(&entry->_head); + mk_list_del(&entry->_head_parent); flb_free(entry->key); flb_free(entry->val); flb_free(entry); } -struct flb_hash *flb_hash_create(size_t size) +struct flb_hash *flb_hash_create(int evict_mode, size_t size, int max_entries) { int i; struct flb_hash_table *tmp; @@ -108,6 +109,10 @@ struct flb_hash *flb_hash_create(size_t size) return NULL; } + mk_list_init(&ht->entries); + ht->evict_mode = evict_mode; + ht->max_entries = max_entries; + ht->total_count = 0; ht->size = size; ht->table = flb_calloc(1, sizeof(struct flb_hash_table) * size); if (!ht->table) { @@ -162,6 +167,20 @@ int flb_hash_add(struct flb_hash *ht, char *key, int key_len, return -1; } + /* Check capacity */ + if (ht->max_entries > 0 && ht->total_count >= ht->max_entries) { + /* FIXME: handle eviction mode */ + if (ht->evict_mode == FLB_HASH_EVICT_NONE) { + + } + else if (ht->evict_mode == FLB_HASH_EVICT_OLDER) { + + } + else if (ht->evict_mode == FLB_HASH_EVICT_LESS_USED) { + + } + } + /* Generate hash number */ hash = gen_hash(key, key_len); id = (hash % ht->size); @@ -172,6 +191,8 @@ int flb_hash_add(struct flb_hash *ht, char *key, int key_len, flb_errno(); return -1; } + entry->created = time(NULL); + entry->hits = 0; /* Store the key and value as a new memory region */ entry->key = flb_strdup(key); @@ -183,6 +204,7 @@ int flb_hash_add(struct flb_hash *ht, char *key, int key_len, flb_free(entry); return -1; } + /* * Copy the buffer and append a NULL byte in case the caller set and * expects a string. @@ -197,6 +219,7 @@ int flb_hash_add(struct flb_hash *ht, char *key, int key_len, /* Check if the new key already exists */ if (table->count == 0) { mk_list_add(&entry->_head, &table->chains); + mk_list_add(&entry->_head_parent, &ht->entries); } else { mk_list_foreach_safe(head, tmp, &table->chains) { @@ -208,6 +231,7 @@ int flb_hash_add(struct flb_hash *ht, char *key, int key_len, } } mk_list_add(&entry->_head, &table->chains); + mk_list_add(&entry->_head_parent, &ht->entries); } table->count++; @@ -270,6 +294,7 @@ int flb_hash_get(struct flb_hash *ht, char *key, int key_len, return -1; } + entry->hits++; *out_buf = entry->val; *out_size = entry->val_size; From 6d4f483ebf744bc12f27519bee3a5cd534d7faaf Mon Sep 17 00:00:00 2001 From: Eduardo Silva Date: Fri, 17 Nov 2017 14:32:34 -0800 Subject: [PATCH 2/5] env: use new hashtable prototype Signed-off-by: Eduardo Silva --- src/flb_env.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/flb_env.c b/src/flb_env.c index b6b48838649..5b5cb8cd4a3 100644 --- a/src/flb_env.c +++ b/src/flb_env.c @@ -99,7 +99,7 @@ struct flb_env *flb_env_create() } /* Create the hash-table */ - ht = flb_hash_create(FLB_ENV_SIZE); + ht = flb_hash_create(FLB_ENV_SIZE, FLB_HASH_EVICT_NONE, -1); if (!ht) { flb_free(env); return NULL; From 891485cfdc14e95006a4847a9a5505b29737e70c Mon Sep 17 00:00:00 2001 From: Eduardo Silva Date: Fri, 17 Nov 2017 14:32:49 -0800 Subject: [PATCH 3/5] filter_kubernetes: use new hashtable prototype Signed-off-by: Eduardo Silva --- plugins/filter_kubernetes/kube_conf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/filter_kubernetes/kube_conf.c b/plugins/filter_kubernetes/kube_conf.c index 7e1b5698658..58e68aea299 100644 --- a/plugins/filter_kubernetes/kube_conf.c +++ b/plugins/filter_kubernetes/kube_conf.c @@ -169,7 +169,8 @@ struct flb_kube *flb_kube_conf_create(struct flb_filter_instance *i, ctx->api_https ? "https" : "http", ctx->api_host, ctx->api_port); - ctx->hash_table = flb_hash_create(FLB_HASH_TABLE_SIZE); + ctx->hash_table = flb_hash_create(FLB_HASH_TABLE_SIZE, + FLB_HASH_EVICT_NONE, -1); if (!ctx->hash_table) { flb_kube_conf_destroy(ctx); return NULL; From 00b173ad285b92021eb05f09d27fd79aa212c8a4 Mon Sep 17 00:00:00 2001 From: Eduardo Silva Date: Fri, 17 Nov 2017 14:34:09 -0800 Subject: [PATCH 4/5] tests: internal: hashtable: use new hashtable func prototype Signed-off-by: Eduardo Silva --- tests/internal/hashtable.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/internal/hashtable.c b/tests/internal/hashtable.c index 0a8a3b4a2c2..7f099346f66 100644 --- a/tests/internal/hashtable.c +++ b/tests/internal/hashtable.c @@ -79,7 +79,7 @@ void test_create_zero() { struct flb_hash *ht; - ht = flb_hash_create(0); + ht = flb_hash_create(0, FLB_HASH_EVICT_NONE, -1); TEST_CHECK(ht == NULL); } @@ -91,7 +91,7 @@ void test_single() size_t out_size; struct flb_hash *ht; - ht = flb_hash_create(1); + ht = flb_hash_create(1, FLB_HASH_EVICT_NONE, -1); TEST_CHECK(ht != NULL); ret = ht_add(ht, "key", "value"); @@ -112,7 +112,7 @@ void test_small_table() struct map *m; struct flb_hash *ht; - ht = flb_hash_create(1); + ht = flb_hash_create(1, FLB_HASH_EVICT_NONE, -1); TEST_CHECK(ht != NULL); for (i = 0; i < sizeof(entries) / sizeof(struct map); i++) { @@ -129,7 +129,7 @@ void test_medium_table() struct map *m; struct flb_hash *ht; - ht = flb_hash_create(8); + ht = flb_hash_create(8, FLB_HASH_EVICT_NONE, -1); TEST_CHECK(ht != NULL); for (i = 0; i < sizeof(entries) / sizeof(struct map); i++) { @@ -151,7 +151,7 @@ void test_chaining() struct flb_hash_table *table; struct flb_hash *ht; - ht = flb_hash_create(8); + ht = flb_hash_create(8, FLB_HASH_EVICT_NONE, -1); TEST_CHECK(ht != NULL); for (i = 0; i < sizeof(entries) / sizeof(struct map); i++) { @@ -187,7 +187,7 @@ void test_delete_all() struct flb_hash_table *table; struct flb_hash *ht; - ht = flb_hash_create(8); + ht = flb_hash_create(8, FLB_HASH_EVICT_NONE, -1); TEST_CHECK(ht != NULL); total = sizeof(entries) / sizeof(struct map); From 60c68c83387283ff982fdfbe86c157df0f98e070 Mon Sep 17 00:00:00 2001 From: James Ravn Date: Tue, 14 Nov 2017 16:33:54 +0000 Subject: [PATCH 5/5] hash: bound the hashtable size The hashtable is being used as a cache for kubernetes metadata and environment variables. The drawback is the hashtable is unbounded and doesn't resize itself. Thus over time it uses more memory and performance becomes worse as the chains get longer. For Kubernetes this can be particularly bad in busy environments as the cache key uses podname, which will change every time a deployment occurs. By bounding the hashtable we ensure memory doesn't become unbounded and performance remains constant. For a cache it should be okay to evict old entries. This implementation tries to keep things simple by performing eviction on adding a key to the table. As long as the hashtable size isn't exceeded, the behaviour remains the same as before - the table is looked up via hash function, and the table chain is extended. When the hashtable size is exceeded, the housekeeping will remove all prior keys in a random chain to bring down the hashtable size. The advantage of a random eviction is it's simple and fast versus constructing an LRU list (or something else). --- include/fluent-bit/flb_hash.h | 2 ++ plugins/filter_kubernetes/kube_conf.c | 5 +-- src/flb_env.c | 2 +- src/flb_hash.c | 38 +++++++++++++++++---- tests/internal/hashtable.c | 48 +++++++++++++++++++++------ 5 files changed, 75 insertions(+), 20 deletions(-) diff --git a/include/fluent-bit/flb_hash.h b/include/fluent-bit/flb_hash.h index ecc6b861af5..d75574cbd2c 100644 --- a/include/fluent-bit/flb_hash.h +++ b/include/fluent-bit/flb_hash.h @@ -30,6 +30,7 @@ #define FLB_HASH_EVICT_NONE 0 #define FLB_HASH_EVICT_OLDER 1 #define FLB_HASH_EVICT_LESS_USED 2 +#define FLB_HASH_EVICT_RANDOM 3 struct flb_hash_entry { time_t created; @@ -38,6 +39,7 @@ struct flb_hash_entry { size_t key_len; char *val; size_t val_size; + struct flb_hash_table *table; /* link to parent flb_hash_table */ struct mk_list _head; /* link to flb_hash_table->chains */ struct mk_list _head_parent; /* link to flb_hash->entries */ }; diff --git a/plugins/filter_kubernetes/kube_conf.c b/plugins/filter_kubernetes/kube_conf.c index 58e68aea299..d4029d71011 100644 --- a/plugins/filter_kubernetes/kube_conf.c +++ b/plugins/filter_kubernetes/kube_conf.c @@ -169,8 +169,9 @@ struct flb_kube *flb_kube_conf_create(struct flb_filter_instance *i, ctx->api_https ? "https" : "http", ctx->api_host, ctx->api_port); - ctx->hash_table = flb_hash_create(FLB_HASH_TABLE_SIZE, - FLB_HASH_EVICT_NONE, -1); + ctx->hash_table = flb_hash_create(FLB_HASH_EVICT_RANDOM, + FLB_HASH_TABLE_SIZE, + FLB_HASH_TABLE_SIZE); if (!ctx->hash_table) { flb_kube_conf_destroy(ctx); return NULL; diff --git a/src/flb_env.c b/src/flb_env.c index 5b5cb8cd4a3..bd64a1f3b8e 100644 --- a/src/flb_env.c +++ b/src/flb_env.c @@ -99,7 +99,7 @@ struct flb_env *flb_env_create() } /* Create the hash-table */ - ht = flb_hash_create(FLB_ENV_SIZE, FLB_HASH_EVICT_NONE, -1); + ht = flb_hash_create(FLB_HASH_EVICT_NONE, FLB_ENV_SIZE, -1); if (!ht) { flb_free(env); return NULL; diff --git a/src/flb_hash.c b/src/flb_hash.c index 68591afbc61..652a6e32ff2 100644 --- a/src/flb_hash.c +++ b/src/flb_hash.c @@ -84,10 +84,12 @@ static unsigned int gen_hash(const void *key, int len) return (unsigned int) h; } -static inline void flb_hash_entry_free(struct flb_hash_entry *entry) +static inline void flb_hash_entry_free(struct flb_hash *ht, struct flb_hash_entry *entry) { mk_list_del(&entry->_head); mk_list_del(&entry->_head_parent); + entry->table->count--; + ht->total_count--; flb_free(entry->key); flb_free(entry->val); flb_free(entry); @@ -114,6 +116,7 @@ struct flb_hash *flb_hash_create(int evict_mode, size_t size, int max_entries) ht->max_entries = max_entries; ht->total_count = 0; ht->size = size; + ht->total_count = 0; ht->table = flb_calloc(1, sizeof(struct flb_hash_table) * size); if (!ht->table) { flb_errno(); @@ -143,8 +146,7 @@ void flb_hash_destroy(struct flb_hash *ht) table = &ht->table[i]; mk_list_foreach_safe(head, tmp, &table->chains) { entry = mk_list_entry(head, struct flb_hash_entry, _head); - flb_hash_entry_free(entry); - table->count--; + flb_hash_entry_free(ht, entry); } } @@ -152,6 +154,25 @@ void flb_hash_destroy(struct flb_hash *ht) flb_free(ht); } +static void flb_hash_evict_random(struct flb_hash *ht) +{ + int id; + int count = 0; + struct mk_list *tmp; + struct mk_list *head; + struct flb_hash_entry *entry; + + id = random() % ht->total_count; + mk_list_foreach_safe(head, tmp, &ht->entries) { + if (id == count) { + entry = mk_list_entry(head, struct flb_hash_entry, _head_parent); + flb_hash_entry_free(ht, entry); + break; + } + count++; + } +} + int flb_hash_add(struct flb_hash *ht, char *key, int key_len, char *val, size_t val_size) { @@ -179,6 +200,9 @@ int flb_hash_add(struct flb_hash *ht, char *key, int key_len, else if (ht->evict_mode == FLB_HASH_EVICT_LESS_USED) { } + else if (ht->evict_mode == FLB_HASH_EVICT_RANDOM) { + flb_hash_evict_random(ht); + } } /* Generate hash number */ @@ -215,6 +239,7 @@ int flb_hash_add(struct flb_hash *ht, char *key, int key_len, /* Link the new entry in our table at the end of the list */ table = &ht->table[id]; + entry->table = table; /* Check if the new key already exists */ if (table->count == 0) { @@ -225,8 +250,7 @@ int flb_hash_add(struct flb_hash *ht, char *key, int key_len, mk_list_foreach_safe(head, tmp, &table->chains) { old = mk_list_entry(head, struct flb_hash_entry, _head); if (strcmp(old->key, entry->key) == 0) { - flb_hash_entry_free(old); - table->count--; + flb_hash_entry_free(ht, old); break; } } @@ -235,6 +259,7 @@ int flb_hash_add(struct flb_hash *ht, char *key, int key_len, } table->count++; + ht->total_count++; return id; } @@ -382,8 +407,7 @@ int flb_hash_del(struct flb_hash *ht, char *key) return -1; } - flb_hash_entry_free(entry); - table->count--; + flb_hash_entry_free(ht, entry); return 0; } diff --git a/tests/internal/hashtable.c b/tests/internal/hashtable.c index 7f099346f66..dc830b145e6 100644 --- a/tests/internal/hashtable.c +++ b/tests/internal/hashtable.c @@ -79,7 +79,7 @@ void test_create_zero() { struct flb_hash *ht; - ht = flb_hash_create(0, FLB_HASH_EVICT_NONE, -1); + ht = flb_hash_create(FLB_HASH_EVICT_NONE, 0, -1); TEST_CHECK(ht == NULL); } @@ -91,7 +91,7 @@ void test_single() size_t out_size; struct flb_hash *ht; - ht = flb_hash_create(1, FLB_HASH_EVICT_NONE, -1); + ht = flb_hash_create(FLB_HASH_EVICT_NONE, 1, -1); TEST_CHECK(ht != NULL); ret = ht_add(ht, "key", "value"); @@ -112,7 +112,7 @@ void test_small_table() struct map *m; struct flb_hash *ht; - ht = flb_hash_create(1, FLB_HASH_EVICT_NONE, -1); + ht = flb_hash_create(FLB_HASH_EVICT_NONE, 8, -1); TEST_CHECK(ht != NULL); for (i = 0; i < sizeof(entries) / sizeof(struct map); i++) { @@ -129,7 +129,7 @@ void test_medium_table() struct map *m; struct flb_hash *ht; - ht = flb_hash_create(8, FLB_HASH_EVICT_NONE, -1); + ht = flb_hash_create(FLB_HASH_EVICT_NONE, 8, -1); TEST_CHECK(ht != NULL); for (i = 0; i < sizeof(entries) / sizeof(struct map); i++) { @@ -145,16 +145,16 @@ void test_chaining() int i; int inserts = 0; int count; - int total = 0; + int chains = 0; struct map *m; struct mk_list *head; struct flb_hash_table *table; struct flb_hash *ht; - ht = flb_hash_create(8, FLB_HASH_EVICT_NONE, -1); + ht = flb_hash_create(FLB_HASH_EVICT_NONE, 8, -1); TEST_CHECK(ht != NULL); - for (i = 0; i < sizeof(entries) / sizeof(struct map); i++) { + for (i = 0; i < 8; i++) { m = &entries[i]; ht_add(ht, m->key, m->val); inserts++; @@ -168,11 +168,13 @@ void test_chaining() } TEST_CHECK(count == table->count); - total += count; + if (count > 0) { + chains++; + } } /* Tests diff between total, new minus 3 overrides */ - TEST_CHECK(total == inserts - 3); + TEST_CHECK(chains == inserts - 3); flb_hash_destroy(ht); } @@ -187,7 +189,7 @@ void test_delete_all() struct flb_hash_table *table; struct flb_hash *ht; - ht = flb_hash_create(8, FLB_HASH_EVICT_NONE, -1); + ht = flb_hash_create(FLB_HASH_EVICT_NONE, 8, -1); TEST_CHECK(ht != NULL); total = sizeof(entries) / sizeof(struct map); @@ -214,6 +216,31 @@ void test_delete_all() flb_hash_destroy(ht); } +void test_random_eviction() +{ + int ret; + char *out_buf; + size_t out_size; + struct flb_hash *ht; + + ht = flb_hash_create(FLB_HASH_EVICT_RANDOM, 8, 1); + TEST_CHECK(ht != NULL); + + ret = ht_add(ht, "key1", "value1"); + TEST_CHECK(ret != -1); + + ret = ht_add(ht, "key2", "value2"); + TEST_CHECK(ret != -1); + + ret = flb_hash_get(ht, "key1", 4, &out_buf, &out_size); + TEST_CHECK(ret == -1); + + ret = flb_hash_get(ht, "key2", 4, &out_buf, &out_size); + TEST_CHECK(ret >= 0); + + flb_hash_destroy(ht); +} + TEST_LIST = { { "zero_size", test_create_zero }, { "single", test_single }, @@ -221,5 +248,6 @@ TEST_LIST = { { "medium_table", test_medium_table }, { "chaining_count", test_chaining }, { "delete_all", test_delete_all }, + { "random_eviction", test_random_eviction }, { 0 } };