From f041e6c25c069c0f39606bf56c93b7a9e90ee48f Mon Sep 17 00:00:00 2001 From: yifan hao Date: Sun, 3 May 2020 13:10:35 -0600 Subject: [PATCH 1/2] [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 2/2] 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);