From 2a7cd7ee64043b861bf372c8a801f88eed41c83f Mon Sep 17 00:00:00 2001 From: hzcheng Date: Sun, 3 May 2020 15:51:59 +0800 Subject: [PATCH 1/6] Add log to TSDB --- src/tsdb/inc/tsdbMain.h | 18 ++++++++++++++++++ src/tsdb/src/tsdbMain.c | 2 ++ 2 files changed, 20 insertions(+) diff --git a/src/tsdb/inc/tsdbMain.h b/src/tsdb/inc/tsdbMain.h index fb77975d25..39808ab02f 100644 --- a/src/tsdb/inc/tsdbMain.h +++ b/src/tsdb/inc/tsdbMain.h @@ -20,11 +20,29 @@ #include "tsdb.h" #include "tskiplist.h" #include "tutil.h" +#include "tlog.h" #ifdef __cplusplus extern "C" { #endif +extern int tsdbDebugFlag; + +#define tsdbError(...) \ + if (tsdbDebugFlag & DEBUG_ERROR) { \ + taosPrintLog("ERROR TSDB ", tsdbDebugFlag, __VA_ARGS__); \ + } +#define tsdbWarn(...) \ + if (tsdbDebugFlag & DEBUG_WARN) { \ + taosPrintLog("WARN TSDB ", tsdbDebugFlag, __VA_ARGS__); \ + } +#define tsdbTrace(...) \ + if (tsdbDebugFlag & DEBUG_TRACE) { \ + taosPrintLog("TSDB ", tsdbDebugFlag, __VA_ARGS__); \ + } +#define tsdbPrint(...) \ + { taosPrintLog("TSDB ", 255, __VA_ARGS__); } + // ------------------------------ TSDB META FILE INTERFACES ------------------------------ #define TSDB_META_FILE_NAME "META" #define TSDB_META_HASH_FRACTION 1.1 diff --git a/src/tsdb/src/tsdbMain.c b/src/tsdb/src/tsdbMain.c index fcfbcc9014..b1ef3d2d9c 100644 --- a/src/tsdb/src/tsdbMain.c +++ b/src/tsdb/src/tsdbMain.c @@ -7,6 +7,8 @@ #include "tscompression.h" #include "tchecksum.h" +int tsdbDebugFlag = 135; + #define TSDB_DEFAULT_PRECISION TSDB_PRECISION_MILLI // default precision #define IS_VALID_PRECISION(precision) (((precision) >= TSDB_PRECISION_MILLI) && ((precision) <= TSDB_PRECISION_NANO)) #define TSDB_DEFAULT_COMPRESSION TWO_STAGE_COMP From 26e0475808c41c749f82ca19a4bf6d556513dffc Mon Sep 17 00:00:00 2001 From: hzcheng Date: Sun, 3 May 2020 16:48:46 +0800 Subject: [PATCH 2/6] Fix TSDB commit invalid read error --- src/common/inc/tdataformat.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/inc/tdataformat.h b/src/common/inc/tdataformat.h index 489635420a..d655bdcfe3 100644 --- a/src/common/inc/tdataformat.h +++ b/src/common/inc/tdataformat.h @@ -174,7 +174,7 @@ typedef struct { #define keyCol(pCols) (&((pCols)->cols[0])) // Key column #define dataColsKeyAt(pCols, idx) ((TSKEY *)(keyCol(pCols)->pData))[(idx)] #define dataColsKeyFirst(pCols) dataColsKeyAt(pCols, 0) -#define dataColsKeyLast(pCols) dataColsKeyAt(pCols, (pCols)->numOfPoints - 1) +#define dataColsKeyLast(pCols) ((pCols->numOfPoints == 0) ? 0 : dataColsKeyAt(pCols, (pCols)->numOfPoints - 1)) SDataCols *tdNewDataCols(int maxRowSize, int maxCols, int maxRows); void tdResetDataCols(SDataCols *pCols); From f041e6c25c069c0f39606bf56c93b7a9e90ee48f Mon Sep 17 00:00:00 2001 From: yifan hao Date: Sun, 3 May 2020 13:10:35 -0600 Subject: [PATCH 3/6] [Trivial] Fix initialization code of TSDB_MOD_MGMT module. * Description This patch fixes enablement of TSDB_MOD_MGMT module, which was incorrectly assign to 'name' variable. * Testing Compile the code. Feel free to suggest more testing that needs to be done. Given that 'tsModule' is zero initialized, I don't think there's any functional difference for this patch. --- src/dnode/src/dnodeModule.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dnode/src/dnodeModule.c b/src/dnode/src/dnodeModule.c index aa53dddbe4..e1aa48d477 100644 --- a/src/dnode/src/dnodeModule.c +++ b/src/dnode/src/dnodeModule.c @@ -44,13 +44,13 @@ static void dnodeUnSetModuleStatus(int32_t module) { } static void dnodeAllocModules() { - tsModule[TSDB_MOD_MGMT].name = false; + tsModule[TSDB_MOD_MGMT].enable = false; tsModule[TSDB_MOD_MGMT].name = "mgmt"; tsModule[TSDB_MOD_MGMT].initFp = mgmtInitSystem; tsModule[TSDB_MOD_MGMT].cleanUpFp = mgmtCleanUpSystem; tsModule[TSDB_MOD_MGMT].startFp = mgmtStartSystem; tsModule[TSDB_MOD_MGMT].stopFp = mgmtStopSystem; - + tsModule[TSDB_MOD_HTTP].enable = (tsEnableHttpModule == 1); tsModule[TSDB_MOD_HTTP].name = "http"; tsModule[TSDB_MOD_HTTP].initFp = httpInitSystem; @@ -60,7 +60,7 @@ static void dnodeAllocModules() { if (tsEnableHttpModule) { dnodeSetModuleStatus(TSDB_MOD_HTTP); } - + tsModule[TSDB_MOD_MONITOR].enable = (tsEnableMonitorModule == 1); tsModule[TSDB_MOD_MONITOR].name = "monitor"; tsModule[TSDB_MOD_MONITOR].initFp = monitorInitSystem; From 9336fd3df1cc848f2b1944c0a8934cf1ef11bab2 Mon Sep 17 00:00:00 2001 From: yifan hao Date: Sun, 3 May 2020 14:46:09 -0600 Subject: [PATCH 4/6] Proper error handling in dnodeOpenVnodes / dnodeCloseVnodes * Description These two functions do not properly handle any error returned from dnodeGetVnodeList(). This patch als changes the signature of dnodeGetVnodeList() to make sure it returns only status, and number of vnodes is passed out through a separate argument. * Testing Before the patch, if I start taosd and then remove 'vnode' folder, the shutdown message would include the following: 05/03 14:26:17.158204 0x140025950476096 DND total vnodes:-2147483296 are all closed This shows the code uses error code as vnode count. After the patch, it outputs proper error message: 05/03 14:30:45.044762 0x139933723604800 DND Get dnode list failed --- src/dnode/src/dnodeMgmt.c | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/src/dnode/src/dnodeMgmt.c b/src/dnode/src/dnodeMgmt.c index 2fa7762c44..fbf1ceea71 100644 --- a/src/dnode/src/dnodeMgmt.c +++ b/src/dnode/src/dnodeMgmt.c @@ -71,13 +71,13 @@ void dnodeMgmt(SRpcMsg *pMsg) { rpcFreeCont(pMsg->pCont); } -static int32_t dnodeGetVnodeList(int32_t vnodeList[]) { +static int32_t dnodeGetVnodeList(int32_t vnodeList[], int32_t *numOfVnodes) { DIR *dir = opendir(tsVnodeDir); if (dir == NULL) { return TSDB_CODE_NO_WRITE_ACCESS; } - int32_t numOfVnodes = 0; + *numOfVnodes = 0; struct dirent *de = NULL; while ((de = readdir(dir)) != NULL) { if (strcmp(de->d_name, ".") == 0 || strcmp(de->d_name, "..") == 0) continue; @@ -86,21 +86,28 @@ static int32_t dnodeGetVnodeList(int32_t vnodeList[]) { int32_t vnode = atoi(de->d_name + 5); if (vnode == 0) continue; - vnodeList[numOfVnodes] = vnode; - numOfVnodes++; + vnodeList[*numOfVnodes] = vnode; + (*numOfVnodes)++; } } closedir(dir); - return numOfVnodes; + return TSDB_CODE_SUCCESS; } static int32_t dnodeOpenVnodes() { char vnodeDir[TSDB_FILENAME_LEN * 3]; int32_t failed = 0; - int32_t *vnodeList = (int32_t *)malloc(sizeof(int32_t) * TSDB_MAX_VNODES); - int32_t numOfVnodes = dnodeGetVnodeList(vnodeList); + int32_t numOfVnodes; + int32_t status; + + status = dnodeGetVnodeList(vnodeList, &numOfVnodes); + + if (status != TSDB_CODE_SUCCESS) { + dPrint("Get dnode list failed"); + return status; + } for (int32_t i = 0; i < numOfVnodes; ++i) { snprintf(vnodeDir, TSDB_FILENAME_LEN * 3, "%s/vnode%d", tsVnodeDir, vnodeList[i]); @@ -115,7 +122,15 @@ static int32_t dnodeOpenVnodes() { static void dnodeCloseVnodes() { int32_t *vnodeList = (int32_t *)malloc(sizeof(int32_t) * TSDB_MAX_VNODES); - int32_t numOfVnodes = dnodeGetVnodeList(vnodeList); + int32_t numOfVnodes; + int32_t status; + + status = dnodeGetVnodeList(vnodeList, &numOfVnodes); + + if (status != TSDB_CODE_SUCCESS) { + dPrint("Get dnode list failed"); + return; + } for (int32_t i = 0; i < numOfVnodes; ++i) { vnodeClose(vnodeList[i]); @@ -143,7 +158,7 @@ static int32_t dnodeProcessCreateVnodeMsg(SRpcMsg *rpcMsg) { for (int32_t j = 0; j < pCreate->cfg.replications; ++j) { pCreate->nodes[j].nodeId = htonl(pCreate->nodes[j].nodeId); } - + void *pVnode = vnodeAccquireVnode(pCreate->cfg.vgId); if (pVnode != NULL) { int32_t code = vnodeAlter(pVnode, pCreate); From 952eec14318b0f5027f5fee1c3db15a5b5ffc0b7 Mon Sep 17 00:00:00 2001 From: yifan hao Date: Sun, 3 May 2020 14:46:09 -0600 Subject: [PATCH 5/6] Proper error handling in dnodeOpenVnodes / dnodeCloseVnodes * Description These two functions do not properly handle any error returned from dnodeGetVnodeList(). This patch als changes the signature of dnodeGetVnodeList() to make sure it returns only status, and number of vnodes is passed out through a separate argument. * Testing Before the patch, if I start taosd and then remove 'vnode' folder, the shutdown message would include the following: 05/03 14:26:17.158204 0x140025950476096 DND total vnodes:-2147483296 are all closed This shows the code uses error code as vnode count. After the patch, it outputs proper error message: 05/03 14:30:45.044762 0x139933723604800 DND Get dnode list failed --- src/dnode/src/dnodeMgmt.c | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/src/dnode/src/dnodeMgmt.c b/src/dnode/src/dnodeMgmt.c index 2fa7762c44..fbf1ceea71 100644 --- a/src/dnode/src/dnodeMgmt.c +++ b/src/dnode/src/dnodeMgmt.c @@ -71,13 +71,13 @@ void dnodeMgmt(SRpcMsg *pMsg) { rpcFreeCont(pMsg->pCont); } -static int32_t dnodeGetVnodeList(int32_t vnodeList[]) { +static int32_t dnodeGetVnodeList(int32_t vnodeList[], int32_t *numOfVnodes) { DIR *dir = opendir(tsVnodeDir); if (dir == NULL) { return TSDB_CODE_NO_WRITE_ACCESS; } - int32_t numOfVnodes = 0; + *numOfVnodes = 0; struct dirent *de = NULL; while ((de = readdir(dir)) != NULL) { if (strcmp(de->d_name, ".") == 0 || strcmp(de->d_name, "..") == 0) continue; @@ -86,21 +86,28 @@ static int32_t dnodeGetVnodeList(int32_t vnodeList[]) { int32_t vnode = atoi(de->d_name + 5); if (vnode == 0) continue; - vnodeList[numOfVnodes] = vnode; - numOfVnodes++; + vnodeList[*numOfVnodes] = vnode; + (*numOfVnodes)++; } } closedir(dir); - return numOfVnodes; + return TSDB_CODE_SUCCESS; } static int32_t dnodeOpenVnodes() { char vnodeDir[TSDB_FILENAME_LEN * 3]; int32_t failed = 0; - int32_t *vnodeList = (int32_t *)malloc(sizeof(int32_t) * TSDB_MAX_VNODES); - int32_t numOfVnodes = dnodeGetVnodeList(vnodeList); + int32_t numOfVnodes; + int32_t status; + + status = dnodeGetVnodeList(vnodeList, &numOfVnodes); + + if (status != TSDB_CODE_SUCCESS) { + dPrint("Get dnode list failed"); + return status; + } for (int32_t i = 0; i < numOfVnodes; ++i) { snprintf(vnodeDir, TSDB_FILENAME_LEN * 3, "%s/vnode%d", tsVnodeDir, vnodeList[i]); @@ -115,7 +122,15 @@ static int32_t dnodeOpenVnodes() { static void dnodeCloseVnodes() { int32_t *vnodeList = (int32_t *)malloc(sizeof(int32_t) * TSDB_MAX_VNODES); - int32_t numOfVnodes = dnodeGetVnodeList(vnodeList); + int32_t numOfVnodes; + int32_t status; + + status = dnodeGetVnodeList(vnodeList, &numOfVnodes); + + if (status != TSDB_CODE_SUCCESS) { + dPrint("Get dnode list failed"); + return; + } for (int32_t i = 0; i < numOfVnodes; ++i) { vnodeClose(vnodeList[i]); @@ -143,7 +158,7 @@ static int32_t dnodeProcessCreateVnodeMsg(SRpcMsg *rpcMsg) { for (int32_t j = 0; j < pCreate->cfg.replications; ++j) { pCreate->nodes[j].nodeId = htonl(pCreate->nodes[j].nodeId); } - + void *pVnode = vnodeAccquireVnode(pCreate->cfg.vgId); if (pVnode != NULL) { int32_t code = vnodeAlter(pVnode, pCreate); From 0d09f6c0b61cf69ab61d15dacaa747c29785ef66 Mon Sep 17 00:00:00 2001 From: slguan Date: Mon, 4 May 2020 10:50:18 +0800 Subject: [PATCH 6/6] add scripts --- tests/script/basicSuite.sim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/script/basicSuite.sim b/tests/script/basicSuite.sim index c9c2023ffe..73d99e2b6d 100644 --- a/tests/script/basicSuite.sim +++ b/tests/script/basicSuite.sim @@ -20,7 +20,7 @@ run general/table/tinyint.sim run general/table/db.table.sim run general/user/basic1.sim -run general/user/pass_alter.sim +#run general/user/pass_alter.sim run general/user/pass_len.sim run general/user/user_create.sim run general/user/user_len.sim