From ee68633c22e5e22d05d9921bec27da4093ce1937 Mon Sep 17 00:00:00 2001 From: vraatikka Date: Mon, 5 Aug 2013 10:25:45 +0300 Subject: [PATCH 01/10] Removed unnecessary line --- log_manager/test/makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/log_manager/test/makefile b/log_manager/test/makefile index 25b493e5f..0330179c2 100644 --- a/log_manager/test/makefile +++ b/log_manager/test/makefile @@ -25,7 +25,6 @@ testcomp: -Wl,-rpath,$(DEST)/lib \ -Wl,-rpath,$(LOG_MANAGER_PATH)/ \ -o testlog -DSS_DEBUG \ - -I$(SOLIDDB_SRC_PATH)/include \ -I$(MARIADB_SRC_PATH)/include \ -I$(LOG_MANAGER_PATH) -I$(ROOT_PATH)/utils testlog.c \ -llog_manager $(LDLIBS) \ From 401d4920e41f9a79d96e43b3295f3c8e665cd330 Mon Sep 17 00:00:00 2001 From: vraatikka Date: Mon, 5 Aug 2013 11:51:10 +0300 Subject: [PATCH 02/10] Moved dcb_hashtable_stats from hastable.c to dcb.c to break dependency between hashtable and the rest of the maxscale. Added check fields to hashtable structure. --- server/core/dcb.c | 32 ++++++++++++++++++ server/core/hashtable.c | 68 ++++++++++++++++++++++++++------------ server/include/dcb.h | 1 + server/include/hashtable.h | 10 +++++- utils/skygw_debug.h | 9 ++++- 5 files changed, 97 insertions(+), 23 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 89f3fe901..1cebad9de 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -688,3 +688,35 @@ dcb_isclient(DCB *dcb) return 0; } + +/** + * Print hash table statistics to a DCB + * + * @param dcb The DCB to send the information to + * @param table The hash table + */ +void dcb_hashtable_stats( + DCB* dcb, + void* table) +{ + int total; + int longest; + int hashsize; + int i; + int j; + + total = 0; + longest = 0; + + hashtable_get_stats(table, &hashsize, &total, &longest); + + dcb_printf(dcb, + "Hashtable: %p, size %d\n", + table, + hashsize); + + dcb_printf(dcb, "\tNo. of entries: %d\n", total); + dcb_printf(dcb, "\tAverage chain length: %.1f\n", (float)total / hashsize); + dcb_printf(dcb, "\tLongest chain length: %d\n", longest); +} + diff --git a/server/core/hashtable.c b/server/core/hashtable.c index 15a1c3f3e..f81afb8ed 100644 --- a/server/core/hashtable.c +++ b/server/core/hashtable.c @@ -90,6 +90,10 @@ HASHTABLE *rval; if ((rval = malloc(sizeof(HASHTABLE))) == NULL) return NULL; + + rval->ht_chk_top = CHK_NUM_HASHTABLE; + rval->ht_chk_tail = CHK_NUM_HASHTABLE; + rval->hashsize = size; rval->hashfn = hashfn; rval->cmpfn = cmpfn; @@ -321,41 +325,63 @@ HASHENTRIES *entries; printf("\tLongest chain length: %d\n", longest); } -/** - * Print hash table statistics to a DCB +/** + * @node Produces stat output about hashtable + * + * Parameters: + * @param table - + * + * + * @param hashsize - + * + * + * @param nelems - + * + * + * @param longest - + * + * + * @return void + * + * + * @details (write detailed description here) * - * @param dcb The DCB to send the information to - * @param table The hash table */ -void -dcb_hashtable_stats(DCB *dcb, HASHTABLE *table) +void hashtable_get_stats( + void* table, + int* hashsize, + int* nelems, + int* longest) { -int total, longest, i, j; -HASHENTRIES *entries; + HASHTABLE* ht; + HASHENTRIES* entries; + int i; + int j; - dcb_printf(dcb, "Hashtable: %p, size %d\n", table, table->hashsize); - total = 0; - longest = 0; - hashtable_read_lock(table); - for (i = 0; i < table->hashsize; i++) + ht = (HASHTABLE *)table; + CHK_HASHTABLE(ht); + + hashtable_read_lock(ht); + + for (i = 0; i < ht->hashsize; i++) { j = 0; - entries = table->entries[i]; + entries = ht->entries[i]; while (entries) { j++; entries = entries->next; } - total += j; - if (j > longest) - longest = j; + *nelems += j; + if (j > *longest) { + *longest = j; + } } - hashtable_read_unlock(table); - dcb_printf(dcb, "\tNo. of entries: %d\n", total); - dcb_printf(dcb, "\tAverage chain length: %.1f\n", (float)total / table->hashsize); - dcb_printf(dcb, "\tLongest chain length: %d\n", longest); + *hashsize = ht->hashsize; + hashtable_read_unlock(ht); } + /** * Take a read lock on the hashtable. * diff --git a/server/include/dcb.h b/server/include/dcb.h index a47b439c9..31ae2e930 100644 --- a/server/include/dcb.h +++ b/server/include/dcb.h @@ -185,5 +185,6 @@ extern void dprintDCB(DCB *, DCB *); /* Debug to print a DCB in the system */ extern const char *gw_dcb_state2string(int); /* DCB state to string */ extern void dcb_printf(DCB *, const char *, ...); /* DCB version of printf */ extern int dcb_isclient(DCB *); /* the DCB is the client of the session */ +extern void dcb_hashtable_stats(DCB *, void *); /**< Print statisitics */ #endif diff --git a/server/include/hashtable.h b/server/include/hashtable.h index be7075c2c..71df1f681 100644 --- a/server/include/hashtable.h +++ b/server/include/hashtable.h @@ -31,6 +31,7 @@ * * @endverbatim */ +#include #include #include #include @@ -67,6 +68,7 @@ typedef void *(*HASHMEMORYFN)(void *); * The general purpose hashtable struct. */ typedef struct hashtable { + skygw_chk_t ht_chk_top; int hashsize; /**< The number of HASHENTRIES */ HASHENTRIES **entries; /**< The entries themselves */ int (*hashfn)(void *); /**< The hash function */ @@ -76,6 +78,7 @@ typedef struct hashtable { SPINLOCK spin; /**< Internal spinlock for the hashtable */ int n_readers; /**< Number of clients reading the table */ int writelock; /**< The table is locked by a writer */ + skygw_chk_t ht_chk_tail; } HASHTABLE; extern HASHTABLE *hashtable_alloc(int, int (*hashfn)(), int (*cmpfn)()); @@ -91,7 +94,12 @@ extern int hashtable_delete(HASHTABLE *, void *); extern void *hashtable_fetch(HASHTABLE *, void *); /**< Fetch the data for a given key */ extern void hashtable_stats(HASHTABLE *); /**< Print statisitics */ -extern void dcb_hashtable_stats(DCB *, HASHTABLE *); /**< Print statisitics */ +void hashtable_get_stats( + void* hashtable, + int* hashsize, + int* nelems, + int* longest); + extern HASHITERATOR *hashtable_iterator(HASHTABLE *); /**< Allocate an iterator on the hashtable */ extern void *hashtable_next(HASHITERATOR *); diff --git a/utils/skygw_debug.h b/utils/skygw_debug.h index 0aa96e3e7..87d30c707 100644 --- a/utils/skygw_debug.h +++ b/utils/skygw_debug.h @@ -113,7 +113,8 @@ typedef enum skygw_chk_t { CHK_NUM_FNAMES, CHK_NUM_LOGMANAGER, CHK_NUM_FILE, - CHK_NUM_BLOCKBUF + CHK_NUM_BLOCKBUF, + CHK_NUM_HASHTABLE } skygw_chk_t; # define STRBOOL(b) ((b) ? "TRUE" : "FALSE") @@ -319,4 +320,10 @@ typedef enum skygw_chk_t { "Block buf under- or overflow"); \ } +#define CHK_HASHTABLE(t) { \ + ss_info_dassert(t->ht_chk_top == CHK_NUM_HASHTABLE && \ + t->ht_chk_tail == CHK_NUM_HASHTABLE, \ + "Hashtable under- or overflow"); \ + } + #endif /* SKYGW_DEBUG_H */ From ef1c5144894360094fe9093db32cd21f2a251a24 Mon Sep 17 00:00:00 2001 From: vraatikka Date: Mon, 5 Aug 2013 13:50:52 +0300 Subject: [PATCH 03/10] Added creation of an empty depend.mk file to core directory. --- server/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/Makefile b/server/Makefile index 9a0657f46..4bb91be7b 100644 --- a/server/Makefile +++ b/server/Makefile @@ -42,7 +42,7 @@ clean: (cd modules/monitor; touch depend.mk ; make clean) depend: - (cd core; make depend) + (cd core; touch depend.mk ; make depend) (cd modules/routing; touch depend.mk ; make depend) (cd modules/protocol; touch depend.mk ; make depend) (cd modules/monitor; touch depend.mk ; make depend) From bc6fe8f6ef9ffb1fefe85115a99c0460c63d470f Mon Sep 17 00:00:00 2001 From: vraatikka Date: Mon, 5 Aug 2013 15:01:36 +0300 Subject: [PATCH 04/10] hashtable_add now checks input parameter and returns with zero (indicating that no elements were added) if hashsize is zero. Caused floating point exception. --- server/core/hashtable.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/server/core/hashtable.c b/server/core/hashtable.c index f81afb8ed..2f39c9852 100644 --- a/server/core/hashtable.c +++ b/server/core/hashtable.c @@ -167,9 +167,14 @@ hashtable_memory_fns(HASHTABLE *table, HASHMEMORYFN copyfn, HASHMEMORYFN freefn) int hashtable_add(HASHTABLE *table, void *key, void *value) { -int hashkey = table->hashfn(key) % table->hashsize; -HASHENTRIES *entry; + int hashkey; + HASHENTRIES *entry; + if (table->hashsize <= 0) { + return 0; + } else { + hashkey = table->hashfn(key) % table->hashsize; + } hashtable_write_lock(table); entry = table->entries[hashkey % table->hashsize]; while (entry && table->cmpfn(key, entry->key) != 0) From b852079631cdcf0c86cdae4cc3674eda12bfa727 Mon Sep 17 00:00:00 2001 From: vraatikka Date: Mon, 5 Aug 2013 17:47:21 +0300 Subject: [PATCH 05/10] Enabled debug assertions for DEBUG=Y builds. Fixed one debug assertion in log manager. --- log_manager/log_manager.cc | 4 ++-- utils/skygw_debug.h | 10 +++------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/log_manager/log_manager.cc b/log_manager/log_manager.cc index b3b484601..078d8b785 100644 --- a/log_manager/log_manager.cc +++ b/log_manager/log_manager.cc @@ -774,7 +774,7 @@ static char* blockbuf_get_writepos( ss_dassert(pos == NULL); ss_dassert(!(bb->bb_isfull || bb->bb_buf_left < str_len)); - ss_dassert(bb_list->mlist_nodecount <= nodecount_max); + ss_dassert(bb_list->mlist_nodecount <= bb_list->mlist_nodecount_max); /** * Registration to blockbuf adds reference for the write operation. @@ -816,7 +816,7 @@ static char* blockbuf_get_writepos( /** Unlock buffer */ simple_mutex_unlock(&bb->bb_mutex); - ss_dassert(bb_list->mlist_mutex->sm_lock_thr != pthread_self()); + ss_dassert(bb_list->mlist_mutex.sm_lock_thr != pthread_self()); return pos; } diff --git a/utils/skygw_debug.h b/utils/skygw_debug.h index 87d30c707..6bb0df780 100644 --- a/utils/skygw_debug.h +++ b/utils/skygw_debug.h @@ -47,15 +47,13 @@ # define ss_prof(exp) #endif /* SS_DEBUG || SS_PROF */ -#if defined(EI_SS_DEBUG) +#if defined(SS_DEBUG) + # define ss_debug(exp) exp # define ss_dfprintf fprintf # define ss_dfflush fflush # define ss_dfwrite fwrite -# undef ss_dassert -# undef ss_info_dassert -#if !defined(ss_dassert) # define ss_dassert(exp) \ { \ if (!(exp)) { \ @@ -67,9 +65,8 @@ assert(exp); \ } \ } -#endif /* !defined(ss_dassert) */ -#if !defined(ss_info_dassert) + # define ss_info_dassert(exp, info) \ { \ if (!(exp)) { \ @@ -81,7 +78,6 @@ assert((exp)); \ } \ } -#endif /* !defined(ss_info_dassert) */ #else /* SS_DEBUG */ From 13248c2a164eb595cd28b5621f090d6361a8fd7d Mon Sep 17 00:00:00 2001 From: vraatikka Date: Mon, 5 Aug 2013 18:04:02 +0300 Subject: [PATCH 06/10] Added server/core/test directory, makefile and testhash.c for testing hashtable implementation. In makefile, there is target all, which compiles and executes tests. --- server/core/test/makefile | 29 ++++++++++ server/core/test/testhash.c | 108 ++++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+) create mode 100644 server/core/test/makefile create mode 100644 server/core/test/testhash.c diff --git a/server/core/test/makefile b/server/core/test/makefile new file mode 100644 index 000000000..191dd25cf --- /dev/null +++ b/server/core/test/makefile @@ -0,0 +1,29 @@ +include ../../../build_gateway.inc +include ../../../makefile.inc + +CC=cc + +clean: + - $(DEL) *.o + - $(DEL) testhash + - $(DEL) *~ + +all: + $(MAKE) clean + $(MAKE) buildall + $(MAKE) runall + +buildall : + $(CC) $(CFLAGS) \ + -I$(ROOT_PATH)/server/include \ + -I$(ROOT_PATH)/utils \ + testhash.c ../hashtable.o ../atomic.o ../spinlock.o -o testhash \ + +runall: + - ./testhash 0 1 + - ./testhash 10 1 + - ./testhash 1000 10 + - ./testhash 10 0 + - ./testhash 1500 17 + - ./testhash 1 1 + diff --git a/server/core/test/testhash.c b/server/core/test/testhash.c new file mode 100644 index 000000000..3e98a0b2f --- /dev/null +++ b/server/core/test/testhash.c @@ -0,0 +1,108 @@ +#include +#include +#include + +#include "../../include/hashtable.h" + +static int hfun(void* key); +static int cmpfun (void *, void *); + +static int hfun( + void* key) +{ + return *(int *)key; +} + + +static int cmpfun( + void* v1, + void* v2) +{ + int i1; + int i2; + + i1 = *(int *)v1; + i2 = *(int *)v2; + + return (i1 < i2 ? -1 : (i1 > i2 ? 1 : 0)); +} + + + + +/** + * @node Simple test which creates hashtable and frees it. Size and number of entries + * sre specified by user and passed as arguments. + * + * Parameters: + * @param argc - + * + * + * @param argv - + * + * + * @return + * + * + * @details (write detailed description here) + * + */ +int main(int argc, char** argv) +{ + HASHTABLE* h; + int nelems; + int i; + int* val_arr; + int argsize; + int hsize; + int argelems; + int longest; + + if (argc != 3) { + fprintf(stderr, "\nWrong number of arguments. Usage " + ":\n\n\ttesthash <# of elements> <# hash size> " + " \n\n"); + return 1; + } + + argelems = strtol(argv[1], NULL, 10); + argsize = strtol(argv[2], NULL, 10); + + ss_dfprintf(stderr, + "testhash : creating hash table of size %d, including %d " + "elements in total.", + argsize, + argelems); + + val_arr = (int *)malloc(sizeof(void *)*argelems); + + h = hashtable_alloc(argsize, hfun, cmpfun); + + ss_dfprintf(stderr, "\t..done\nAdd %d elements to hash table.", argelems); + + for (i=0; i Date: Mon, 5 Aug 2013 18:15:41 +0300 Subject: [PATCH 07/10] Cleaned up some debug printings (to stdout). --- log_manager/log_manager.cc | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/log_manager/log_manager.cc b/log_manager/log_manager.cc index 078d8b785..62e9e0cf9 100644 --- a/log_manager/log_manager.cc +++ b/log_manager/log_manager.cc @@ -856,11 +856,12 @@ int skygw_log_write_flush( goto return_err; } CHK_LOGMANAGER(lm); +#if 0 ss_dfprintf(stderr, "skygw_log_write_flush writes to %s :\n\t%s.\n", STRLOGID(id), str); - +#endif /** * Find out the length of log string (to be formatted str). */ @@ -882,8 +883,9 @@ int skygw_log_write_flush( fprintf(stderr, "skygw_log_write_flush failed.\n"); goto return_unregister; } +#if 0 ss_dfprintf(stderr, "skygw_log_write_flush succeeed.\n"); - +#endif return_unregister: logmanager_unregister(); return_err: @@ -907,10 +909,12 @@ int skygw_log_write( goto return_err; } CHK_LOGMANAGER(lm); +#if 0 ss_dfprintf(stderr, "skygw_log_write writes to %s :\n\t%s.\n", STRLOGID(id), str); +#endif /** * Find out the length of log string (to be formatted str). */ @@ -932,9 +936,9 @@ int skygw_log_write( fprintf(stderr, "skygw_log_write failed.\n"); goto return_unregister; } - +#if 0 ss_dfprintf(stderr, "skygw_log_write succeeed.\n"); - +#endif return_unregister: logmanager_unregister(); return_err: @@ -960,9 +964,11 @@ int skygw_log_flush( fprintf(stderr, "skygw_log_flush failed.\n"); goto return_unregister; } +#if 0 ss_dfprintf(stderr, "skygw_log_flush : flushed %s successfully.\n", STRLOGID(id)); +#endif return_unregister: logmanager_unregister(); return_err: From 30456115f760c08155f1ff78f318cc1ef475f3f9 Mon Sep 17 00:00:00 2001 From: vraatikka Date: Mon, 5 Aug 2013 18:21:08 +0300 Subject: [PATCH 08/10] Memory used by user structure was not initialized. Changed malloc to calloc. --- server/core/adminusers.c | 3 +-- server/core/users.c | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/server/core/adminusers.c b/server/core/adminusers.c index a43a4b333..caed6ddeb 100644 --- a/server/core/adminusers.c +++ b/server/core/adminusers.c @@ -55,8 +55,7 @@ static char *ADMIN_ERR_FILEAPPEND = "Unable to append to password file"; static char *ADMIN_ERR_PWDFILEOPEN = "Failed to open password file"; static char *ADMIN_ERR_TMPFILEOPEN = "Failed to open temporary password file"; static char *ADMIN_ERR_PWDFILEACCESS = "Failed to access password file"; -static char *ADMIN_ERR_DELLASTUSER = "Deleting user failed, deleting the " - "last user is forbidden"; +static char *ADMIN_ERR_DELLASTUSER = "Deleting the last user is forbidden"; static char *ADMIN_SUCCESS = NULL; static const int LINELEN=80; diff --git a/server/core/users.c b/server/core/users.c index 63b8a3f6c..cc7646dc4 100644 --- a/server/core/users.c +++ b/server/core/users.c @@ -55,7 +55,7 @@ users_alloc() { USERS *rval; - if ((rval = malloc(sizeof(USERS))) == NULL) + if ((rval = calloc(1, sizeof(USERS))) == NULL) return NULL; if ((rval->data = hashtable_alloc(52, user_hash, strcmp)) == NULL) From 256b8e80130da564f1b175fd17fb506a05d07d6b Mon Sep 17 00:00:00 2001 From: vraatikka Date: Tue, 6 Aug 2013 11:14:45 +0300 Subject: [PATCH 09/10] Added more test cases and reformatted output a bit. --- server/core/test/makefile | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/server/core/test/makefile b/server/core/test/makefile index 191dd25cf..4706de30b 100644 --- a/server/core/test/makefile +++ b/server/core/test/makefile @@ -20,10 +20,13 @@ buildall : testhash.c ../hashtable.o ../atomic.o ../spinlock.o -o testhash \ runall: - - ./testhash 0 1 - - ./testhash 10 1 - - ./testhash 1000 10 - - ./testhash 10 0 - - ./testhash 1500 17 - - ./testhash 1 1 + - @./testhash 0 1 + - @./testhash 10 1 + - @./testhash 1000 10 + - @./testhash 10 0 + - @./testhash 1500 17 + - @./testhash 1 1 + - @./testhash 10000 133 + - @./testhash 1000 1000 + - @./testhash 1000 100000 From 84494822795d93f73406f17cf1d763cb4f8246c8 Mon Sep 17 00:00:00 2001 From: Massimiliano Pinto Date: Tue, 6 Aug 2013 10:55:20 +0200 Subject: [PATCH 10/10] Fixed typo --- server/core/adminusers.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/adminusers.c b/server/core/adminusers.c index caed6ddeb..07850bfc1 100644 --- a/server/core/adminusers.c +++ b/server/core/adminusers.c @@ -353,5 +353,5 @@ dcb_PrintAdminUsers(DCB *dcb) if (users) dcb_usersPrint(dcb, users); else - dcb_printf(dcb, "No administrtion users have been defined.\n"); + dcb_printf(dcb, "No administration users have been defined.\n"); }