From fdc0febbe6e9cf8d41f50fcaae51ba8ff48d1120 Mon Sep 17 00:00:00 2001 From: Shengliang Guan Date: Thu, 18 Mar 2021 16:00:55 +0800 Subject: [PATCH] [TD-3355]: Fix invalid table id problem caused by idpool --- src/mnode/inc/mnodeVgroup.h | 2 +- src/mnode/src/mnodeSdb.c | 2 +- src/mnode/src/mnodeTable.c | 36 ++++++++++++++++++++++++++++++------ src/mnode/src/mnodeVgroup.c | 27 ++++++++++++++++++++------- src/util/inc/tidpool.h | 2 +- src/util/src/tidpool.c | 24 ++++++++++++++++++++---- 6 files changed, 73 insertions(+), 20 deletions(-) diff --git a/src/mnode/inc/mnodeVgroup.h b/src/mnode/inc/mnodeVgroup.h index 2067ad04cc..02dc42a1f8 100644 --- a/src/mnode/inc/mnodeVgroup.h +++ b/src/mnode/inc/mnodeVgroup.h @@ -44,7 +44,7 @@ void mnodeDropVgroup(SVgObj *pVgroup, void *ahandle); void mnodeAlterVgroup(SVgObj *pVgroup, void *ahandle); int32_t mnodeGetAvailableVgroup(struct SMnodeMsg *pMsg, SVgObj **pVgroup, int32_t *sid); -void mnodeAddTableIntoVgroup(SVgObj *pVgroup, SCTableObj *pTable); +int32_t mnodeAddTableIntoVgroup(SVgObj *pVgroup, SCTableObj *pTable); void mnodeRemoveTableFromVgroup(SVgObj *pVgroup, SCTableObj *pTable); void mnodeSendDropVnodeMsg(int32_t vgId, SRpcEpSet *epSet, void *ahandle); void mnodeSendCreateVgroupMsg(SVgObj *pVgroup, void *ahandle); diff --git a/src/mnode/src/mnodeSdb.c b/src/mnode/src/mnodeSdb.c index 319b16e62a..7810c9f91e 100644 --- a/src/mnode/src/mnodeSdb.c +++ b/src/mnode/src/mnodeSdb.c @@ -552,7 +552,7 @@ static int32_t sdbInsertHash(SSdbTable *pTable, SSdbRow *pRow) { int32_t code = (*pTable->fpInsert)(pRow); if (code != TSDB_CODE_SUCCESS) { - sdbError("vgId:1, sdb:%s, failed to insert key:%s to hash, remove it", pTable->name, + sdbError("vgId:1, sdb:%s, failed to perform insert action for key:%s, remove it", pTable->name, sdbGetRowStr(pTable, pRow->pObj)); sdbDeleteHash(pTable, pRow); } diff --git a/src/mnode/src/mnodeTable.c b/src/mnode/src/mnodeTable.c index 1378847d0b..65b0408a8f 100644 --- a/src/mnode/src/mnodeTable.c +++ b/src/mnode/src/mnodeTable.c @@ -108,10 +108,12 @@ static int32_t mnodeChildTableActionDestroy(SSdbRow *pRow) { static int32_t mnodeChildTableActionInsert(SSdbRow *pRow) { SCTableObj *pTable = pRow->pObj; + int32_t code = 0; SVgObj *pVgroup = mnodeGetVgroup(pTable->vgId); if (pVgroup == NULL) { mError("ctable:%s, not in vgId:%d", pTable->info.tableId, pTable->vgId); + code = -1; } SDbObj *pDb = NULL; @@ -119,6 +121,7 @@ static int32_t mnodeChildTableActionInsert(SSdbRow *pRow) { pDb = mnodeGetDb(pVgroup->dbName); if (pDb == NULL) { mError("ctable:%s, vgId:%d not in db:%s", pTable->info.tableId, pVgroup->vgId, pVgroup->dbName); + code = -1; } } @@ -127,6 +130,7 @@ static int32_t mnodeChildTableActionInsert(SSdbRow *pRow) { pAcct = mnodeGetAcct(pDb->acct); if (pAcct == NULL) { mError("ctable:%s, acct:%s not exists", pTable->info.tableId, pDb->acct); + code = -1; } } @@ -139,6 +143,7 @@ static int32_t mnodeChildTableActionInsert(SSdbRow *pRow) { if (pAcct) pAcct->acctInfo.numOfTimeSeries += (pTable->superTable->numOfColumns - 1); } else { mError("table:%s:%p, correspond stable not found suid:%" PRIu64, pTable->info.tableId, pTable, pTable->suid); + code = -1; } } else { grantAdd(TSDB_GRANT_TIMESERIES, pTable->numOfColumns - 1); @@ -146,18 +151,31 @@ static int32_t mnodeChildTableActionInsert(SSdbRow *pRow) { } if (pDb) mnodeAddTableIntoDb(pDb); - if (pVgroup) mnodeAddTableIntoVgroup(pVgroup, pTable); + if (pVgroup) { + if (mnodeAddTableIntoVgroup(pVgroup, pTable) != 0) { + mError("table:%s, vgId:%d tid:%d, failed to perform insert action, uid:%" PRIu64 " suid:%" PRIu64, + pTable->info.tableId, pTable->vgId, pTable->tid, pTable->uid, pTable->suid); + code = -1; + } + } mnodeDecVgroupRef(pVgroup); mnodeDecDbRef(pDb); mnodeDecAcctRef(pAcct); - return TSDB_CODE_SUCCESS; + if (code == 0) { + mTrace("table:%s, vgId:%d tid:%d, perform insert action, uid:%" PRIu64 " suid:%" PRIu64, pTable->info.tableId, + pTable->vgId, pTable->tid, pTable->uid, pTable->suid); + } + + return code; } static int32_t mnodeChildTableActionDelete(SSdbRow *pRow) { SCTableObj *pTable = pRow->pObj; if (pTable->vgId == 0) { + mError("table:%s, vgId:%d tid:%d, failed to perform delete action, uid:%" PRIu64 " suid:%" PRIu64, + pTable->info.tableId, pTable->vgId, pTable->tid, pTable->uid, pTable->suid); return TSDB_CODE_MND_VGROUP_NOT_EXIST; } @@ -188,6 +206,8 @@ static int32_t mnodeChildTableActionDelete(SSdbRow *pRow) { mnodeDecDbRef(pDb); mnodeDecAcctRef(pAcct); + mTrace("table:%s, vgId:%d tid:%d, perform delete action, uid:%" PRIu64 " suid:%" PRIu64, pTable->info.tableId, + pTable->vgId, pTable->tid, pTable->uid, pTable->suid); return TSDB_CODE_SUCCESS; } @@ -399,13 +419,13 @@ static void mnodeAddTableIntoStable(SSTableObj *pStable, SCTableObj *pCtable) { if (pStable->vgHash == NULL) { pStable->vgHash = taosHashInit(64, taosGetDefaultHashFunction(TSDB_DATA_TYPE_INT), true, HASH_ENTRY_LOCK); - mDebug("table:%s, create hash:%p", pStable->info.tableId, pStable->vgHash); + mDebug("stable:%s, create vgId hash:%p", pStable->info.tableId, pStable->vgHash); } if (pStable->vgHash != NULL) { if (taosHashGet(pStable->vgHash, &pCtable->vgId, sizeof(pCtable->vgId)) == NULL) { taosHashPut(pStable->vgHash, &pCtable->vgId, sizeof(pCtable->vgId), &pCtable->vgId, sizeof(pCtable->vgId)); - mDebug("table:%s, vgId:%d is put into stable hash:%p, sizeOfVgList:%d", pStable->info.tableId, pCtable->vgId, + mDebug("stable:%s, vgId:%d is put into stable vgId hash:%p, sizeOfVgList:%d", pStable->info.tableId, pCtable->vgId, pStable->vgHash, taosHashGetSize(pStable->vgHash)); } } @@ -443,19 +463,21 @@ static int32_t mnodeSuperTableActionDestroy(SSdbRow *pRow) { static int32_t mnodeSuperTableActionInsert(SSdbRow *pRow) { SSTableObj *pStable = pRow->pObj; - SDbObj *pDb = mnodeGetDbByTableName(pStable->info.tableId); + SDbObj * pDb = mnodeGetDbByTableName(pStable->info.tableId); if (pDb != NULL && pDb->status == TSDB_DB_STATUS_READY) { mnodeAddSuperTableIntoDb(pDb); } mnodeDecDbRef(pDb); taosHashPut(tsSTableUidHash, &pStable->uid, sizeof(int64_t), &pStable, sizeof(int64_t)); + + mTrace("stable:%s, perform insert action, uid:%" PRIu64, pStable->info.tableId, pStable->uid); return TSDB_CODE_SUCCESS; } static int32_t mnodeSuperTableActionDelete(SSdbRow *pRow) { SSTableObj *pStable = pRow->pObj; - SDbObj *pDb = mnodeGetDbByTableName(pStable->info.tableId); + SDbObj * pDb = mnodeGetDbByTableName(pStable->info.tableId); if (pDb != NULL) { mnodeRemoveSuperTableFromDb(pDb); mnodeDropAllChildTablesInStable((SSTableObj *)pStable); @@ -463,6 +485,8 @@ static int32_t mnodeSuperTableActionDelete(SSdbRow *pRow) { mnodeDecDbRef(pDb); taosHashRemove(tsSTableUidHash, &pStable->uid, sizeof(int64_t)); + + mTrace("stable:%s, perform delete action, uid:%" PRIu64, pStable->info.tableId, pStable->uid); return TSDB_CODE_SUCCESS; } diff --git a/src/mnode/src/mnodeVgroup.c b/src/mnode/src/mnodeVgroup.c index 008a655597..ef45936eaa 100644 --- a/src/mnode/src/mnodeVgroup.c +++ b/src/mnode/src/mnodeVgroup.c @@ -443,6 +443,7 @@ int32_t mnodeGetAvailableVgroup(SMnodeMsg *pMsg, SVgObj **ppVgroup, int32_t *pSi mDebug("msg:%p, app:%p db:%s, no enough sid in vgId:%d", pMsg, pMsg->rpcMsg.ahandle, pDb->name, pVgroup->vgId); continue; } + mTrace("vgId:%d, alloc tid:%d", pVgroup->vgId, sid); *pSid = sid; *ppVgroup = pVgroup; @@ -507,6 +508,7 @@ int32_t mnodeGetAvailableVgroup(SMnodeMsg *pMsg, SVgObj **ppVgroup, int32_t *pSi pDb->vgListIndex = 0; pthread_mutex_unlock(&pDb->mutex); + mTrace("vgId:%d, alloc tid:%d", pVgroup->vgId, sid); return TSDB_CODE_SUCCESS; } @@ -832,26 +834,37 @@ static int32_t mnodeRetrieveVgroups(SShowObj *pShow, char *data, int32_t rows, v return numOfRows; } -void mnodeAddTableIntoVgroup(SVgObj *pVgroup, SCTableObj *pTable) { +int32_t mnodeAddTableIntoVgroup(SVgObj *pVgroup, SCTableObj *pTable) { int32_t idPoolSize = taosIdPoolMaxSize(pVgroup->idPool); if (pTable->tid > idPoolSize) { mnodeAllocVgroupIdPool(pVgroup); } if (pTable->tid >= 1) { - taosIdPoolMarkStatus(pVgroup->idPool, pTable->tid); - pVgroup->numOfTables++; - // The create vgroup message may be received later than the create table message - // and the writing order in sdb is therefore uncertain - // which will cause the reference count of the vgroup to be incorrect when restarting - // mnodeIncVgroupRef(pVgroup); + if (taosIdPoolMarkStatus(pVgroup->idPool, pTable->tid)) { + pVgroup->numOfTables++; + mTrace("table:%s, vgId:%d tid:%d, mark tid used, uid:%" PRIu64, pTable->info.tableId, pTable->vgId, pTable->tid, + pTable->uid); + // The create vgroup message may be received later than the create table message + // and the writing order in sdb is therefore uncertain + // which will cause the reference count of the vgroup to be incorrect when restarting + // mnodeIncVgroupRef(pVgroup); + } else { + mError("table:%s, vgId:%d tid:%d, failed to mark tid, uid:%" PRIu64, pTable->info.tableId, pTable->vgId, + pTable->tid, pTable->uid); + return -1; + } } + + return 0; } void mnodeRemoveTableFromVgroup(SVgObj *pVgroup, SCTableObj *pTable) { if (pTable->tid >= 1) { taosFreeId(pVgroup->idPool, pTable->tid); pVgroup->numOfTables--; + mTrace("table:%s, vgId:%d tid:%d, put tid back uid:%" PRIu64, pTable->info.tableId, pTable->vgId, pTable->tid, + pTable->uid); // The create vgroup message may be received later than the create table message // and the writing order in sdb is therefore uncertain // which will cause the reference count of the vgroup to be incorrect when restarting diff --git a/src/util/inc/tidpool.h b/src/util/inc/tidpool.h index bf35251631..e4439439ce 100644 --- a/src/util/inc/tidpool.h +++ b/src/util/inc/tidpool.h @@ -34,7 +34,7 @@ void taosIdPoolCleanUp(void *handle); int taosIdPoolNumOfUsed(void *handle); -void taosIdPoolMarkStatus(void *handle, int id); +bool taosIdPoolMarkStatus(void *handle, int id); #ifdef __cplusplus } diff --git a/src/util/src/tidpool.c b/src/util/src/tidpool.c index 53d81bb542..3955960539 100644 --- a/src/util/src/tidpool.c +++ b/src/util/src/tidpool.c @@ -104,10 +104,16 @@ void taosIdPoolCleanUp(void *handle) { int taosIdPoolNumOfUsed(void *handle) { id_pool_t *pIdPool = handle; - return pIdPool->maxId - pIdPool->numOfFree; + + pthread_mutex_lock(&pIdPool->mutex); + int ret = pIdPool->maxId - pIdPool->numOfFree; + pthread_mutex_unlock(&pIdPool->mutex); + + return ret; } -void taosIdPoolMarkStatus(void *handle, int id) { +bool taosIdPoolMarkStatus(void *handle, int id) { + bool ret = false; id_pool_t *pIdPool = handle; pthread_mutex_lock(&pIdPool->mutex); @@ -115,9 +121,14 @@ void taosIdPoolMarkStatus(void *handle, int id) { if (!pIdPool->freeList[slot]) { pIdPool->freeList[slot] = true; pIdPool->numOfFree--; + ret = true; + } else { + ret = false; + uError("pool:%p, id:%d is already used by other obj", pIdPool, id); } pthread_mutex_unlock(&pIdPool->mutex); + return ret; } int taosUpdateIdPool(id_pool_t *handle, int maxId) { @@ -147,6 +158,11 @@ int taosUpdateIdPool(id_pool_t *handle, int maxId) { } int taosIdPoolMaxSize(void *handle) { - id_pool_t *pIdPool = (id_pool_t*)handle; - return pIdPool->maxId; + id_pool_t *pIdPool = (id_pool_t *)handle; + + pthread_mutex_lock(&pIdPool->mutex); + int ret = pIdPool->maxId; + pthread_mutex_unlock(&pIdPool->mutex); + + return ret; } \ No newline at end of file -- GitLab