From 411d034d29b5a705f5ce6341433f2c8d77e3b12c Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Mon, 13 Jul 2020 12:42:46 +0800 Subject: [PATCH] [td-225] fix bugs in cache. --- src/client/src/tscSql.c | 8 +-- src/util/src/tcache.c | 111 ++++++++++++++++++++++++++-------------- 2 files changed, 76 insertions(+), 43 deletions(-) diff --git a/src/client/src/tscSql.c b/src/client/src/tscSql.c index 9f422b2d76..874923aea7 100644 --- a/src/client/src/tscSql.c +++ b/src/client/src/tscSql.c @@ -481,10 +481,10 @@ static bool tscFreeQhandleInVnode(SSqlObj* pSql) { if (pRes->code == TSDB_CODE_SUCCESS && pRes->completed == false && !tscIsTwoStageSTableQuery(pQueryInfo, 0) && (pCmd->command == TSDB_SQL_SELECT || - pCmd->command == TSDB_SQL_SHOW || - pCmd->command == TSDB_SQL_RETRIEVE || - pCmd->command == TSDB_SQL_FETCH) && - (pCmd->command == TSDB_SQL_SELECT && pSql->pStream == NULL && pTableMetaInfo->pTableMeta != NULL)) { + pCmd->command == TSDB_SQL_SHOW || + pCmd->command == TSDB_SQL_RETRIEVE || + pCmd->command == TSDB_SQL_FETCH) && + (pSql->pStream == NULL && pTableMetaInfo->pTableMeta != NULL)) { pCmd->command = (pCmd->command > TSDB_SQL_MGMT) ? TSDB_SQL_RETRIEVE : TSDB_SQL_FETCH; tscDebug("%p send msg to dnode to free qhandle ASAP, command:%s, ", pSql, sqlCmd[pCmd->command]); diff --git a/src/util/src/tcache.c b/src/util/src/tcache.c index d3c622633d..df63d567c7 100644 --- a/src/util/src/tcache.c +++ b/src/util/src/tcache.c @@ -413,57 +413,90 @@ void taosCacheRelease(SCacheObj *pCacheObj, void **data, bool _remove) { *data = NULL; // note: extend lifespan before dec ref count - if (pCacheObj->extendLifespan) { + bool inTrashCan = pNode->inTrashCan; + + if (pCacheObj->extendLifespan && (!inTrashCan)) { atomic_store_64(&pNode->expireTime, pNode->lifespan + taosGetTimestampMs()); uDebug("cache:%s data:%p extend life time to %"PRId64 " before release", pCacheObj->name, pNode->data, pNode->expireTime); } - bool inTrashCan = pNode->inTrashCan; - uDebug("cache:%s, key:%p, %p is released, refcnt:%d", pCacheObj->name, pNode->key, pNode->data, T_REF_VAL_GET(pNode) - 1); - - // NOTE: once refcount is decrease, pNode may be free by other thread immediately. - int32_t ref = T_REF_DEC(pNode); - - if (inTrashCan) { - // Remove it if the ref count is 0. - // The ref count does not need to load and check again after lock acquired, since ref count can not be increased when - // the node is in trashcan. - if (ref == 0) { - __cache_wr_lock(pCacheObj); - assert(pNode->pTNodeHeader->pData == pNode); - taosRemoveFromTrashCan(pCacheObj, pNode->pTNodeHeader); - __cache_unlock(pCacheObj); + if (_remove) { + __cache_wr_lock(pCacheObj); + + // NOTE: once refcount is decrease, pNode may be freed by other thread immediately. + int32_t ref = T_REF_DEC(pNode); + uDebug("cache:%s, key:%p, %p is released, refcnt:%d", pCacheObj->name, pNode->key, pNode->data, ref); + + /* + * If it is not referenced by other users, remove it immediately. Otherwise move this node to trashcan wait for all users + * releasing this resources. + * + * NOTE: previous ref is 0, and current ref is still 0, remove it. If previous is not 0, there is another thread + * that tries to do the same thing. + */ + if (pNode->inTrashCan) { + if (ref == 0) { + assert(pNode->pTNodeHeader->pData == pNode); + taosRemoveFromTrashCan(pCacheObj, pNode->pTNodeHeader); + } + } else { + if (ref > 0) { + assert(pNode->pTNodeHeader == NULL); + taosCacheMoveToTrash(pCacheObj, pNode); + } else { + taosCacheReleaseNode(pCacheObj, pNode); + } } + __cache_unlock(pCacheObj); + } else { - assert(pNode->pTNodeHeader == NULL); - - if (_remove) { // not in trash can, but need to remove it - __cache_wr_lock(pCacheObj); - - /* - * If not referenced by other users. Otherwise move this node to trashcan wait for all users - * releasing this resources. - * - * NOTE: previous ref is 0, and current ref is still 0, remove it. If previous is not 0, there is another thread - * that tries to do the same thing. - */ + uDebug("cache:%s, key:%p, %p is released, refcnt:%d", pCacheObj->name, pNode->key, pNode->data, T_REF_VAL_GET(pNode) - 1); + + // NOTE: once refcount is decrease, pNode may be freed by other thread immediately. + int32_t ref = T_REF_DEC(pNode); + + if (inTrashCan) { + // Remove it if the ref count is 0. + // The ref count does not need to load and check again after lock acquired, since ref count can not be increased when + // the node is in trashcan. if (ref == 0) { - if (T_REF_VAL_GET(pNode) == 0) { - taosCacheReleaseNode(pCacheObj, pNode); - } else { - taosCacheMoveToTrash(pCacheObj, pNode); - } + __cache_wr_lock(pCacheObj); + assert(pNode->pTNodeHeader->pData == pNode); + taosRemoveFromTrashCan(pCacheObj, pNode->pTNodeHeader); + __cache_unlock(pCacheObj); } - __cache_unlock(pCacheObj); -// } else { // extend its life time -// if (pCacheObj->extendLifespan) { -// atomic_store_64(&pNode->expireTime, pNode->lifespan + taosGetTimestampMs()); -// uDebug("cache:%s data:%p extend life time to %"PRId64 " after release", pCacheObj->name, pNode->data, pNode->expireTime); -// } } } + +// else { +// if (_remove) { // not in trash can, but need to remove it +// __cache_wr_lock(pCacheObj); +// +// /* +// * If not referenced by other users. Otherwise move this node to trashcan wait for all users +// * releasing this resources. +// * +// * NOTE: previous ref is 0, and current ref is still 0, remove it. If previous is not 0, there is another thread +// * that tries to do the same thing. +// */ +// if (ref == 0) { +// if (T_REF_VAL_GET(pNode) == 0) { +// taosCacheReleaseNode(pCacheObj, pNode); +// } else { +// taosCacheMoveToTrash(pCacheObj, pNode); +// } +// } else if (ref > 0) { +// if (!pNode->inTrashCan) { +// assert(pNode->pTNodeHeader == NULL); +// taosCacheMoveToTrash(pCacheObj, pNode); +// } +// } +// +// __cache_unlock(pCacheObj); +// } +// } } void taosCacheEmpty(SCacheObj *pCacheObj) { -- GitLab