From 5fb431aff4e172d50c76391a17b4f0dfda91d7dc Mon Sep 17 00:00:00 2001 From: sima Date: Mon, 19 Aug 2024 15:24:05 +0800 Subject: [PATCH 01/28] enh:[TD-31525] Remove ASSERT in libs/function. --- include/util/taoserror.h | 2 + source/libs/function/inc/thistogram.h | 2 +- source/libs/function/inc/tpercentile.h | 2 +- source/libs/function/src/builtinsimpl.c | 4 +- source/libs/function/src/thistogram.c | 89 +++++++++-------- source/libs/function/src/tpercentile.c | 126 ++++++++++++++++-------- source/util/src/terror.c | 2 + 7 files changed, 142 insertions(+), 85 deletions(-) diff --git a/include/util/taoserror.h b/include/util/taoserror.h index 1911c48d26..94b6d29cd9 100644 --- a/include/util/taoserror.h +++ b/include/util/taoserror.h @@ -878,6 +878,8 @@ int32_t taosGetErrSize(); #define TSDB_CODE_FUNC_INVALID_VALUE_RANGE TAOS_DEF_ERROR_CODE(0, 0x280C) #define TSDB_CODE_FUNC_SETUP_ERROR TAOS_DEF_ERROR_CODE(0, 0x280D) #define TSDB_CODE_FUNC_INVALID_RES_LENGTH TAOS_DEF_ERROR_CODE(0, 0x280E) +#define TSDB_CODE_FUNC_HISTOGRAM_ERROR TAOS_DEF_ERROR_CODE(0, 0x280F) +#define TSDB_CODE_FUNC_PERCENTILE_ERROR TAOS_DEF_ERROR_CODE(0, 0x2810) //udf diff --git a/source/libs/function/inc/thistogram.h b/source/libs/function/inc/thistogram.h index 5bc6a87c70..08bff7117e 100644 --- a/source/libs/function/inc/thistogram.h +++ b/source/libs/function/inc/thistogram.h @@ -59,7 +59,7 @@ int32_t tHistogramCreate(int32_t numOfEntries, SHistogramInfo** pHisto); SHistogramInfo* tHistogramCreateFrom(void* pBuf, int32_t numOfBins); int32_t tHistogramAdd(SHistogramInfo** pHisto, double val); -int64_t tHistogramSum(SHistogramInfo* pHisto, double v); +int32_t tHistogramSum(SHistogramInfo* pHisto, double v, int64_t *res); int32_t tHistogramUniform(SHistogramInfo* pHisto, double* ratio, int32_t num, double** pVal); int32_t tHistogramMerge(SHistogramInfo* pHisto1, SHistogramInfo* pHisto2, int32_t numOfEntries, diff --git a/source/libs/function/inc/tpercentile.h b/source/libs/function/inc/tpercentile.h index 118571c8aa..1b80c2b1da 100644 --- a/source/libs/function/inc/tpercentile.h +++ b/source/libs/function/inc/tpercentile.h @@ -47,7 +47,7 @@ typedef struct tMemBucketSlot { } tMemBucketSlot; struct tMemBucket; -typedef int32_t (*__perc_hash_func_t)(struct tMemBucket *pBucket, const void *value); +typedef int32_t (*__perc_hash_func_t)(struct tMemBucket *pBucket, const void *value, int32_t *index); typedef struct tMemBucket { int16_t numOfSlots; diff --git a/source/libs/function/src/builtinsimpl.c b/source/libs/function/src/builtinsimpl.c index 2d664e5d31..08abf41973 100644 --- a/source/libs/function/src/builtinsimpl.c +++ b/source/libs/function/src/builtinsimpl.c @@ -4018,7 +4018,9 @@ int32_t saveTupleData(SqlFunctionCtx* pCtx, int32_t rowIndex, const SSDataBlock* if (NULL == pColInfo) { return TSDB_CODE_OUT_OF_RANGE; } - ASSERT(pColInfo->info.type == TSDB_DATA_TYPE_TIMESTAMP); + if (pColInfo->info.type != TSDB_DATA_TYPE_TIMESTAMP) { + return TSDB_CODE_FUNC_FUNTION_PARA_TYPE; + } key.groupId = pSrcBlock->info.id.groupId; key.ts = *(int64_t*)colDataGetData(pColInfo, rowIndex); } diff --git a/source/libs/function/src/thistogram.c b/source/libs/function/src/thistogram.c index f57f6aa118..8594b0584c 100644 --- a/source/libs/function/src/thistogram.c +++ b/source/libs/function/src/thistogram.c @@ -14,6 +14,7 @@ */ #include "os.h" +#include "query.h" #include "taosdef.h" #include "thistogram.h" #include "tlosertree.h" @@ -81,9 +82,9 @@ int32_t tHistogramAdd(SHistogramInfo** pHisto, double val) { #if defined(USE_ARRAYLIST) int32_t idx = histoBinarySearch((*pHisto)->elems, (*pHisto)->numOfEntries, val); - if (ASSERTS(idx >= 0 && idx <= (*pHisto)->maxEntries && (*pHisto)->elems != NULL, "tHistogramAdd Error, idx:%d, maxEntries:%d, elems:%p", - idx, (*pHisto)->maxEntries, (*pHisto)->elems)) { - return TSDB_CODE_FAILED; + if (idx < 0 || idx > (*pHisto)->maxEntries || (*pHisto)->elems == NULL) { + qError("tHistogramAdd Error, idx:%d, maxEntries:%d, elems:%p", idx, (*pHisto)->maxEntries, (*pHisto)->elems); + return TSDB_CODE_FUNC_HISTOGRAM_ERROR; } if ((*pHisto)->elems[idx].val == val && idx >= 0) { @@ -95,21 +96,19 @@ int32_t tHistogramAdd(SHistogramInfo** pHisto, double val) { } else { /* insert a new slot */ if ((*pHisto)->numOfElems >= 1 && idx < (*pHisto)->numOfEntries) { if (idx > 0) { - if (ASSERTS((*pHisto)->elems[idx - 1].val <= val, "tHistogramAdd Error, elems[%d].val:%lf, val:%lf", - idx - 1, (*pHisto)->elems[idx - 1].val, val)) { - return TSDB_CODE_FAILED; + if ((*pHisto)->elems[idx - 1].val > val) { + qError("tHistogramAdd Error, elems[%d].val:%lf, val:%lf", idx - 1, (*pHisto)->elems[idx - 1].val, val); + return TSDB_CODE_FUNC_HISTOGRAM_ERROR; } } else { - if (ASSERTS((*pHisto)->elems[idx].val > val, "tHistogramAdd Error, elems[%d].val:%lf, val:%lf", - idx, (*pHisto)->elems[idx].val, val)) { - return TSDB_CODE_FAILED; + if ((*pHisto)->elems[idx].val <= val) { + qError("tHistogramAdd Error, elems[%d].val:%lf, val:%lf", idx, (*pHisto)->elems[idx].val, val); + return TSDB_CODE_FUNC_HISTOGRAM_ERROR; } } - } else if ((*pHisto)->numOfElems > 0) { - if (ASSERTS((*pHisto)->elems[(*pHisto)->numOfEntries].val <= val, "tHistogramAdd Error, elems[%d].val:%lf, val:%lf", - (*pHisto)->numOfEntries, (*pHisto)->elems[idx].val, val)) { - return TSDB_CODE_FAILED; - } + } else if ((*pHisto)->numOfElems > 0 && (*pHisto)->elems[(*pHisto)->numOfEntries].val > val) { + qError("tHistogramAdd Error, elems[%d].val:%lf, val:%lf", (*pHisto)->numOfEntries, (*pHisto)->elems[idx].val, val); + return TSDB_CODE_FUNC_HISTOGRAM_ERROR; } code = histogramCreateBin(*pHisto, idx, val); @@ -225,9 +224,9 @@ int32_t tHistogramAdd(SHistogramInfo** pHisto, double val) { tSkipListNode* pNext = pNode->pForward[0]; SHistBin* pNextEntry = (SHistBin*)pNext->pData; - if (ASSERTS(pNextEntry->val - pEntry->val == pEntry->delta, "tHistogramAdd Error, pNextEntry->val:%lf, pEntry->val:%lf, pEntry->delta:%lf", - pNextEntry->val, pEntry->val, pEntry->delta)) { - return -1; + if (pNextEntry->val - pEntry->val != pEntry->delta) { + qError("tHistogramAdd Error, pNextEntry->val:%lf, pEntry->val:%lf, pEntry->delta:%lf", pNextEntry->val, pEntry->val, pEntry->delta); + return TSDB_CODE_FUNC_HISTOGRAM_ERROR; } double newVal = (pEntry->val * pEntry->num + pNextEntry->val * pNextEntry->num) / (pEntry->num + pNextEntry->num); @@ -278,8 +277,9 @@ int32_t tHistogramAdd(SHistogramInfo** pHisto, double val) { } else { SHistBin* pEntry = (SHistBin*)pResNode->pData; - if (ASSERTS(pEntry->val == val, "tHistogramAdd Error, pEntry->val:%lf, val:%lf")) { - return -1; + if (pEntry->val != val) { + qError("tHistogramAdd Error, pEntry->val:%lf, val:%lf", pEntry->val, val); + return TSDB_CODE_FUNC_HISTOGRAM_ERROR; } pEntry->num += 1; } @@ -356,9 +356,9 @@ int32_t histogramCreateBin(SHistogramInfo* pHisto, int32_t index, double val) { (void)memmove(&pHisto->elems[index + 1], &pHisto->elems[index], sizeof(SHistBin) * remain); } - if (ASSERTS(index >= 0 && index <= pHisto->maxEntries, "histogramCreateBin Error, index:%d, maxEntries:%d", - index, pHisto->maxEntries)) { - return TSDB_CODE_FAILED; + if (index < 0 || index > pHisto->maxEntries) { + qError("histogramCreateBin Error, index:%d, maxEntries:%d", index, pHisto->maxEntries); + return TSDB_CODE_FUNC_HISTOGRAM_ERROR; } pHisto->elems[index].num = 1; @@ -373,9 +373,9 @@ int32_t histogramCreateBin(SHistogramInfo* pHisto, int32_t index, double val) { pHisto->elems[pHisto->maxEntries].num = 0; } #endif - if (ASSERTS(pHisto->numOfEntries <= pHisto->maxEntries, "histogramCreateBin Error, numOfEntries:%d, maxEntries:%d", - pHisto->numOfEntries, pHisto->maxEntries)) { - return TSDB_CODE_FAILED; + if (pHisto->numOfEntries > pHisto->maxEntries) { + qError("histogramCreateBin Error, numOfEntries:%d, maxEntries:%d", pHisto->numOfEntries, pHisto->maxEntries); + return TSDB_CODE_FUNC_HISTOGRAM_ERROR; } return TSDB_CODE_SUCCESS; @@ -411,8 +411,9 @@ void tHistogramPrint(SHistogramInfo* pHisto) { * Estimated number of points in the interval (−inf,b]. * @param pHisto * @param v + * @param res */ -int64_t tHistogramSum(SHistogramInfo* pHisto, double v) { +int32_t tHistogramSum(SHistogramInfo* pHisto, double v, int64_t *res) { #if defined(USE_ARRAYLIST) int32_t slotIdx = histoBinarySearch(pHisto->elems, pHisto->numOfEntries, v); if (pHisto->elems[slotIdx].val != v) { @@ -420,14 +421,18 @@ int64_t tHistogramSum(SHistogramInfo* pHisto, double v) { if (slotIdx < 0) { slotIdx = 0; - ASSERTS(v <= pHisto->elems[slotIdx].val, "tHistogramSum Error, elems[%d].val:%lf, v:%lf", - slotIdx, pHisto->elems[slotIdx].val, v); + if (v > pHisto->elems[slotIdx].val) { + qError("tHistogramSum Error, elems[%d].val:%lf, v:%lf", slotIdx, pHisto->elems[slotIdx].val, v); + return TSDB_CODE_FUNC_HISTOGRAM_ERROR; + } } else { - ASSERTS(v >= pHisto->elems[slotIdx].val, "tHistogramSum Error, elems[%d].val:%lf, v:%lf", - slotIdx, pHisto->elems[slotIdx].val, v); - if (slotIdx + 1 < pHisto->numOfEntries) { - ASSERTS(v < pHisto->elems[slotIdx + 1].val, "tHistogramSum Error, elems[%d].val:%lf, v:%lf", - slotIdx + 1, pHisto->elems[slotIdx + 1].val, v); + if (v < pHisto->elems[slotIdx].val) { + qError("tHistogramSum Error, elems[%d].val:%lf, v:%lf", slotIdx, pHisto->elems[slotIdx].val, v); + return TSDB_CODE_FUNC_HISTOGRAM_ERROR; + } + if (slotIdx + 1 < pHisto->numOfEntries && v >= pHisto->elems[slotIdx + 1].val) { + qError("tHistogramSum Error, elems[%d].val:%lf, v:%lf", slotIdx + 1, pHisto->elems[slotIdx + 1].val, v); + return TSDB_CODE_FUNC_HISTOGRAM_ERROR; } } } @@ -447,8 +452,9 @@ int64_t tHistogramSum(SHistogramInfo* pHisto, double v) { s1 = s1 + m1 / 2; - return (int64_t)s1; + *res = (int64_t)s1; #endif + return TSDB_CODE_SUCCESS; } int32_t tHistogramUniform(SHistogramInfo* pHisto, double* ratio, int32_t num, double** pVal) { @@ -484,9 +490,11 @@ int32_t tHistogramUniform(SHistogramInfo* pHisto, double* ratio, int32_t num, do j += 1; } - ASSERTS(total <= numOfElem && total + pHisto->elems[j + 1].num > numOfElem, - "tHistogramUniform Error, total:%ld, numOfElem:%ld, elems[%d].num:%ld", - total, (int64_t)numOfElem, j + 1, pHisto->elems[j + 1].num); + if (total > numOfElem || total + pHisto->elems[j + 1].num <= numOfElem) { + qError("tHistogramUniform Error, total:%d, numOfElem:%d, elems[%d].num:%d", + (int32_t)total, (int32_t)numOfElem, j + 1, (int32_t)pHisto->elems[j + 1].num); + return TSDB_CODE_FUNC_HISTOGRAM_ERROR; + } double delta = numOfElem - total; if (fabs(delta) < FLT_EPSILON) { @@ -545,9 +553,10 @@ int32_t tHistogramUniform(SHistogramInfo* pHisto, double* ratio, int32_t num, do j += 1; } - ASSERTS(total <= numOfElem && total + pEntry->num > numOfElem, - "tHistogramUniform Error, total:%d, numOfElem:%d, pEntry->num:%d", - total, numOfElem, pEntry->num); + if (total > numOfElem || total + pEntry->num <= numOfElem) { + qError("tHistogramUniform Error, total:%d, numOfElem:%d, pEntry->num:%d", (int32_t)total, (int32_t)numOfElem, (int32_t)pEntry->num); + return TSDB_CODE_FUNC_HISTOGRAM_ERROR; + } double delta = numOfElem - total; if (fabs(delta) < FLT_EPSILON) { diff --git a/source/libs/function/src/tpercentile.c b/source/libs/function/src/tpercentile.c index 4eefd150f3..ae0459427e 100644 --- a/source/libs/function/src/tpercentile.c +++ b/source/libs/function/src/tpercentile.c @@ -13,6 +13,7 @@ * along with this program. If not, see . */ +#include "query.h" #include "taoserror.h" #include "tcompare.h" #include "tglobal.h" @@ -107,7 +108,10 @@ static void resetPosInfo(SSlotInfo *pInfo) { } int32_t findOnlyResult(tMemBucket *pMemBucket, double *result) { - ASSERT(pMemBucket->total == 1); + if (pMemBucket->total != 1) { + qError("MemBucket:%p, total:%d, but only one element is expected", pMemBucket, pMemBucket->total); + return TSDB_CODE_FUNC_PERCENTILE_ERROR; + } terrno = 0; for (int32_t i = 0; i < pMemBucket->numOfSlots; ++i) { @@ -120,7 +124,10 @@ int32_t findOnlyResult(tMemBucket *pMemBucket, double *result) { SArray **pList = taosHashGet(pMemBucket->groupPagesMap, &groupId, sizeof(groupId)); if (pList != NULL) { SArray *list = *pList; - ASSERT(list->size == 1); + if (list->size != 1) { + qError("list:%p, total list size:%zu, but only one element is expected", list, list->size); + return TSDB_CODE_FUNC_PERCENTILE_ERROR; + } int32_t *pageId = taosArrayGet(list, 0); if (NULL == pageId) { @@ -130,7 +137,11 @@ int32_t findOnlyResult(tMemBucket *pMemBucket, double *result) { if (pPage == NULL) { return terrno; } - ASSERT(pPage->num == 1); + if (pPage->num != 1) { + qError("page:%p, total num:%d, but only one element is expected", pPage, pPage->num); + releaseBufPage(pMemBucket->pBuffer, pPage); + return TSDB_CODE_FUNC_PERCENTILE_ERROR; + } GET_TYPED_DATA(*result, double, pMemBucket->type, pPage->data); return TSDB_CODE_SUCCESS; @@ -141,64 +152,69 @@ int32_t findOnlyResult(tMemBucket *pMemBucket, double *result) { return TSDB_CODE_SUCCESS; } -int32_t tBucketIntHash(tMemBucket *pBucket, const void *value) { +int32_t tBucketIntHash(tMemBucket *pBucket, const void *value, int32_t *index) { int64_t v = 0; GET_TYPED_DATA(v, int64_t, pBucket->type, value); - int32_t index = -1; + *index = -1; if (v > pBucket->range.dMaxVal || v < pBucket->range.dMinVal) { - return index; + return TSDB_CODE_SUCCESS; } // divide the value range into 1024 buckets uint64_t span = pBucket->range.dMaxVal - pBucket->range.dMinVal; if (span < pBucket->numOfSlots) { int64_t delta = v - pBucket->range.dMinVal; - index = (delta % pBucket->numOfSlots); + *index = (delta % pBucket->numOfSlots); } else { double slotSpan = ((double)span) / pBucket->numOfSlots; uint64_t delta = (uint64_t)(v - pBucket->range.dMinVal); - index = delta / slotSpan; - if (v == pBucket->range.dMaxVal || index == pBucket->numOfSlots) { - index -= 1; + *index = delta / slotSpan; + if (v == pBucket->range.dMaxVal || *index == pBucket->numOfSlots) { + *index -= 1; } } - ASSERTS(index >= 0 && index < pBucket->numOfSlots, "tBucketIntHash Error, index:%d, numOfSlots:%d", - index, pBucket->numOfSlots); - return index; + if (*index < 0 || *index >= pBucket->numOfSlots) { + qError("tBucketIntHash Error, index:%d, numOfSlots:%d", *index, pBucket->numOfSlots); + return TSDB_CODE_FUNC_PERCENTILE_ERROR; + } + return TSDB_CODE_SUCCESS; } -int32_t tBucketUintHash(tMemBucket *pBucket, const void *value) { +int32_t tBucketUintHash(tMemBucket *pBucket, const void *value, int32_t *index) { int64_t v = 0; GET_TYPED_DATA(v, uint64_t, pBucket->type, value); - int32_t index = -1; + *index = -1; if (v > pBucket->range.u64MaxVal || v < pBucket->range.u64MinVal) { - return index; + return TSDB_CODE_SUCCESS; } // divide the value range into 1024 buckets uint64_t span = pBucket->range.u64MaxVal - pBucket->range.u64MinVal; if (span < pBucket->numOfSlots) { int64_t delta = v - pBucket->range.u64MinVal; - index = (int32_t)(delta % pBucket->numOfSlots); + *index = (int32_t)(delta % pBucket->numOfSlots); } else { double slotSpan = (double)span / pBucket->numOfSlots; - index = (int32_t)((v - pBucket->range.u64MinVal) / slotSpan); + *index = (int32_t)((v - pBucket->range.u64MinVal) / slotSpan); if (v == pBucket->range.u64MaxVal) { - index -= 1; + *index -= 1; } } - ASSERT(index >= 0 && index < pBucket->numOfSlots); - return index; + if (*index < 0 || *index >= pBucket->numOfSlots) { + qError("tBucketUintHash Error, index:%d, numOfSlots:%d", *index, pBucket->numOfSlots); + return TSDB_CODE_FUNC_PERCENTILE_ERROR; + } + return TSDB_CODE_SUCCESS; } -int32_t tBucketDoubleHash(tMemBucket *pBucket, const void *value) { +int32_t tBucketDoubleHash(tMemBucket *pBucket, const void *value, int32_t *index) { double v = 0; if (pBucket->type == TSDB_DATA_TYPE_FLOAT) { v = GET_FLOAT_VAL(value); @@ -206,27 +222,30 @@ int32_t tBucketDoubleHash(tMemBucket *pBucket, const void *value) { v = GET_DOUBLE_VAL(value); } - int32_t index = -1; + *index = -1; if (v > pBucket->range.dMaxVal || v < pBucket->range.dMinVal) { - return index; + return TSDB_CODE_SUCCESS; } // divide a range of [dMinVal, dMaxVal] into 1024 buckets double span = pBucket->range.dMaxVal - pBucket->range.dMinVal; if (span < pBucket->numOfSlots) { int32_t delta = (int32_t)(v - pBucket->range.dMinVal); - index = (delta % pBucket->numOfSlots); + *index = (delta % pBucket->numOfSlots); } else { double slotSpan = span / pBucket->numOfSlots; - index = (int32_t)((v - pBucket->range.dMinVal) / slotSpan); + *index = (int32_t)((v - pBucket->range.dMinVal) / slotSpan); if (v == pBucket->range.dMaxVal) { - index -= 1; + *index -= 1; } } - ASSERT(index >= 0 && index < pBucket->numOfSlots); - return index; + if (*index < 0 || *index >= pBucket->numOfSlots) { + qError("tBucketDoubleHash Error, index:%d, numOfSlots:%d", *index, pBucket->numOfSlots); + return TSDB_CODE_FUNC_PERCENTILE_ERROR; + } + return TSDB_CODE_SUCCESS; } static __perc_hash_func_t getHashFunc(int32_t type) { @@ -333,7 +352,7 @@ void tMemBucketDestroy(tMemBucket *pBucket) { taosMemoryFreeClear(pBucket); } -void tMemBucketUpdateBoundingBox(MinMaxEntry *r, const char *data, int32_t dataType) { +int32_t tMemBucketUpdateBoundingBox(MinMaxEntry *r, const char *data, int32_t dataType) { if (IS_SIGNED_NUMERIC_TYPE(dataType)) { int64_t v = 0; GET_TYPED_DATA(v, int64_t, dataType, data); @@ -368,8 +387,10 @@ void tMemBucketUpdateBoundingBox(MinMaxEntry *r, const char *data, int32_t dataT r->dMaxVal = v; } } else { - ASSERT(0); + qError("tMemBucketUpdateBoundingBox Error, invalid data type:%d", dataType); + return TSDB_CODE_FUNC_PERCENTILE_ERROR; } + return TSDB_CODE_SUCCESS; } /* @@ -378,9 +399,14 @@ void tMemBucketUpdateBoundingBox(MinMaxEntry *r, const char *data, int32_t dataT int32_t tMemBucketPut(tMemBucket *pBucket, const void *data, size_t size) { int32_t count = 0; int32_t bytes = pBucket->bytes; + int32_t code = TSDB_CODE_SUCCESS; for (int32_t i = 0; i < size; ++i) { char *d = (char *)data + i * bytes; - int32_t index = (pBucket->hashFunc)(pBucket, d); + int32_t index = -1; + code = (pBucket->hashFunc)(pBucket, d, &index); + if (TSDB_CODE_SUCCESS != code) { + return code; + } if (index < 0) { continue; } @@ -388,7 +414,10 @@ int32_t tMemBucketPut(tMemBucket *pBucket, const void *data, size_t size) { count += 1; tMemBucketSlot *pSlot = &pBucket->pSlots[index]; - tMemBucketUpdateBoundingBox(&pSlot->range, d, pBucket->type); + code = tMemBucketUpdateBoundingBox(&pSlot->range, d, pBucket->type); + if (TSDB_CODE_SUCCESS != code) { + return code; + } // ensure available memory pages to allocate int32_t groupId = getGroupId(pBucket->numOfSlots, index, pBucket->times); @@ -396,7 +425,11 @@ int32_t tMemBucketPut(tMemBucket *pBucket, const void *data, size_t size) { if (pSlot->info.data == NULL || pSlot->info.data->num >= pBucket->elemPerPage) { if (pSlot->info.data != NULL) { - ASSERT(pSlot->info.data->num >= pBucket->elemPerPage && pSlot->info.size > 0); + if (pSlot->info.data->num < pBucket->elemPerPage || pSlot->info.size <= 0) { + qError("tMemBucketPut failed since wrong pSLot info dataNum : %d, size : %d", + pSlot->info.data->num, pSlot->info.size); + return TSDB_CODE_FUNC_PERCENTILE_ERROR; + } // keep the pointer in memory setBufPageDirty(pSlot->info.data, true); @@ -411,7 +444,7 @@ int32_t tMemBucketPut(tMemBucket *pBucket, const void *data, size_t size) { if (NULL == pPageIdList) { return terrno; } - int32_t code = taosHashPut(pBucket->groupPagesMap, &groupId, sizeof(groupId), &pPageIdList, POINTER_BYTES); + code = taosHashPut(pBucket->groupPagesMap, &groupId, sizeof(groupId), &pPageIdList, POINTER_BYTES); if (TSDB_CODE_SUCCESS != code) { taosArrayDestroy(pPageIdList); return code; @@ -449,21 +482,23 @@ int32_t tMemBucketPut(tMemBucket *pBucket, const void *data, size_t size) { * j is the last slot of current segment, we need to get the first * slot of the next segment. */ -static MinMaxEntry getMinMaxEntryOfNextSlotWithData(tMemBucket *pMemBucket, int32_t slotIdx) { +static int32_t getMinMaxEntryOfNextSlotWithData(tMemBucket *pMemBucket, int32_t slotIdx, MinMaxEntry *next) { int32_t j = slotIdx + 1; while (j < pMemBucket->numOfSlots && (pMemBucket->pSlots[j].info.size == 0)) { ++j; } - ASSERT(j < pMemBucket->numOfSlots); - return pMemBucket->pSlots[j].range; + if (j >= pMemBucket->numOfSlots) { + qError("getMinMaxEntryOfNextSlotWithData can not get valid slot, start with slotIdx:%d", slotIdx); + return TSDB_CODE_FUNC_PERCENTILE_ERROR; + } + *next = pMemBucket->pSlots[j].range; + return TSDB_CODE_SUCCESS; } static bool isIdenticalData(tMemBucket *pMemBucket, int32_t index); static double getIdenticalDataVal(tMemBucket *pMemBucket, int32_t slotIndex) { - ASSERT(isIdenticalData(pMemBucket, slotIndex)); - tMemBucketSlot *pSlot = &pMemBucket->pSlots[slotIndex]; double finalResult = 0.0; @@ -494,7 +529,11 @@ int32_t getPercentileImpl(tMemBucket *pMemBucket, int32_t count, double fraction * now, we need to find the minimum value of the next slot for interpolating the percentile value * j is the last slot of current segment, we need to get the first slot of the next segment. */ - MinMaxEntry next = getMinMaxEntryOfNextSlotWithData(pMemBucket, i); + MinMaxEntry next; + int32_t code = getMinMaxEntryOfNextSlotWithData(pMemBucket, i, &next); + if (TSDB_CODE_SUCCESS != code) { + return code; + } double maxOfThisSlot = 0; double minOfNextSlot = 0; @@ -509,7 +548,10 @@ int32_t getPercentileImpl(tMemBucket *pMemBucket, int32_t count, double fraction minOfNextSlot = (double)next.dMinVal; } - ASSERT(minOfNextSlot > maxOfThisSlot); + if (minOfNextSlot <= maxOfThisSlot) { + qError("getPercentileImpl get minOfNextSlot : %f less equal than maxOfThisSlot : %f", minOfNextSlot, maxOfThisSlot); + return TSDB_CODE_FUNC_PERCENTILE_ERROR; + } *result = (1 - fraction) * maxOfThisSlot + fraction * minOfNextSlot; return TSDB_CODE_SUCCESS; diff --git a/source/util/src/terror.c b/source/util/src/terror.c index 396abf21a7..f2d15abb80 100644 --- a/source/util/src/terror.c +++ b/source/util/src/terror.c @@ -724,6 +724,8 @@ TAOS_DEFINE_ERROR(TSDB_CODE_FUNC_TIME_UNIT_TOO_SMALL, "Function time unit c TAOS_DEFINE_ERROR(TSDB_CODE_FUNC_INVALID_VALUE_RANGE, "Function got invalid value range") TAOS_DEFINE_ERROR(TSDB_CODE_FUNC_SETUP_ERROR, "Function set up failed") TAOS_DEFINE_ERROR(TSDB_CODE_FUNC_INVALID_RES_LENGTH, "Function result exceed max length") +TAOS_DEFINE_ERROR(TSDB_CODE_FUNC_HISTOGRAM_ERROR, "Function failed to calculate histogram") +TAOS_DEFINE_ERROR(TSDB_CODE_FUNC_PERCENTILE_ERROR, "Function failed to calculate percentile") //udf TAOS_DEFINE_ERROR(TSDB_CODE_UDF_STOPPING, "udf is stopping") From 800e7c4e7acd19ec33c2524d1fdde61e8ce64495 Mon Sep 17 00:00:00 2001 From: kailixu Date: Mon, 19 Aug 2024 17:27:15 +0800 Subject: [PATCH 02/28] fix: memory leak of geos --- source/client/src/clientMain.c | 2 + source/dnode/mgmt/node_mgmt/src/dmMgmt.c | 2 + source/util/src/tgeosctx.c | 92 +++++++++++++++++------- source/util/src/tsched.c | 2 +- source/util/src/tworker.c | 4 +- 5 files changed, 75 insertions(+), 27 deletions(-) diff --git a/source/client/src/clientMain.c b/source/client/src/clientMain.c index 12702a93f3..a403f9d1c2 100644 --- a/source/client/src/clientMain.c +++ b/source/client/src/clientMain.c @@ -23,6 +23,7 @@ #include "query.h" #include "scheduler.h" #include "tdatablock.h" +#include "tgeosctx.h" #include "tglobal.h" #include "tmsg.h" #include "tref.h" @@ -94,6 +95,7 @@ void taos_cleanup(void) { tmqMgmtClose(); DestroyRegexCache(); + destroyThreadLocalGeosCtx(); tscInfo("all local resources released"); taosCleanupCfg(); diff --git a/source/dnode/mgmt/node_mgmt/src/dmMgmt.c b/source/dnode/mgmt/node_mgmt/src/dmMgmt.c index fdce9fd4c9..1d62d4bd90 100644 --- a/source/dnode/mgmt/node_mgmt/src/dmMgmt.c +++ b/source/dnode/mgmt/node_mgmt/src/dmMgmt.c @@ -19,6 +19,7 @@ #include "index.h" #include "qworker.h" #include "tcompression.h" +#include "tgeosctx.h" #include "tglobal.h" #include "tgrant.h" #include "tstream.h" @@ -121,6 +122,7 @@ void dmCleanupDnode(SDnode *pDnode) { streamMetaCleanup(); indexCleanup(); taosConvDestroy(); + destroyThreadLocalGeosCtx(); // compress destroy tsCompressExit(); diff --git a/source/util/src/tgeosctx.c b/source/util/src/tgeosctx.c index a05734c911..47d5cc992b 100644 --- a/source/util/src/tgeosctx.c +++ b/source/util/src/tgeosctx.c @@ -15,38 +15,82 @@ #include "tgeosctx.h" #include "tdef.h" +#include "tlockfree.h" +#include "tlog.h" -static threadlocal SGeosContext tlGeosCtx = {0}; +typedef struct { + SGeosContext *pool; + int32_t capacity; + int32_t size; + SRWLatch lock; +} SGeosContextPool; -SGeosContext* getThreadLocalGeosCtx() { return &tlGeosCtx; } +static SGeosContextPool sGeosPool = {0}; -void destroyThreadLocalGeosCtx() { - if (tlGeosCtx.WKTReader) { - GEOSWKTReader_destroy_r(tlGeosCtx.handle, tlGeosCtx.WKTReader); - tlGeosCtx.WKTReader = NULL; +static threadlocal SGeosContext *tlGeosCtx = NULL; + +SGeosContext *getThreadLocalGeosCtx() { + if (tlGeosCtx) return tlGeosCtx; + + taosWLockLatch(&sGeosPool.lock); + if (sGeosPool.size >= sGeosPool.capacity) { + sGeosPool.capacity += 64; + void *tmp = taosMemoryRealloc(sGeosPool.pool, sGeosPool.capacity * sizeof(SGeosContext)); + if (!tmp) { + taosWUnLockLatch(&sGeosPool.lock); + terrno = TSDB_CODE_OUT_OF_MEMORY; + return NULL; + } + sGeosPool.pool = tmp; + TAOS_MEMSET(sGeosPool.pool + sGeosPool.size, 0, (sGeosPool.capacity - sGeosPool.size) * sizeof(SGeosContext)); } + tlGeosCtx = sGeosPool.pool + sGeosPool.size; + ++sGeosPool.size; + taosWUnLockLatch(&sGeosPool.lock); - if (tlGeosCtx.WKTWriter) { - GEOSWKTWriter_destroy_r(tlGeosCtx.handle, tlGeosCtx.WKTWriter); - tlGeosCtx.WKTWriter = NULL; - } + return tlGeosCtx; +} - if (tlGeosCtx.WKBReader) { - GEOSWKBReader_destroy_r(tlGeosCtx.handle, tlGeosCtx.WKBReader); - tlGeosCtx.WKBReader = NULL; - } +static void destroyGeosCtx(SGeosContext *pCtx) { + if (pCtx) { + if (pCtx->WKTReader) { + GEOSWKTReader_destroy_r(pCtx->handle, pCtx->WKTReader); + pCtx->WKTReader = NULL; + } - if (tlGeosCtx.WKBWriter) { - GEOSWKBWriter_destroy_r(tlGeosCtx.handle, tlGeosCtx.WKBWriter); - tlGeosCtx.WKBWriter = NULL; - } + if (pCtx->WKTWriter) { + GEOSWKTWriter_destroy_r(pCtx->handle, pCtx->WKTWriter); + pCtx->WKTWriter = NULL; + } - if (tlGeosCtx.WKTRegex) { - destroyRegexes(tlGeosCtx.WKTRegex, tlGeosCtx.WKTMatchData); - } + if (pCtx->WKBReader) { + GEOSWKBReader_destroy_r(pCtx->handle, pCtx->WKBReader); + pCtx->WKBReader = NULL; + } - if (tlGeosCtx.handle) { - GEOS_finish_r(tlGeosCtx.handle); - tlGeosCtx.handle = NULL; + if (pCtx->WKBWriter) { + GEOSWKBWriter_destroy_r(pCtx->handle, pCtx->WKBWriter); + pCtx->WKBWriter = NULL; + } + + if (pCtx->WKTRegex) { + destroyRegexes(pCtx->WKTRegex, pCtx->WKTMatchData); + pCtx->WKTRegex = NULL; + pCtx->WKTMatchData = NULL; + } + + if (pCtx->handle) { + GEOS_finish_r(pCtx->handle); + pCtx->handle = NULL; + } } } + +void destroyThreadLocalGeosCtx() { + uInfo("geos ctx is cleaned up"); + if (!sGeosPool.pool) return; + for (int32_t i = 0; i < sGeosPool.size; ++i) { + destroyGeosCtx(sGeosPool.pool + i); + } + taosMemoryFreeClear(sGeosPool.pool); +} \ No newline at end of file diff --git a/source/util/src/tsched.c b/source/util/src/tsched.c index 6779e8dee5..509dba0890 100644 --- a/source/util/src/tsched.c +++ b/source/util/src/tsched.c @@ -178,7 +178,7 @@ void *taosProcessSchedQueue(void *scheduler) { (*(msg.tfp))(msg.ahandle, msg.thandle); } - destroyThreadLocalGeosCtx(); + // destroyThreadLocalGeosCtx(); return NULL; } diff --git a/source/util/src/tworker.c b/source/util/src/tworker.c index b2064d6787..2da1abed78 100644 --- a/source/util/src/tworker.c +++ b/source/util/src/tworker.c @@ -105,7 +105,7 @@ static void *tQWorkerThreadFp(SQueueWorker *worker) { taosUpdateItemSize(qinfo.queue, 1); } - destroyThreadLocalGeosCtx(); + // destroyThreadLocalGeosCtx(); DestoryThreadLocalRegComp(); return NULL; @@ -665,7 +665,7 @@ static void *tQueryAutoQWorkerThreadFp(SQueryAutoQWorker *worker) { } } - destroyThreadLocalGeosCtx(); + // destroyThreadLocalGeosCtx(); DestoryThreadLocalRegComp(); return NULL; From 5ce7bcad1e42a37bd3e2fcf8ffb4252120aad5b0 Mon Sep 17 00:00:00 2001 From: Shengliang Guan Date: Mon, 19 Aug 2024 17:30:10 +0800 Subject: [PATCH 03/28] refact: remove the return value of function taosCloseRef --- .gitignore | 1 + include/util/tref.h | 3 +-- source/client/src/clientMain.c | 8 ++------ source/client/src/clientTmq.c | 2 +- source/dnode/vnode/src/sma/smaEnv.c | 4 ++-- source/libs/executor/src/executor.c | 2 +- source/libs/index/src/index.c | 2 +- source/libs/nodes/src/nodesUtilFuncs.c | 2 +- source/libs/qworker/src/qwUtil.c | 2 +- source/libs/scheduler/src/schUtil.c | 2 +- source/libs/stream/src/streamMeta.c | 6 +++--- source/libs/transport/src/transComm.c | 2 +- source/libs/wal/src/walMgmt.c | 2 +- source/util/src/tref.c | 6 ++---- 14 files changed, 19 insertions(+), 25 deletions(-) diff --git a/.gitignore b/.gitignore index f8b42f9176..334947a64c 100644 --- a/.gitignore +++ b/.gitignore @@ -134,3 +134,4 @@ tags .clangd *CMakeCache* *CMakeFiles* +.history/ diff --git a/include/util/tref.h b/include/util/tref.h index c4b2ec8fa7..1520ced14e 100644 --- a/include/util/tref.h +++ b/include/util/tref.h @@ -29,8 +29,7 @@ typedef void (*RefFp)(void *); int32_t taosOpenRef(int32_t max, RefFp fp); // close the reference set, refId is the return value by taosOpenRef -// return 0 if success. On error, -1 is returned, and terrno is set appropriately -int32_t taosCloseRef(int32_t rsetId); +void taosCloseRef(int32_t rsetId); // add ref, p is the pointer to resource or pointer ID // return Reference ID(rid) allocated. On error, -1 is returned, and terrno is set appropriately diff --git a/source/client/src/clientMain.c b/source/client/src/clientMain.c index 12702a93f3..d007dae7f7 100644 --- a/source/client/src/clientMain.c +++ b/source/client/src/clientMain.c @@ -74,15 +74,11 @@ void taos_cleanup(void) { int32_t id = clientReqRefPool; clientReqRefPool = -1; - if (TSDB_CODE_SUCCESS != taosCloseRef(id)) { - tscWarn("failed to close clientReqRefPool"); - } + taosCloseRef(id); id = clientConnRefPool; clientConnRefPool = -1; - if (TSDB_CODE_SUCCESS != taosCloseRef(id)) { - tscWarn("failed to close clientReqRefPool"); - } + taosCloseRef(id); nodesDestroyAllocatorSet(); cleanupAppInfo(); diff --git a/source/client/src/clientTmq.c b/source/client/src/clientTmq.c index 91883869e9..c9f166e565 100644 --- a/source/client/src/clientTmq.c +++ b/source/client/src/clientTmq.c @@ -1184,7 +1184,7 @@ void tmqMgmtClose(void) { } if (tmqMgmt.rsetId >= 0) { - (void)taosCloseRef(tmqMgmt.rsetId); + taosCloseRef(tmqMgmt.rsetId); tmqMgmt.rsetId = -1; } } diff --git a/source/dnode/vnode/src/sma/smaEnv.c b/source/dnode/vnode/src/sma/smaEnv.c index a219da33db..d90e869bd4 100644 --- a/source/dnode/vnode/src/sma/smaEnv.c +++ b/source/dnode/vnode/src/sma/smaEnv.c @@ -69,7 +69,7 @@ int32_t smaInit() { if (!smaMgmt.refHash || !smaMgmt.tmrHandle) { code = terrno; - (void)taosCloseRef(smaMgmt.rsetId); + taosCloseRef(smaMgmt.rsetId); if (smaMgmt.refHash) { taosHashCleanup(smaMgmt.refHash); smaMgmt.refHash = NULL; @@ -103,7 +103,7 @@ void smaCleanUp() { } if (old == 1) { - (void)taosCloseRef(smaMgmt.rsetId); + taosCloseRef(smaMgmt.rsetId); taosHashCleanup(smaMgmt.refHash); smaMgmt.refHash = NULL; taosTmrCleanUp(smaMgmt.tmrHandle); diff --git a/source/libs/executor/src/executor.c b/source/libs/executor/src/executor.c index b1c9207ab7..eb249575fc 100644 --- a/source/libs/executor/src/executor.c +++ b/source/libs/executor/src/executor.c @@ -31,7 +31,7 @@ int32_t exchangeObjRefPool = -1; static void cleanupRefPool() { int32_t ref = atomic_val_compare_exchange_32(&exchangeObjRefPool, exchangeObjRefPool, 0); - (void)taosCloseRef(ref); + taosCloseRef(ref); } static void initRefPool() { diff --git a/source/libs/index/src/index.c b/source/libs/index/src/index.c index 986693ab00..b881d2cac8 100644 --- a/source/libs/index/src/index.c +++ b/source/libs/index/src/index.c @@ -74,7 +74,7 @@ void indexCleanup() { // refacto later taosCleanUpScheduler(indexQhandle); taosMemoryFreeClear(indexQhandle); - (void)taosCloseRef(indexRefMgt); + taosCloseRef(indexRefMgt); } typedef struct SIdxColInfo { diff --git a/source/libs/nodes/src/nodesUtilFuncs.c b/source/libs/nodes/src/nodesUtilFuncs.c index 6b06530b3e..c1afc4afb3 100644 --- a/source/libs/nodes/src/nodesUtilFuncs.c +++ b/source/libs/nodes/src/nodesUtilFuncs.c @@ -238,7 +238,7 @@ void nodesDestroyAllocatorSet() { (void)taosRemoveRef(g_allocatorReqRefPool, refId); pAllocator = taosIterateRef(g_allocatorReqRefPool, refId); } - (void)taosCloseRef(g_allocatorReqRefPool); + taosCloseRef(g_allocatorReqRefPool); } } diff --git a/source/libs/qworker/src/qwUtil.c b/source/libs/qworker/src/qwUtil.c index 441714313c..4b9067a191 100644 --- a/source/libs/qworker/src/qwUtil.c +++ b/source/libs/qworker/src/qwUtil.c @@ -564,7 +564,7 @@ int32_t qwSaveTbVersionInfo(qTaskInfo_t pTaskInfo, SQWTaskCtx *ctx) { void qwCloseRef(void) { taosWLockLatch(&gQwMgmt.lock); if (atomic_load_32(&gQwMgmt.qwNum) <= 0 && gQwMgmt.qwRef >= 0) { - (void)taosCloseRef(gQwMgmt.qwRef); // ignore error + taosCloseRef(gQwMgmt.qwRef); // ignore error gQwMgmt.qwRef = -1; } taosWUnLockLatch(&gQwMgmt.lock); diff --git a/source/libs/scheduler/src/schUtil.c b/source/libs/scheduler/src/schUtil.c index 811890dde5..01249dbb98 100644 --- a/source/libs/scheduler/src/schUtil.c +++ b/source/libs/scheduler/src/schUtil.c @@ -273,7 +273,7 @@ void schCloseJobRef(void) { } if (schMgmt.jobRef >= 0) { - (void)taosCloseRef(schMgmt.jobRef); + taosCloseRef(schMgmt.jobRef); schMgmt.jobRef = -1; } } diff --git a/source/libs/stream/src/streamMeta.c b/source/libs/stream/src/streamMeta.c index a9976760b6..bc12f4307b 100644 --- a/source/libs/stream/src/streamMeta.c +++ b/source/libs/stream/src/streamMeta.c @@ -65,9 +65,9 @@ static void streamMetaEnvInit() { void streamMetaInit() { (void)taosThreadOnce(&streamMetaModuleInit, streamMetaEnvInit); } void streamMetaCleanup() { - (void)taosCloseRef(streamBackendId); - (void)taosCloseRef(streamBackendCfWrapperId); - (void)taosCloseRef(streamMetaId); + taosCloseRef(streamBackendId); + taosCloseRef(streamBackendCfWrapperId); + taosCloseRef(streamMetaId); metaRefMgtCleanup(); streamTimerCleanUp(); diff --git a/source/libs/transport/src/transComm.c b/source/libs/transport/src/transComm.c index b940c494d8..5d82e157b3 100644 --- a/source/libs/transport/src/transComm.c +++ b/source/libs/transport/src/transComm.c @@ -736,7 +736,7 @@ int32_t transOpenRefMgt(int size, void (*func)(void*)) { } void transCloseRefMgt(int32_t mgt) { // close ref - (void)taosCloseRef(mgt); + taosCloseRef(mgt); } int64_t transAddExHandle(int32_t refMgt, void* p) { // acquire extern handle diff --git a/source/libs/wal/src/walMgmt.c b/source/libs/wal/src/walMgmt.c index 581a63671c..e1d31ce113 100644 --- a/source/libs/wal/src/walMgmt.c +++ b/source/libs/wal/src/walMgmt.c @@ -75,7 +75,7 @@ void walCleanUp() { if (old == 1) { walStopThread(); - TAOS_UNUSED(taosCloseRef(tsWal.refSetId)); + taosCloseRef(tsWal.refSetId); wInfo("wal module is cleaned up"); atomic_store_8(&tsWal.inited, 0); } diff --git a/source/util/src/tref.c b/source/util/src/tref.c index f1d9a24757..4b1b477e06 100644 --- a/source/util/src/tref.c +++ b/source/util/src/tref.c @@ -110,13 +110,13 @@ int32_t taosOpenRef(int32_t max, RefFp fp) { return rsetId; } -int32_t taosCloseRef(int32_t rsetId) { +void taosCloseRef(int32_t rsetId) { SRefSet *pSet; int32_t deleted = 0; if (rsetId < 0 || rsetId >= TSDB_REF_OBJECTS) { uTrace("rsetId:%d is invalid, out of range", rsetId); - return terrno = TSDB_CODE_REF_INVALID_ID; + return; } pSet = tsRefSetList + rsetId; @@ -134,8 +134,6 @@ int32_t taosCloseRef(int32_t rsetId) { (void)taosThreadMutexUnlock(&tsRefMutex); if (deleted) taosDecRsetCount(pSet); - - return 0; } int64_t taosAddRef(int32_t rsetId, void *p) { From e296a2a076e2b443ca5e0a03190a7bad2c02e0f5 Mon Sep 17 00:00:00 2001 From: Shengliang Guan Date: Mon, 19 Aug 2024 18:04:29 +0800 Subject: [PATCH 04/28] fix: init refId before setting env start --- source/libs/sync/inc/syncEnv.h | 4 --- source/libs/sync/src/syncEnv.c | 58 ++++++++++++++++++++++++---------- 2 files changed, 41 insertions(+), 21 deletions(-) diff --git a/source/libs/sync/inc/syncEnv.h b/source/libs/sync/inc/syncEnv.h index 4dc5f58cfe..41ad915333 100644 --- a/source/libs/sync/inc/syncEnv.h +++ b/source/libs/sync/inc/syncEnv.h @@ -42,10 +42,6 @@ typedef struct SSyncEnv { // timer manager tmr_h pTimerManager; - - // other resources shared by SyncNodes - // ... - } SSyncEnv; SSyncEnv* syncEnv(); diff --git a/source/libs/sync/src/syncEnv.c b/source/libs/sync/src/syncEnv.c index 8d1e2cfebd..6d16a76d98 100644 --- a/source/libs/sync/src/syncEnv.c +++ b/source/libs/sync/src/syncEnv.c @@ -42,23 +42,25 @@ int32_t syncInit() { // start tmr thread gSyncEnv.pTimerManager = taosTmrInit(1000, 50, 10000, "SYNC-ENV"); - atomic_store_8(&gSyncEnv.isStart, 1); gNodeRefId = taosOpenRef(200, (RefFp)syncNodeClose); if (gNodeRefId < 0) { - sError("failed to init node ref"); + sError("failed to init node rset"); syncCleanUp(); return TSDB_CODE_SYN_WRONG_REF; } + sDebug("sync node rset is open, rsetId:%d", gNodeRefId); gHbDataRefId = taosOpenRef(200, (RefFp)syncHbTimerDataFree); if (gHbDataRefId < 0) { - sError("failed to init hb-data ref"); + sError("failed to init hbdata rset"); syncCleanUp(); return TSDB_CODE_SYN_WRONG_REF; } - sDebug("sync rsetId:%d is open", gNodeRefId); + sDebug("sync hbdata rset is open, rsetId:%d", gHbDataRefId); + + atomic_store_8(&gSyncEnv.isStart, 1); return 0; } @@ -68,32 +70,40 @@ void syncCleanUp() { memset(&gSyncEnv, 0, sizeof(SSyncEnv)); if (gNodeRefId != -1) { - sDebug("sync rsetId:%d is closed", gNodeRefId); - (void)taosCloseRef(gNodeRefId); + sDebug("sync node rset is closed, rsetId:%d", gNodeRefId); + taosCloseRef(gNodeRefId); gNodeRefId = -1; } if (gHbDataRefId != -1) { - sDebug("sync rsetId:%d is closed", gHbDataRefId); - (void)taosCloseRef(gHbDataRefId); + sDebug("sync hbdata rset is closed, rsetId:%d", gHbDataRefId); + taosCloseRef(gHbDataRefId); gHbDataRefId = -1; } } int64_t syncNodeAdd(SSyncNode *pNode) { pNode->rid = taosAddRef(gNodeRefId, pNode); - if (pNode->rid < 0) return -1; + if (pNode->rid < 0) { + terrno = TSDB_CODE_SYN_WRONG_REF; + return -1; + } - sDebug("vgId:%d, sync rid:%" PRId64 " is added to rsetId:%d", pNode->vgId, pNode->rid, gNodeRefId); + sDebug("vgId:%d, sync node refId:%" PRId64 " is added to rsetId:%d", pNode->vgId, pNode->rid, gNodeRefId); return pNode->rid; } -void syncNodeRemove(int64_t rid) { (void)taosRemoveRef(gNodeRefId, rid); } +void syncNodeRemove(int64_t rid) { + sDebug("sync node refId:%" PRId64 " is removed from rsetId:%d", rid, gNodeRefId); + if (rid > 0) { + (void)taosRemoveRef(gNodeRefId, rid); + } +} SSyncNode *syncNodeAcquire(int64_t rid) { SSyncNode *pNode = taosAcquireRef(gNodeRefId, rid); if (pNode == NULL) { - sError("failed to acquire node from refId:%" PRId64, rid); + sError("failed to acquire sync node from refId:%" PRId64 ", rsetId:%d", rid, gNodeRefId); terrno = TSDB_CODE_SYN_INTERNAL_ERROR; } @@ -101,28 +111,42 @@ SSyncNode *syncNodeAcquire(int64_t rid) { } void syncNodeRelease(SSyncNode *pNode) { - if (pNode) (void)taosReleaseRef(gNodeRefId, pNode->rid); + if (pNode) { + (void)taosReleaseRef(gNodeRefId, pNode->rid); + } } int64_t syncHbTimerDataAdd(SSyncHbTimerData *pData) { pData->rid = taosAddRef(gHbDataRefId, pData); - if (pData->rid < 0) return TSDB_CODE_SYN_WRONG_REF; + if (pData->rid < 0) { + terrno = TSDB_CODE_SYN_WRONG_REF; + return -1; + } + return pData->rid; } -void syncHbTimerDataRemove(int64_t rid) { (void)taosRemoveRef(gHbDataRefId, rid); } +void syncHbTimerDataRemove(int64_t rid) { + if (rid > 0) { + (void)taosRemoveRef(gHbDataRefId, rid); + } +} SSyncHbTimerData *syncHbTimerDataAcquire(int64_t rid) { SSyncHbTimerData *pData = taosAcquireRef(gHbDataRefId, rid); if (pData == NULL && rid > 0) { - sInfo("failed to acquire hb-timer-data from refId:%" PRId64, rid); + sInfo("failed to acquire hbdata from refId:%" PRId64 ", rsetId:%d", rid, gHbDataRefId); terrno = TSDB_CODE_SYN_INTERNAL_ERROR; } return pData; } -void syncHbTimerDataRelease(SSyncHbTimerData *pData) { (void)taosReleaseRef(gHbDataRefId, pData->rid); } +void syncHbTimerDataRelease(SSyncHbTimerData *pData) { + if (pData) { + (void)taosReleaseRef(gHbDataRefId, pData->rid); + } +} #if 0 void syncEnvStartTimer() { From aef00cfb8338e0cebe43f7d77cb55c0407fcd28c Mon Sep 17 00:00:00 2001 From: Shengliang Guan Date: Mon, 19 Aug 2024 18:04:50 +0800 Subject: [PATCH 05/28] refact: remove unused variables --- source/libs/sync/inc/syncEnv.h | 15 +----------- source/libs/sync/src/syncEnv.c | 45 ---------------------------------- 2 files changed, 1 insertion(+), 59 deletions(-) diff --git a/source/libs/sync/inc/syncEnv.h b/source/libs/sync/inc/syncEnv.h index 41ad915333..0376920e8a 100644 --- a/source/libs/sync/inc/syncEnv.h +++ b/source/libs/sync/inc/syncEnv.h @@ -23,25 +23,12 @@ extern "C" { #include "syncInt.h" #define TIMER_MAX_MS 0x7FFFFFFF -#define ENV_TICK_TIMER_MS 1000 #define PING_TIMER_MS 5000 -#define ELECT_TIMER_MS_MIN 2500 -#define HEARTBEAT_TIMER_MS 1000 #define HEARTBEAT_TICK_NUM 20 typedef struct SSyncEnv { uint8_t isStart; - - // tick timer - tmr_h pEnvTickTimer; - int32_t envTickTimerMS; - uint64_t envTickTimerLogicClock; // if use queue, should pass logic clock into queue item - uint64_t envTickTimerLogicClockUser; - TAOS_TMR_CALLBACK FpEnvTickTimer; // Timer Fp - uint64_t envTickTimerCounter; - - // timer manager - tmr_h pTimerManager; + tmr_h pTimerManager; } SSyncEnv; SSyncEnv* syncEnv(); diff --git a/source/libs/sync/src/syncEnv.c b/source/libs/sync/src/syncEnv.c index 6d16a76d98..7474cea83d 100644 --- a/source/libs/sync/src/syncEnv.c +++ b/source/libs/sync/src/syncEnv.c @@ -21,7 +21,6 @@ static SSyncEnv gSyncEnv = {0}; static int32_t gNodeRefId = -1; static int32_t gHbDataRefId = -1; -static void syncEnvTick(void *param, void *tmrId); SSyncEnv *syncEnv() { return &gSyncEnv; } @@ -34,13 +33,6 @@ int32_t syncInit() { taosSeedRand(seed); memset(&gSyncEnv, 0, sizeof(SSyncEnv)); - gSyncEnv.envTickTimerCounter = 0; - gSyncEnv.envTickTimerMS = ENV_TICK_TIMER_MS; - gSyncEnv.FpEnvTickTimer = syncEnvTick; - atomic_store_64(&gSyncEnv.envTickTimerLogicClock, 0); - atomic_store_64(&gSyncEnv.envTickTimerLogicClockUser, 0); - - // start tmr thread gSyncEnv.pTimerManager = taosTmrInit(1000, 50, 10000, "SYNC-ENV"); gNodeRefId = taosOpenRef(200, (RefFp)syncNodeClose); @@ -147,40 +139,3 @@ void syncHbTimerDataRelease(SSyncHbTimerData *pData) { (void)taosReleaseRef(gHbDataRefId, pData->rid); } } - -#if 0 -void syncEnvStartTimer() { - taosTmrReset(gSyncEnv.FpEnvTickTimer, gSyncEnv.envTickTimerMS, &gSyncEnv, gSyncEnv.pTimerManager, - &gSyncEnv.pEnvTickTimer); - atomic_store_64(&gSyncEnv.envTickTimerLogicClock, gSyncEnv.envTickTimerLogicClockUser); -} - -void syncEnvStopTimer() { - int32_t ret = 0; - atomic_add_fetch_64(&gSyncEnv.envTickTimerLogicClockUser, 1); - taosTmrStop(gSyncEnv.pEnvTickTimer); - gSyncEnv.pEnvTickTimer = NULL; - return ret; -} -#endif - -static void syncEnvTick(void *param, void *tmrId) { -#if 0 - SSyncEnv *pSyncEnv = param; - if (atomic_load_64(&gSyncEnv.envTickTimerLogicClockUser) <= atomic_load_64(&gSyncEnv.envTickTimerLogicClock)) { - gSyncEnv.envTickTimerCounter++; - sTrace("syncEnvTick do ... envTickTimerLogicClockUser:%" PRIu64 ", envTickTimerLogicClock:%" PRIu64 - ", envTickTimerCounter:%" PRIu64 ", envTickTimerMS:%d, tmrId:%p", - gSyncEnv.envTickTimerLogicClockUser, gSyncEnv.envTickTimerLogicClock, gSyncEnv.envTickTimerCounter, - gSyncEnv.envTickTimerMS, tmrId); - - // do something, tick ... - taosTmrReset(syncEnvTick, gSyncEnv.envTickTimerMS, pSyncEnv, gSyncEnv.pTimerManager, &gSyncEnv.pEnvTickTimer); - } else { - sTrace("syncEnvTick pass ... envTickTimerLogicClockUser:%" PRIu64 ", envTickTimerLogicClock:%" PRIu64 - ", envTickTimerCounter:%" PRIu64 ", envTickTimerMS:%d, tmrId:%p", - gSyncEnv.envTickTimerLogicClockUser, gSyncEnv.envTickTimerLogicClock, gSyncEnv.envTickTimerCounter, - gSyncEnv.envTickTimerMS, tmrId); - } -#endif -} From 0531a4f4bd7a26468232cbedfd1ae4b62c33aa0f Mon Sep 17 00:00:00 2001 From: kailixu Date: Mon, 19 Aug 2024 18:36:39 +0800 Subject: [PATCH 06/28] fix: memory leak of geos --- include/util/tgeosctx.h | 8 ++-- source/client/src/clientMain.c | 2 +- source/dnode/mgmt/node_mgmt/src/dmMgmt.c | 2 +- source/libs/executor/src/sysscanoperator.c | 2 +- source/libs/geometry/src/geosWrapper.c | 45 +++++++++++++++++++++- source/libs/parser/src/parInsertSql.c | 4 +- source/libs/scalar/src/sclvector.c | 8 ++-- source/util/src/tgeosctx.c | 17 +++++--- source/util/src/tsched.c | 2 - source/util/src/tworker.c | 2 - tools/shell/src/shellEngine.c | 5 +-- 11 files changed, 71 insertions(+), 26 deletions(-) diff --git a/include/util/tgeosctx.h b/include/util/tgeosctx.h index 267ba9e049..a4355db29a 100644 --- a/include/util/tgeosctx.h +++ b/include/util/tgeosctx.h @@ -32,14 +32,16 @@ typedef struct SGeosContext { GEOSWKBReader *WKBReader; GEOSWKBWriter *WKBWriter; - pcre2_code *WKTRegex; + pcre2_code *WKTRegex; pcre2_match_data *WKTMatchData; char errMsg[512]; } SGeosContext; -SGeosContext* getThreadLocalGeosCtx(); -void destroyThreadLocalGeosCtx(); +SGeosContext *acquireThreadLocalGeosCtx(); +SGeosContext *getThreadLocalGeosCtx(); +const char *getGeosErrMsg(int32_t code); +void taosGeosDestroy(); #ifdef __cplusplus } diff --git a/source/client/src/clientMain.c b/source/client/src/clientMain.c index a403f9d1c2..ec3147f49a 100644 --- a/source/client/src/clientMain.c +++ b/source/client/src/clientMain.c @@ -91,11 +91,11 @@ void taos_cleanup(void) { tscDebug("rpc cleanup"); taosConvDestroy(); + taosGeosDestroy(); tmqMgmtClose(); DestroyRegexCache(); - destroyThreadLocalGeosCtx(); tscInfo("all local resources released"); taosCleanupCfg(); diff --git a/source/dnode/mgmt/node_mgmt/src/dmMgmt.c b/source/dnode/mgmt/node_mgmt/src/dmMgmt.c index 1d62d4bd90..ae6efa7af4 100644 --- a/source/dnode/mgmt/node_mgmt/src/dmMgmt.c +++ b/source/dnode/mgmt/node_mgmt/src/dmMgmt.c @@ -122,7 +122,7 @@ void dmCleanupDnode(SDnode *pDnode) { streamMetaCleanup(); indexCleanup(); taosConvDestroy(); - destroyThreadLocalGeosCtx(); + taosGeosDestroy(); // compress destroy tsCompressExit(); diff --git a/source/libs/executor/src/sysscanoperator.c b/source/libs/executor/src/sysscanoperator.c index d8a2331980..082d4e7789 100644 --- a/source/libs/executor/src/sysscanoperator.c +++ b/source/libs/executor/src/sysscanoperator.c @@ -979,7 +979,7 @@ static int32_t sysTableGetGeomText(char* iGeom, int32_t nGeom, char** output, in if (TSDB_CODE_SUCCESS != (code = initCtxAsText()) || TSDB_CODE_SUCCESS != (code = doAsText(iGeom, nGeom, &outputWKT))) { - qError("geo text for systable failed:%s", getThreadLocalGeosCtx()->errMsg); + qError("geo text for systable failed:%s", getGeosErrMsg(code)); *output = NULL; *nOutput = 0; return code; diff --git a/source/libs/geometry/src/geosWrapper.c b/source/libs/geometry/src/geosWrapper.c index dde34edc91..4f3f7d75c2 100644 --- a/source/libs/geometry/src/geosWrapper.c +++ b/source/libs/geometry/src/geosWrapper.c @@ -23,7 +23,8 @@ typedef char (*_geosPreparedRelationFunc_t)(GEOSContextHandle_t handle, const GE void geosFreeBuffer(void *buffer) { if (buffer) { - GEOSFree_r(getThreadLocalGeosCtx()->handle, buffer); + SGeosContext *pCtx = acquireThreadLocalGeosCtx(); + if (pCtx) GEOSFree_r(pCtx->handle, buffer); } } @@ -36,6 +37,11 @@ int32_t initCtxMakePoint() { int32_t code = TSDB_CODE_FAILED; SGeosContext *geosCtx = getThreadLocalGeosCtx(); + if (!geosCtx) { + code = TSDB_CODE_OUT_OF_MEMORY; + return code; + } + if (geosCtx->handle == NULL) { geosCtx->handle = GEOS_init_r(); if (geosCtx->handle == NULL) { @@ -61,6 +67,11 @@ int32_t doMakePoint(double x, double y, unsigned char **outputGeom, size_t *size int32_t code = TSDB_CODE_FAILED; SGeosContext *geosCtx = getThreadLocalGeosCtx(); + if (!geosCtx) { + code = TSDB_CODE_OUT_OF_MEMORY; + return code; + } + GEOSGeometry *geom = NULL; unsigned char *wkb = NULL; @@ -166,6 +177,11 @@ int32_t initCtxGeomFromText() { int32_t code = TSDB_CODE_FAILED; SGeosContext *geosCtx = getThreadLocalGeosCtx(); + if (!geosCtx) { + code = TSDB_CODE_OUT_OF_MEMORY; + return code; + } + if (geosCtx->handle == NULL) { geosCtx->handle = GEOS_init_r(); if (geosCtx->handle == NULL) { @@ -202,6 +218,11 @@ int32_t doGeomFromText(const char *inputWKT, unsigned char **outputGeom, size_t int32_t code = TSDB_CODE_FAILED; SGeosContext *geosCtx = getThreadLocalGeosCtx(); + if (!geosCtx) { + code = TSDB_CODE_OUT_OF_MEMORY; + return code; + } + GEOSGeometry *geom = NULL; unsigned char *wkb = NULL; @@ -237,6 +258,11 @@ int32_t initCtxAsText() { int32_t code = TSDB_CODE_FAILED; SGeosContext *geosCtx = getThreadLocalGeosCtx(); + if (!geosCtx) { + code = TSDB_CODE_OUT_OF_MEMORY; + return code; + } + if (geosCtx->handle == NULL) { geosCtx->handle = GEOS_init_r(); if (geosCtx->handle == NULL) { @@ -273,6 +299,11 @@ int32_t doAsText(const unsigned char *inputGeom, size_t size, char **outputWKT) int32_t code = TSDB_CODE_FAILED; SGeosContext *geosCtx = getThreadLocalGeosCtx(); + if (!geosCtx) { + code = TSDB_CODE_OUT_OF_MEMORY; + return code; + } + GEOSGeometry *geom = NULL; char *wkt = NULL; @@ -304,6 +335,11 @@ int32_t initCtxRelationFunc() { int32_t code = TSDB_CODE_FAILED; SGeosContext *geosCtx = getThreadLocalGeosCtx(); + if (!geosCtx) { + code = TSDB_CODE_OUT_OF_MEMORY; + return code; + } + if (geosCtx->handle == NULL) { geosCtx->handle = GEOS_init_r(); if (geosCtx->handle == NULL) { @@ -329,6 +365,10 @@ int32_t doGeosRelation(const GEOSGeometry *geom1, const GEOSPreparedGeometry *pr _geosPreparedRelationFunc_t swappedPreparedRelationFn) { SGeosContext *geosCtx = getThreadLocalGeosCtx(); + if (!geosCtx) { + return TSDB_CODE_OUT_OF_MEMORY; + } + if (!preparedGeom1) { if (!swapped) { ASSERT(relationFn); @@ -390,6 +430,9 @@ int32_t doContainsProperly(const GEOSGeometry *geom1, const GEOSPreparedGeometry int32_t readGeometry(const unsigned char *input, GEOSGeometry **outputGeom, const GEOSPreparedGeometry **outputPreparedGeom) { SGeosContext *geosCtx = getThreadLocalGeosCtx(); + if (!geosCtx) { + return TSDB_CODE_OUT_OF_MEMORY; + } ASSERT(outputGeom); // it is not allowed if outputGeom is NULL *outputGeom = NULL; diff --git a/source/libs/parser/src/parInsertSql.c b/source/libs/parser/src/parInsertSql.c index cb94cd42f7..aa6116287e 100644 --- a/source/libs/parser/src/parInsertSql.c +++ b/source/libs/parser/src/parInsertSql.c @@ -655,7 +655,7 @@ static int32_t parseTagToken(const char** end, SToken* pToken, SSchema* pSchema, code = parseGeometry(pToken, &output, &size); if (code != TSDB_CODE_SUCCESS) { - code = buildSyntaxErrMsg(pMsgBuf, getThreadLocalGeosCtx()->errMsg, pToken->z); + code = buildSyntaxErrMsg(pMsgBuf, getGeosErrMsg(code), pToken->z); } else if (size + VARSTR_HEADER_SIZE > pSchema->bytes) { // Too long values will raise the invalid sql error message code = generateSyntaxErrMsg(pMsgBuf, TSDB_CODE_PAR_VALUE_TOO_LONG, pSchema->name); @@ -1646,7 +1646,7 @@ static int32_t parseValueTokenImpl(SInsertParseContext* pCxt, const char** pSql, code = parseGeometry(pToken, &output, &size); if (code != TSDB_CODE_SUCCESS) { - code = buildSyntaxErrMsg(&pCxt->msg, getThreadLocalGeosCtx()->errMsg, pToken->z); + code = buildSyntaxErrMsg(&pCxt->msg, getGeosErrMsg(code), pToken->z); } // Too long values will raise the invalid sql error message else if (size + VARSTR_HEADER_SIZE > pSchema->bytes) { diff --git a/source/libs/scalar/src/sclvector.c b/source/libs/scalar/src/sclvector.c index 71773ced57..daf44ec527 100644 --- a/source/libs/scalar/src/sclvector.c +++ b/source/libs/scalar/src/sclvector.c @@ -441,12 +441,12 @@ static FORCE_INLINE int32_t varToGeometry(char *buf, SScalarParam *pOut, int32_t unsigned char *t = NULL; char *output = NULL; - if (initCtxGeomFromText()) { - sclError("failed to init geometry ctx, %s", getThreadLocalGeosCtx()->errMsg); + if ((code = initCtxGeomFromText()) != 0) { + sclError("failed to init geometry ctx, %s", getGeosErrMsg(code)); SCL_ERR_JRET(TSDB_CODE_APP_ERROR); } - if (doGeomFromText(buf, &t, &len)) { - sclInfo("failed to convert text to geometry, %s", getThreadLocalGeosCtx()->errMsg); + if ((code = doGeomFromText(buf, &t, &len))) { + sclInfo("failed to convert text to geometry, %s", getGeosErrMsg(code)); SCL_ERR_JRET(TSDB_CODE_SCALAR_CONVERT_ERROR); } diff --git a/source/util/src/tgeosctx.c b/source/util/src/tgeosctx.c index 47d5cc992b..473b7539fc 100644 --- a/source/util/src/tgeosctx.c +++ b/source/util/src/tgeosctx.c @@ -25,16 +25,19 @@ typedef struct { SRWLatch lock; } SGeosContextPool; -static SGeosContextPool sGeosPool = {0}; - +static SGeosContextPool sGeosPool = {0}; static threadlocal SGeosContext *tlGeosCtx = NULL; +SGeosContext *acquireThreadLocalGeosCtx() { return tlGeosCtx; } + SGeosContext *getThreadLocalGeosCtx() { - if (tlGeosCtx) return tlGeosCtx; + if (tlGeosCtx) { + return tlGeosCtx; + } taosWLockLatch(&sGeosPool.lock); if (sGeosPool.size >= sGeosPool.capacity) { - sGeosPool.capacity += 64; + sGeosPool.capacity += 128; void *tmp = taosMemoryRealloc(sGeosPool.pool, sGeosPool.capacity * sizeof(SGeosContext)); if (!tmp) { taosWUnLockLatch(&sGeosPool.lock); @@ -51,6 +54,8 @@ SGeosContext *getThreadLocalGeosCtx() { return tlGeosCtx; } +const char *getGeosErrMsg(int32_t code) { return tlGeosCtx ? tlGeosCtx->errMsg : (code != 0 ? tstrerror(code) : ""); } + static void destroyGeosCtx(SGeosContext *pCtx) { if (pCtx) { if (pCtx->WKTReader) { @@ -86,8 +91,8 @@ static void destroyGeosCtx(SGeosContext *pCtx) { } } -void destroyThreadLocalGeosCtx() { - uInfo("geos ctx is cleaned up"); +void taosGeosDestroy() { + uInfo("geos is cleaned up"); if (!sGeosPool.pool) return; for (int32_t i = 0; i < sGeosPool.size; ++i) { destroyGeosCtx(sGeosPool.pool + i); diff --git a/source/util/src/tsched.c b/source/util/src/tsched.c index 509dba0890..55a927f340 100644 --- a/source/util/src/tsched.c +++ b/source/util/src/tsched.c @@ -178,8 +178,6 @@ void *taosProcessSchedQueue(void *scheduler) { (*(msg.tfp))(msg.ahandle, msg.thandle); } - // destroyThreadLocalGeosCtx(); - return NULL; } diff --git a/source/util/src/tworker.c b/source/util/src/tworker.c index 2da1abed78..ebec134c91 100644 --- a/source/util/src/tworker.c +++ b/source/util/src/tworker.c @@ -105,7 +105,6 @@ static void *tQWorkerThreadFp(SQueueWorker *worker) { taosUpdateItemSize(qinfo.queue, 1); } - // destroyThreadLocalGeosCtx(); DestoryThreadLocalRegComp(); return NULL; @@ -665,7 +664,6 @@ static void *tQueryAutoQWorkerThreadFp(SQueryAutoQWorker *worker) { } } - // destroyThreadLocalGeosCtx(); DestoryThreadLocalRegComp(); return NULL; diff --git a/tools/shell/src/shellEngine.c b/tools/shell/src/shellEngine.c index 0ccbd683dc..2c8330c433 100644 --- a/tools/shell/src/shellEngine.c +++ b/tools/shell/src/shellEngine.c @@ -611,14 +611,14 @@ void shellPrintGeometry(const unsigned char *val, int32_t length, int32_t width) code = initCtxAsText(); if (code != TSDB_CODE_SUCCESS) { - shellPrintString(getThreadLocalGeosCtx()->errMsg, width); + shellPrintString(getGeosErrMsg(code), width); return; } char *outputWKT = NULL; code = doAsText(val, length, &outputWKT); if (code != TSDB_CODE_SUCCESS) { - shellPrintString(getThreadLocalGeosCtx()->errMsg, width); // should NOT happen + shellPrintString(getGeosErrMsg(code), width); // should NOT happen return; } @@ -1282,7 +1282,6 @@ void *shellThreadLoop(void *arg) { taosResetTerminalMode(); } while (shellRunCommand(command, true) == 0); - destroyThreadLocalGeosCtx(); taosMemoryFreeClear(command); shellWriteHistory(); shellExit(); From 660bfde59370e531dd290c2d1ab61c65235a650f Mon Sep 17 00:00:00 2001 From: Shengliang Guan Date: Mon, 19 Aug 2024 19:16:37 +0800 Subject: [PATCH 07/28] refact: adjust util log --- source/libs/sync/inc/syncUtil.h | 2 -- source/libs/sync/src/syncEnv.c | 10 ++++---- source/libs/sync/src/syncUtil.c | 41 +++++++++++++++++---------------- 3 files changed, 25 insertions(+), 28 deletions(-) diff --git a/source/libs/sync/inc/syncUtil.h b/source/libs/sync/inc/syncUtil.h index a550ae8fbb..555607d40c 100644 --- a/source/libs/sync/inc/syncUtil.h +++ b/source/libs/sync/inc/syncUtil.h @@ -75,8 +75,6 @@ int32_t syncUtilElectRandomMS(int32_t min, int32_t max); int32_t syncUtilQuorum(int32_t replicaNum); const char* syncStr(ESyncState state); void syncUtilMsgHtoN(void* msg); -bool syncUtilUserPreCommit(tmsg_t msgType); -bool syncUtilUserRollback(tmsg_t msgType); void syncUtilGenerateArbToken(int32_t nodeId, int32_t groupId, char* buf); diff --git a/source/libs/sync/src/syncEnv.c b/source/libs/sync/src/syncEnv.c index 7474cea83d..1ebf47403a 100644 --- a/source/libs/sync/src/syncEnv.c +++ b/source/libs/sync/src/syncEnv.c @@ -32,7 +32,7 @@ int32_t syncInit() { uint32_t seed = (uint32_t)(taosGetTimestampNs() & 0x00000000FFFFFFFF); taosSeedRand(seed); - memset(&gSyncEnv, 0, sizeof(SSyncEnv)); + (void)memset(&gSyncEnv, 0, sizeof(SSyncEnv)); gSyncEnv.pTimerManager = taosTmrInit(1000, 50, 10000, "SYNC-ENV"); gNodeRefId = taosOpenRef(200, (RefFp)syncNodeClose); @@ -59,7 +59,7 @@ int32_t syncInit() { void syncCleanUp() { atomic_store_8(&gSyncEnv.isStart, 0); taosTmrCleanUp(gSyncEnv.pTimerManager); - memset(&gSyncEnv, 0, sizeof(SSyncEnv)); + (void)memset(&gSyncEnv, 0, sizeof(SSyncEnv)); if (gNodeRefId != -1) { sDebug("sync node rset is closed, rsetId:%d", gNodeRefId); @@ -77,8 +77,7 @@ void syncCleanUp() { int64_t syncNodeAdd(SSyncNode *pNode) { pNode->rid = taosAddRef(gNodeRefId, pNode); if (pNode->rid < 0) { - terrno = TSDB_CODE_SYN_WRONG_REF; - return -1; + return terrno = TSDB_CODE_SYN_WRONG_REF; } sDebug("vgId:%d, sync node refId:%" PRId64 " is added to rsetId:%d", pNode->vgId, pNode->rid, gNodeRefId); @@ -111,8 +110,7 @@ void syncNodeRelease(SSyncNode *pNode) { int64_t syncHbTimerDataAdd(SSyncHbTimerData *pData) { pData->rid = taosAddRef(gHbDataRefId, pData); if (pData->rid < 0) { - terrno = TSDB_CODE_SYN_WRONG_REF; - return -1; + return terrno = TSDB_CODE_SYN_WRONG_REF; } return pData->rid; diff --git a/source/libs/sync/src/syncUtil.c b/source/libs/sync/src/syncUtil.c index 49737b9045..ca879f70d9 100644 --- a/source/libs/sync/src/syncUtil.c +++ b/source/libs/sync/src/syncUtil.c @@ -23,7 +23,7 @@ #include "syncSnapshot.h" #include "tglobal.h" -void syncCfg2SimpleStr(const SSyncCfg* pCfg, char* buf, int32_t bufLen) { +static void syncCfg2SimpleStr(const SSyncCfg* pCfg, char* buf, int32_t bufLen) { int32_t len = snprintf(buf, bufLen, "{num:%d, as:%d, [", pCfg->replicaNum, pCfg->myIndex); for (int32_t i = 0; i < pCfg->replicaNum; ++i) { len += snprintf(buf + len, bufLen - len, "%s:%d", pCfg->nodeInfo[i].nodeFqdn, pCfg->nodeInfo[i].nodePort); @@ -43,14 +43,13 @@ void syncUtilNodeInfo2EpSet(const SNodeInfo* pInfo, SEpSet* pEpSet) { bool syncUtilNodeInfo2RaftId(const SNodeInfo* pInfo, SyncGroupId vgId, SRaftId* raftId) { uint32_t ipv4 = 0xFFFFFFFF; - sDebug( - "vgId:%d, start to resolve sync addr fqdn in %d seconds, " - "dnode:%d cluster:%" PRId64 " fqdn:%s port:%u ", - vgId, tsResolveFQDNRetryTime, pInfo->nodeId, pInfo->clusterId, pInfo->nodeFqdn, pInfo->nodePort); - for (int i = 0; i < tsResolveFQDNRetryTime; i++) { + sDebug("vgId:%d, resolve sync addr from fqdn, dnode:%d cluster:%" PRId64 " fqdn:%s port:%u", vgId, pInfo->nodeId, + pInfo->clusterId, pInfo->nodeFqdn, pInfo->nodePort); + for (int32_t i = 0; i < tsResolveFQDNRetryTime; i++) { int32_t code = taosGetIpv4FromFqdn(pInfo->nodeFqdn, &ipv4); if (code) { - sError("failed to resolve ipv4 addr, fqdn:%s, wait one second", pInfo->nodeFqdn); + sError("vgId:%d, failed to resolve sync addr, dnode:%d fqdn:%s, wait one second", vgId, pInfo->nodeId, + pInfo->nodeFqdn); taosSsleep(1); } else { break; @@ -58,7 +57,7 @@ bool syncUtilNodeInfo2RaftId(const SNodeInfo* pInfo, SyncGroupId vgId, SRaftId* } if (ipv4 == 0xFFFFFFFF || ipv4 == 1) { - sError("failed to resolve ipv4 addr, fqdn:%s", pInfo->nodeFqdn); + sError("vgId:%d, failed to resolve sync addr, fqdn:%s", vgId, pInfo->nodeFqdn); terrno = TSDB_CODE_TSC_INVALID_FQDN; return false; } @@ -68,14 +67,20 @@ bool syncUtilNodeInfo2RaftId(const SNodeInfo* pInfo, SyncGroupId vgId, SRaftId* raftId->addr = SYNC_ADDR(pInfo); raftId->vgId = vgId; - sInfo("vgId:%d, sync addr:%" PRIu64 ", dnode:%d cluster:%" PRId64 " fqdn:%s ip:%s port:%u ipv4:%u", vgId, - raftId->addr, pInfo->nodeId, pInfo->clusterId, pInfo->nodeFqdn, ipbuf, pInfo->nodePort, ipv4); + sInfo("vgId:%d, sync addr:%" PRIu64 " is resolved, dnode:%d cluster:%" PRId64 " fqdn:%s port:%u ip:%s ipv4:%u", vgId, + raftId->addr, pInfo->nodeId, pInfo->clusterId, pInfo->nodeFqdn, pInfo->nodePort, ipbuf, ipv4); return true; } bool syncUtilSameId(const SRaftId* pId1, const SRaftId* pId2) { - if (pId1->addr == pId2->addr && pId1->vgId == pId2->vgId) return true; - if ((CID(pId1) == 0 || CID(pId2) == 0) && (DID(pId1) == DID(pId2)) && pId1->vgId == pId2->vgId) return true; + if (pId1->addr == pId2->addr && pId1->vgId == pId2->vgId) { + return true; + } + + if ((CID(pId1) == 0 || CID(pId2) == 0) && (DID(pId1) == DID(pId2)) && pId1->vgId == pId2->vgId) { + return true; + } + return false; } @@ -98,10 +103,6 @@ void syncUtilMsgHtoN(void* msg) { pHead->vgId = htonl(pHead->vgId); } -bool syncUtilUserPreCommit(tmsg_t msgType) { return msgType != TDMT_SYNC_NOOP && msgType != TDMT_SYNC_LEADER_TRANSFER; } - -bool syncUtilUserRollback(tmsg_t msgType) { return msgType != TDMT_SYNC_NOOP && msgType != TDMT_SYNC_LEADER_TRANSFER; } - void syncUtilGenerateArbToken(int32_t nodeId, int32_t groupId, char* buf) { (void)memset(buf, 0, TSDB_ARB_TOKEN_SIZE); int32_t randVal = taosSafeRand() % 1000; @@ -142,18 +143,18 @@ static void syncLogBufferStates2Str(SSyncNode* pSyncNode, char* buf, int32_t buf if (pBuf == NULL) { return; } - int len = 0; + int32_t len = 0; len += snprintf(buf + len, bufLen - len, "[%" PRId64 " %" PRId64 " %" PRId64 ", %" PRId64 ")", pBuf->startIndex, pBuf->commitIndex, pBuf->matchIndex, pBuf->endIndex); } static void syncLogReplStates2Str(SSyncNode* pSyncNode, char* buf, int32_t bufLen) { - int len = 0; + int32_t len = 0; len += snprintf(buf + len, bufLen - len, "%s", "{"); for (int32_t i = 0; i < pSyncNode->replicaNum; i++) { SSyncLogReplMgr* pMgr = pSyncNode->logReplMgrs[i]; if (pMgr == NULL) break; - len += snprintf(buf + len, bufLen - len, "%d:%d [%" PRId64 " %" PRId64 ", %" PRId64 ")", i, pMgr->restored, + len += snprintf(buf + len, bufLen - len, "%d:%d [%" PRId64 " %" PRId64 ", %" PRId64 "]", i, pMgr->restored, pMgr->startIndex, pMgr->matchIndex, pMgr->endIndex); if (i + 1 < pSyncNode->replicaNum) { len += snprintf(buf + len, bufLen - len, "%s", ", "); @@ -280,7 +281,7 @@ void syncPrintSnapshotSenderLog(const char* flags, ELogLevel level, int32_t dfla " end:%" PRId64 " last-index:%" PRId64 " last-term:%" PRId64 " last-cfg:%" PRId64 ", seq:%d, ack:%d, " " buf:[%" PRId64 " %" PRId64 ", %" PRId64 - "), finish:%d, as:%d, to-dnode:%d}" + "], finish:%d, as:%d, to-dnode:%d}" ", term:%" PRIu64 ", commit-index:%" PRId64 ", firstver:%" PRId64 ", lastver:%" PRId64 ", min-match:%" PRId64 ", snap:{last-index:%" PRId64 ", term:%" PRIu64 "}, standby:%d, batch-sz:%d, replicas:%d, last-cfg:%" PRId64 From 50672cd1a875f81dcb83db05d2b50e6e9fd1ad86 Mon Sep 17 00:00:00 2001 From: sima Date: Mon, 19 Aug 2024 16:52:30 +0800 Subject: [PATCH 08/28] enh:[TD-31529] Remove ASSERT in libs/scalar. --- include/util/taoserror.h | 3 + source/libs/geometry/src/geomFunc.c | 6 +- source/libs/scalar/inc/sclvector.h | 37 ++-- source/libs/scalar/src/filter.c | 172 ++++++++++++++---- source/libs/scalar/src/sclfunc.c | 7 +- source/libs/scalar/src/sclvector.c | 168 ++++++++++------- source/libs/scalar/test/filter/CMakeLists.txt | 2 +- .../libs/scalar/test/filter/filterTests.cpp | 3 +- .../libs/scalar/test/scalar/scalarTests.cpp | 14 +- source/util/src/terror.c | 3 + 10 files changed, 281 insertions(+), 134 deletions(-) diff --git a/include/util/taoserror.h b/include/util/taoserror.h index 1911c48d26..cc31cde61a 100644 --- a/include/util/taoserror.h +++ b/include/util/taoserror.h @@ -604,6 +604,9 @@ int32_t taosGetErrSize(); #define TSDB_CODE_QRY_EXECUTOR_INTERNAL_ERROR TAOS_DEF_ERROR_CODE(0, 0x0732) #define TSDB_CODE_QRY_INVALID_JOIN_CONDITION TAOS_DEF_ERROR_CODE(0, 0x0733) #define TSDB_CODE_QRY_FILTER_NOT_SUPPORT_TYPE TAOS_DEF_ERROR_CODE(0, 0x0734) +#define TSDB_CODE_QRY_FILTER_WRONG_OPTR_TYPE TAOS_DEF_ERROR_CODE(0, 0x0735) +#define TSDB_CODE_QRY_FILTER_RANGE_ERROR TAOS_DEF_ERROR_CODE(0, 0x0736) +#define TSDB_CODE_QRY_FILTER_INVALID_TYPE TAOS_DEF_ERROR_CODE(0, 0x0737) // grant #define TSDB_CODE_GRANT_EXPIRED TAOS_DEF_ERROR_CODE(0, 0x0800) diff --git a/source/libs/geometry/src/geomFunc.c b/source/libs/geometry/src/geomFunc.c index 4426427bf5..1752493dff 100644 --- a/source/libs/geometry/src/geomFunc.c +++ b/source/libs/geometry/src/geomFunc.c @@ -131,12 +131,12 @@ _exit: int32_t executeMakePointFunc(SColumnInfoData *pInputData[], int32_t iLeft, int32_t iRight, SColumnInfoData *pOutputData) { int32_t code = TSDB_CODE_FAILED; + unsigned char *output = NULL; _getDoubleValue_fn_t getDoubleValueFn[2]; - getDoubleValueFn[0] = getVectorDoubleValueFn(pInputData[0]->info.type); - getDoubleValueFn[1] = getVectorDoubleValueFn(pInputData[1]->info.type); + TAOS_CHECK_GOTO(getVectorDoubleValueFn(pInputData[0]->info.type, &getDoubleValueFn[0]), NULL, _exit); + TAOS_CHECK_GOTO(getVectorDoubleValueFn(pInputData[1]->info.type, &getDoubleValueFn[1]), NULL, _exit); - unsigned char *output = NULL; double leftRes = 0; double rightRes = 0; diff --git a/source/libs/scalar/inc/sclvector.h b/source/libs/scalar/inc/sclvector.h index c2eb13dc75..fdd3e92501 100644 --- a/source/libs/scalar/inc/sclvector.h +++ b/source/libs/scalar/inc/sclvector.h @@ -78,40 +78,41 @@ static FORCE_INLINE int32_t getVectorDoubleValue_BOOL(void *src, int32_t index, int32_t getVectorDoubleValue_JSON(void *src, int32_t index, double *out); -static FORCE_INLINE _getDoubleValue_fn_t getVectorDoubleValueFn(int32_t srcType) { - _getDoubleValue_fn_t p = NULL; +static FORCE_INLINE int32_t getVectorDoubleValueFn(int32_t srcType, _getDoubleValue_fn_t *p) { + *p = NULL; if (srcType == TSDB_DATA_TYPE_TINYINT) { - p = getVectorDoubleValue_TINYINT; + *p = getVectorDoubleValue_TINYINT; } else if (srcType == TSDB_DATA_TYPE_UTINYINT) { - p = getVectorDoubleValue_UTINYINT; + *p = getVectorDoubleValue_UTINYINT; } else if (srcType == TSDB_DATA_TYPE_SMALLINT) { - p = getVectorDoubleValue_SMALLINT; + *p = getVectorDoubleValue_SMALLINT; } else if (srcType == TSDB_DATA_TYPE_USMALLINT) { - p = getVectorDoubleValue_USMALLINT; + *p = getVectorDoubleValue_USMALLINT; } else if (srcType == TSDB_DATA_TYPE_INT) { - p = getVectorDoubleValue_INT; + *p = getVectorDoubleValue_INT; } else if (srcType == TSDB_DATA_TYPE_UINT) { - p = getVectorDoubleValue_UINT; + *p = getVectorDoubleValue_UINT; } else if (srcType == TSDB_DATA_TYPE_BIGINT) { - p = getVectorDoubleValue_BIGINT; + *p = getVectorDoubleValue_BIGINT; } else if (srcType == TSDB_DATA_TYPE_UBIGINT) { - p = getVectorDoubleValue_UBIGINT; + *p = getVectorDoubleValue_UBIGINT; } else if (srcType == TSDB_DATA_TYPE_FLOAT) { - p = getVectorDoubleValue_FLOAT; + *p = getVectorDoubleValue_FLOAT; } else if (srcType == TSDB_DATA_TYPE_DOUBLE) { - p = getVectorDoubleValue_DOUBLE; + *p = getVectorDoubleValue_DOUBLE; } else if (srcType == TSDB_DATA_TYPE_TIMESTAMP) { - p = getVectorDoubleValue_BIGINT; + *p = getVectorDoubleValue_BIGINT; } else if (srcType == TSDB_DATA_TYPE_JSON) { - p = getVectorDoubleValue_JSON; + *p = getVectorDoubleValue_JSON; } else if (srcType == TSDB_DATA_TYPE_BOOL) { - p = getVectorDoubleValue_BOOL; + *p = getVectorDoubleValue_BOOL; } else if (srcType == TSDB_DATA_TYPE_NULL) { - p = NULL; + *p = NULL; } else { - ASSERT(0); + *p = NULL; + return TSDB_CODE_SCALAR_CONVERT_ERROR; } - return p; + return TSDB_CODE_SUCCESS; } typedef int32_t (*_bufConverteFunc)(char *buf, SScalarParam *pOut, int32_t outType, int32_t *overflow); diff --git a/source/libs/scalar/src/filter.c b/source/libs/scalar/src/filter.c index cc9cc9ed76..988aeba46f 100644 --- a/source/libs/scalar/src/filter.c +++ b/source/libs/scalar/src/filter.c @@ -92,7 +92,9 @@ rangeCompFunc gRangeCompare[] = {filterRangeCompee, filterRangeCompei, filterRan int8_t filterGetRangeCompFuncFromOptrs(uint8_t optr, uint8_t optr2) { if (optr2) { - ASSERT(optr2 == OP_TYPE_LOWER_THAN || optr2 == OP_TYPE_LOWER_EQUAL); + if (optr2 != OP_TYPE_LOWER_THAN && optr2 != OP_TYPE_LOWER_EQUAL) { + return -1; + } if (optr == OP_TYPE_GREATER_THAN) { if (optr2 == OP_TYPE_LOWER_THAN) { @@ -763,7 +765,10 @@ int32_t filterAddRangeCtx(void *dst, void *src, int32_t optr) { SFilterRangeCtx *dctx = (SFilterRangeCtx *)dst; SFilterRangeCtx *sctx = (SFilterRangeCtx *)src; - ASSERT(optr == LOGIC_COND_TYPE_OR); + if (optr != LOGIC_COND_TYPE_OR) { + fltError("filterAddRangeCtx get invalid optr:%d", optr); + return TSDB_CODE_QRY_FILTER_WRONG_OPTR_TYPE; + } if (sctx->rs == NULL) { return TSDB_CODE_SUCCESS; @@ -1204,7 +1209,10 @@ int32_t filterAddUnitImpl(SFilterInfo *info, uint8_t optr, SFilterFieldId *left, if (u->right.type == FLD_TYPE_VALUE) { SFilterField *val = FILTER_UNIT_RIGHT_FIELD(info, u); - ASSERT(FILTER_GET_FLAG(val->flag, FLD_TYPE_VALUE)); + if (!FILTER_GET_FLAG(val->flag, FLD_TYPE_VALUE)) { + fltError("filterAddUnitImpl get invalid flag : %d in val", val->flag); + return TSDB_CODE_APP_ERROR; + } } else { int32_t paramNum = scalarGetOperatorParamNum(optr); if (1 != paramNum) { @@ -1214,7 +1222,10 @@ int32_t filterAddUnitImpl(SFilterInfo *info, uint8_t optr, SFilterFieldId *left, } SFilterField *col = FILTER_UNIT_LEFT_FIELD(info, u); - ASSERT(FILTER_GET_FLAG(col->flag, FLD_TYPE_COLUMN)); + if (!FILTER_GET_FLAG(col->flag, FLD_TYPE_COLUMN)) { + fltError("filterAddUnitImpl get invalid flag : %d in col", col->flag); + return TSDB_CODE_APP_ERROR; + } info->units[info->unitNum].compare.type = FILTER_GET_COL_FIELD_TYPE(col); info->units[info->unitNum].compare.precision = FILTER_GET_COL_FIELD_PRECISION(col); @@ -1398,29 +1409,48 @@ int32_t filterAddGroupUnitFromCtx(SFilterInfo *dst, SFilterInfo *src, SFilterRan if (optr == LOGIC_COND_TYPE_AND) { if (ctx->isnull) { - ASSERT(ctx->notnull == false && ctx->isrange == false); + if (ctx->notnull || ctx->isrange) { + fltError("filterAddGroupUnitFromCtx get invalid ctx : isnull %d, notnull %d, isrange %d", + ctx->isnull, ctx->notnull, ctx->isrange); + FLT_ERR_RET(TSDB_CODE_QRY_FILTER_RANGE_ERROR); + } FLT_ERR_RET(filterAddUnit(dst, OP_TYPE_IS_NULL, &left, NULL, &uidx)); FLT_ERR_RET(filterAddUnitToGroup(g, uidx)); return TSDB_CODE_SUCCESS; } if (ctx->notnull) { - ASSERT(ctx->isnull == false && ctx->isrange == false); + if (ctx->isnull || ctx->isrange) { + fltError("filterAddGroupUnitFromCtx get invalid ctx : isnull %d, notnull %d, isrange %d", + ctx->isnull, ctx->notnull, ctx->isrange); + FLT_ERR_RET(TSDB_CODE_QRY_FILTER_RANGE_ERROR); + } FLT_ERR_RET(filterAddUnit(dst, OP_TYPE_IS_NOT_NULL, &left, NULL, &uidx)); FLT_ERR_RET(filterAddUnitToGroup(g, uidx)); return TSDB_CODE_SUCCESS; } if (!ctx->isrange) { - ASSERT(ctx->isnull || ctx->notnull); + if (!ctx->isnull && !ctx->notnull) { + fltError("filterAddGroupUnitFromCtx get invalid ctx : isnull %d, notnull %d, isrange %d", + ctx->isnull, ctx->notnull, ctx->isrange); + FLT_ERR_RET(TSDB_CODE_QRY_FILTER_RANGE_ERROR); + } return TSDB_CODE_SUCCESS; } - ASSERT(ctx->rs && ctx->rs->next == NULL); + if (!ctx->rs || ctx->rs->next != NULL) { + fltError("filterAddGroupUnitFromCtx get invalid range node with rs:%p", ctx->rs); + FLT_ERR_RET(TSDB_CODE_QRY_FILTER_RANGE_ERROR); + } SFilterRange *ra = &ctx->rs->ra; - ASSERT(!((FILTER_GET_FLAG(ra->sflag, RANGE_FLG_NULL)) && (FILTER_GET_FLAG(ra->eflag, RANGE_FLG_NULL)))); + if (((FILTER_GET_FLAG(ra->sflag, RANGE_FLG_NULL)) && (FILTER_GET_FLAG(ra->eflag, RANGE_FLG_NULL)))) { + fltError("filterAddGroupUnitFromCtx get invalid range with sflag:%d, eflag:%d", + FILTER_GET_FLAG(ra->sflag, RANGE_FLG_NULL), FILTER_GET_FLAG(ra->eflag, RANGE_FLG_NULL)); + FLT_ERR_RET(TSDB_CODE_QRY_FILTER_RANGE_ERROR); + } if ((!FILTER_GET_FLAG(ra->sflag, RANGE_FLG_NULL)) && (!FILTER_GET_FLAG(ra->eflag, RANGE_FLG_NULL))) { __compar_fn_t func = getComparFunc(type, 0); @@ -1489,7 +1519,11 @@ int32_t filterAddGroupUnitFromCtx(SFilterInfo *dst, SFilterInfo *src, SFilterRan SFilterGroup ng = {0}; g = &ng; - ASSERT(ctx->isnull || ctx->notnull || ctx->isrange); + if (!ctx->isnull && !ctx->notnull && !ctx->isrange) { + fltError("filterAddGroupUnitFromCtx get invalid ctx : isnull %d, notnull %d, isrange %d", + ctx->isnull, ctx->notnull, ctx->isrange); + FLT_ERR_RET(TSDB_CODE_APP_ERROR); + } if (ctx->isnull) { FLT_ERR_RET(filterAddUnit(dst, OP_TYPE_IS_NULL, &left, NULL, &uidx)); @@ -1500,7 +1534,11 @@ int32_t filterAddGroupUnitFromCtx(SFilterInfo *dst, SFilterInfo *src, SFilterRan } if (ctx->notnull) { - ASSERT(!ctx->isrange); + if (ctx->isrange) { + fltError("filterAddGroupUnitFromCtx get invalid ctx : isnull %d, notnull %d, isrange %d", + ctx->isnull, ctx->notnull, ctx->isrange); + FLT_ERR_RET(TSDB_CODE_QRY_FILTER_RANGE_ERROR); + } (void)memset(g, 0, sizeof(*g)); FLT_ERR_RET(filterAddUnit(dst, OP_TYPE_IS_NOT_NULL, &left, NULL, &uidx)); @@ -1511,7 +1549,11 @@ int32_t filterAddGroupUnitFromCtx(SFilterInfo *dst, SFilterInfo *src, SFilterRan } if (!ctx->isrange) { - ASSERT(ctx->isnull || ctx->notnull); + if (!ctx->isnull && !ctx->notnull) { + fltError("filterAddGroupUnitFromCtx get invalid ctx : isnull %d, notnull %d, isrange %d", + ctx->isnull, ctx->notnull, ctx->isrange); + FLT_ERR_RET(TSDB_CODE_QRY_FILTER_RANGE_ERROR); + } g->unitNum = 0; return TSDB_CODE_SUCCESS; } @@ -1586,7 +1628,10 @@ int32_t filterAddGroupUnitFromCtx(SFilterInfo *dst, SFilterInfo *src, SFilterRan FLT_ERR_RET(filterAddUnitToGroup(g, uidx)); } - ASSERT(g->unitNum > 0); + if (g->unitNum <= 0) { + fltError("filterAddGroupUnitFromCtx get invalid filter group unit num %d", g->unitNum); + FLT_ERR_RET(TSDB_CODE_QRY_FILTER_RANGE_ERROR); + } if (NULL == taosArrayPush(res,g)) { FLT_ERR_RET(TSDB_CODE_OUT_OF_MEMORY); @@ -2066,7 +2111,10 @@ void filterFreeInfo(SFilterInfo *info) { } int32_t filterHandleValueExtInfo(SFilterUnit *unit, char extInfo) { - ASSERT(extInfo > 0 || extInfo < 0); + if (extInfo == 0) { + fltError("filterHandleValueExtInfo get invalid extInfo : %d", extInfo); + return TSDB_CODE_APP_ERROR; + } uint8_t optr = FILTER_UNIT_OPTR(unit); switch (optr) { @@ -2093,13 +2141,20 @@ int32_t fltInitValFieldData(SFilterInfo *info) { for (uint32_t i = 0; i < info->unitNum; ++i) { SFilterUnit *unit = &info->units[i]; if (unit->right.type != FLD_TYPE_VALUE) { - ASSERT(unit->compare.optr == FILTER_DUMMY_EMPTY_OPTR || scalarGetOperatorParamNum(unit->compare.optr) == 1); + if (unit->compare.optr != FILTER_DUMMY_EMPTY_OPTR && scalarGetOperatorParamNum(unit->compare.optr) != 1) { + fltError("filterInitValFieldData get invalid operator param num : %d and invalid compare optr %d", + scalarGetOperatorParamNum(unit->compare.optr), unit->compare.optr); + return TSDB_CODE_APP_ERROR; + } continue; } SFilterField *right = FILTER_UNIT_RIGHT_FIELD(info, unit); - ASSERT(FILTER_GET_FLAG(right->flag, FLD_TYPE_VALUE)); + if (!FILTER_GET_FLAG(right->flag, FLD_TYPE_VALUE)) { + fltError("filterInitValFieldData get invalid field flag : %d", right->flag); + return TSDB_CODE_APP_ERROR; + } uint32_t type = FILTER_UNIT_DATA_TYPE(unit); int8_t precision = FILTER_UNIT_DATA_PRECISION(unit); @@ -2107,7 +2162,10 @@ int32_t fltInitValFieldData(SFilterInfo *info) { SValueNode *var = (SValueNode *)fi->desc; if (var == NULL) { - ASSERT(fi->data != NULL); + if (!fi->data) { + fltError("filterInitValFieldData get invalid field data : NULL"); + return TSDB_CODE_APP_ERROR; + } continue; } @@ -2257,7 +2315,10 @@ int32_t filterAddUnitRange(SFilterInfo *info, SFilterUnit *u, SFilterRangeCtx *c FILTER_SET_FLAG(ra.sflag, RANGE_FLG_NULL); break; case OP_TYPE_NOT_EQUAL: - ASSERT(type == TSDB_DATA_TYPE_BOOL); + if (type != TSDB_DATA_TYPE_BOOL) { + fltError("filterAddUnitRange get invalid type : %d", type); + return TSDB_CODE_QRY_FILTER_INVALID_TYPE; + } if (GET_INT8_VAL(val)) { SIMPLE_COPY_VALUES(&ra.s, &tmp); SIMPLE_COPY_VALUES(&ra.e, &tmp); @@ -2273,7 +2334,7 @@ int32_t filterAddUnitRange(SFilterInfo *info, SFilterUnit *u, SFilterRangeCtx *c break; default: fltError("unsupported operator type"); - return TSDB_CODE_APP_ERROR; + return TSDB_CODE_QRY_FILTER_NOT_SUPPORT_TYPE; } FLT_ERR_RET(filterAddRange(ctx, &ra, optr)); @@ -2547,8 +2608,11 @@ int32_t filterMergeTwoGroupsImpl(SFilterInfo *info, SFilterRangeCtx **ctx, int32 FLT_ERR_RET(filterReuseRangeCtx(*ctx, type, 0)); } - ASSERT(gRes2->colInfo[cidx].type == RANGE_TYPE_MR_CTX); - ASSERT(gRes1->colInfo[cidx].type == RANGE_TYPE_MR_CTX); + if (gRes2->colInfo[cidx].type != RANGE_TYPE_MR_CTX || gRes1->colInfo[cidx].type != RANGE_TYPE_MR_CTX) { + fltError("filterMergeTwoGroupsImpl get invalid col type : %d and %d", + gRes2->colInfo[cidx].type, gRes1->colInfo[cidx].type); + return TSDB_CODE_QRY_FILTER_NOT_SUPPORT_TYPE; + } FLT_ERR_RET(filterCopyRangeCtx(*ctx, gRes2->colInfo[cidx].info)); FLT_ERR_RET(filterSourceRangeFromCtx(*ctx, gRes1->colInfo[cidx].info, optr, empty, all)); @@ -2588,7 +2652,10 @@ int32_t filterMergeTwoGroups(SFilterInfo *info, SFilterGroupCtx **gRes1, SFilter continue; } - ASSERT(idx1 == idx2); + if (idx1 != idx2) { + fltError("filterMergeTwoGroups get invalid idx : %d and %d", idx1, idx2); + FLT_ERR_JRET(TSDB_CODE_APP_ERROR); + } ++merNum; @@ -2644,16 +2711,19 @@ int32_t filterMergeTwoGroups(SFilterInfo *info, SFilterGroupCtx **gRes1, SFilter } } - ASSERT(merNum > 0); - - SFilterColInfo *colInfo = NULL; - ASSERT(merNum == equal1 || merNum == equal2); + if (merNum == 0 || (equal1 != merNum && equal2 != merNum)) { + fltError("filterMergeTwoGroups get invalid merge num : %d, equal1 : %d, equal2 : %d", merNum, equal1, equal2); + FLT_ERR_JRET(TSDB_CODE_APP_ERROR); + } filterFreeGroupCtx(*gRes2); *gRes2 = NULL; - ASSERT(colCtxs && taosArrayGetSize(colCtxs) > 0); - + if (!colCtxs || taosArrayGetSize(colCtxs) <= 0) { + fltError("filterMergeTwoGroups get invalid colCtxs with size %zu", taosArrayGetSize(colCtxs)); + FLT_ERR_JRET(TSDB_CODE_APP_ERROR); + } + SFilterColInfo *colInfo = NULL; int32_t ctxSize = (int32_t)taosArrayGetSize(colCtxs); SFilterColCtx *pctx = NULL; @@ -2713,7 +2783,10 @@ int32_t filterMergeGroups(SFilterInfo *info, SFilterGroupCtx **gRes, int32_t *gR if (pColNum > 0) { for (int32_t m = 0; m <= pEnd; ++m) { for (int32_t n = cStart; n <= cEnd; ++n) { - ASSERT(m < n); + if (m >= n) { + fltError("filterMergeGroups get invalid m : %d and n : %d", m, n); + FLT_ERR_JRET(TSDB_CODE_APP_ERROR); + } FLT_ERR_JRET(filterMergeTwoGroups(info, &gRes[m], &gRes[n], &all)); FLT_CHK_JMP(all); @@ -2734,7 +2807,10 @@ int32_t filterMergeGroups(SFilterInfo *info, SFilterGroupCtx **gRes, int32_t *gR for (int32_t m = cStart; m < cEnd; ++m) { for (int32_t n = m + 1; n <= cEnd; ++n) { - ASSERT(m < n); + if (m >= n) { + fltError("filterMergeGroups get invalid m : %d and n : %d", m, n); + FLT_ERR_JRET(TSDB_CODE_APP_ERROR); + } FLT_ERR_JRET(filterMergeTwoGroups(info, &gRes[m], &gRes[n], &all)); FLT_CHK_JMP(all); @@ -2844,7 +2920,10 @@ int32_t filterRewrite(SFilterInfo *info, SFilterGroupCtx **gRes, int32_t gResNum for (uint32_t m = 0; m < res->colNum; ++m) { colInfo = &res->colInfo[res->colIdx[m]]; if (FILTER_NO_MERGE_DATA_TYPE(colInfo->dataType)) { - ASSERT(colInfo->type == RANGE_TYPE_UNIT); + if (colInfo->type != RANGE_TYPE_UNIT) { + fltError("filterRewrite get invalid col type : %d", colInfo->type); + FLT_ERR_JRET(TSDB_CODE_QRY_FILTER_INVALID_TYPE); + } int32_t usize = (int32_t)taosArrayGetSize((SArray *)colInfo->info); for (int32_t n = 0; n < usize; ++n) { @@ -2859,7 +2938,10 @@ int32_t filterRewrite(SFilterInfo *info, SFilterGroupCtx **gRes, int32_t gResNum continue; } - ASSERT(colInfo->type == RANGE_TYPE_MR_CTX); + if (colInfo->type != RANGE_TYPE_MR_CTX) { + fltError("filterRewrite get invalid col type : %d", colInfo->type); + FLT_ERR_JRET(TSDB_CODE_QRY_FILTER_INVALID_TYPE); + } FLT_ERR_JRET(filterAddGroupUnitFromCtx(info, &oinfo, colInfo->info, res->colIdx[m], &ng, optr, group)); } @@ -2905,7 +2987,10 @@ int32_t filterGenerateColRange(SFilterInfo *info, SFilterGroupCtx **gRes, int32_ continue; } - ASSERT(idxNum[i] == gResNum); + if (idxNum[i] != gResNum) { + fltError("filterGenerateColRange get invalid idxNum : %d and gResNum : %d", idxNum[i], gResNum); + FLT_ERR_JRET(TSDB_CODE_APP_ERROR); + } if (idxs == NULL) { idxs = taosMemoryCalloc(info->fields[FLD_TYPE_COLUMN].num, sizeof(*idxs)); @@ -2936,7 +3021,10 @@ int32_t filterGenerateColRange(SFilterInfo *info, SFilterGroupCtx **gRes, int32_ continue; } - ASSERT(res->colIdx[n] == idxs[m]); + if (res->colIdx[n] != idxs[m]) { + fltError("filterGenerateColRange get invalid colIdx : %d and idxs : %d", res->colIdx[n], idxs[m]); + SCL_ERR_JRET(TSDB_CODE_APP_ERROR); + } SFilterColInfo *colInfo = &res->colInfo[res->colIdx[n]]; if (info->colRange[m] == NULL) { @@ -2945,7 +3033,10 @@ int32_t filterGenerateColRange(SFilterInfo *info, SFilterGroupCtx **gRes, int32_ info->colRange[m]->colId = FILTER_GET_COL_FIELD_ID(fi); } - ASSERT(colInfo->type == RANGE_TYPE_MR_CTX); + if (colInfo->type != RANGE_TYPE_MR_CTX) { + fltError("filterGenerateColRange get invalid col type : %d", colInfo->type); + FLT_ERR_JRET(TSDB_CODE_QRY_FILTER_INVALID_TYPE); + } bool all = false; FLT_ERR_JRET(filterSourceRangeFromCtx(info->colRange[m], colInfo->info, LOGIC_COND_TYPE_OR, NULL, &all)); @@ -3195,7 +3286,10 @@ int32_t filterRmUnitByRange(SFilterInfo *info, SColumnDataAgg *pDataStatis, int3 unitIdx = pGroupIdx; --info->blkGroupNum; - ASSERT(empty || all); + if (!empty && !all) { + fltError("filterRmUnitByRange get invalid empty and all : %d and %d", empty, all); + FLT_ERR_RET(TSDB_CODE_APP_ERROR); + } if (empty) { FILTER_SET_FLAG(info->blkFlag, FI_STATUS_BLK_EMPTY); @@ -3303,7 +3397,10 @@ int32_t filterExecuteBasedOnStatis(SFilterInfo *info, int32_t numOfRows, SColumn goto _return; } - ASSERT(info->unitNum > 1); + if (info->unitNum <= 1) { + fltError("filterExecuteBasedOnStatis get invalid unit num : %d", info->unitNum); + FLT_ERR_JRET(TSDB_CODE_APP_ERROR); + } *all = filterExecuteBasedOnStatisImpl(info, numOfRows, p, statis, numOfCols); goto _return; @@ -5161,7 +5258,6 @@ int32_t filterExecute(SFilterInfo *info, SSDataBlock *pSrc, SColumnInfoData **p, return TSDB_CODE_SUCCESS; } - ASSERT(false == info->scalarMode); *p = output.columnData; output.numOfRows = pSrc->info.rows; diff --git a/source/libs/scalar/src/sclfunc.c b/source/libs/scalar/src/sclfunc.c index f81205df7a..37daff1d63 100644 --- a/source/libs/scalar/src/sclfunc.c +++ b/source/libs/scalar/src/sclfunc.c @@ -297,7 +297,8 @@ static int32_t doScalarFunctionUnique(SScalarParam *pInput, int32_t inputNum, SS SColumnInfoData *pInputData = pInput->columnData; SColumnInfoData *pOutputData = pOutput->columnData; - _getDoubleValue_fn_t getValueFn = getVectorDoubleValueFn(type); + _getDoubleValue_fn_t getValueFn; + SCL_ERR_RET(getVectorDoubleValueFn(type, &getValueFn)); double *out = (double *)pOutputData->pData; @@ -328,7 +329,7 @@ static int32_t doScalarFunctionUnique2(SScalarParam *pInput, int32_t inputNum, S for (int32_t i = 0; i < inputNum; ++i) { pInputData[i] = pInput[i].columnData; - getValueFn[i] = getVectorDoubleValueFn(GET_PARAM_TYPE(&pInput[i])); + SCL_ERR_RET(getVectorDoubleValueFn(GET_PARAM_TYPE(&pInput[i]), &getValueFn[i])); } double *out = (double *)pOutputData->pData; @@ -2918,7 +2919,7 @@ static int32_t doScalarFunction2(SScalarParam *pInput, int32_t inputNum, SScalar for (int32_t i = 0; i < inputNum; ++i) { pInputData[i] = pInput[i].columnData; - getValueFn[i] = getVectorDoubleValueFn(GET_PARAM_TYPE(&pInput[i])); + SCL_ERR_RET(getVectorDoubleValueFn(GET_PARAM_TYPE(&pInput[i]), &getValueFn[i])); } bool hasNullType = (IS_NULL_TYPE(GET_PARAM_TYPE(&pInput[0])) || IS_NULL_TYPE(GET_PARAM_TYPE(&pInput[1]))); diff --git a/source/libs/scalar/src/sclvector.c b/source/libs/scalar/src/sclvector.c index 71773ced57..5556108a52 100644 --- a/source/libs/scalar/src/sclvector.c +++ b/source/libs/scalar/src/sclvector.c @@ -43,7 +43,7 @@ bool noConvertBeforeCompare(int32_t leftType, int32_t rightType, int32_t optr) { (optr >= OP_TYPE_GREATER_THAN && optr <= OP_TYPE_NOT_EQUAL); } -void convertNumberToNumber(const void *inData, void *outData, int8_t inType, int8_t outType) { +int32_t convertNumberToNumber(const void *inData, void *outData, int8_t inType, int8_t outType) { switch (outType) { case TSDB_DATA_TYPE_BOOL: { GET_TYPED_DATA(*((bool *)outData), bool, inType, inData); @@ -91,9 +91,10 @@ void convertNumberToNumber(const void *inData, void *outData, int8_t inType, int break; } default: { - ASSERT(0); + return TSDB_CODE_SCALAR_CONVERT_ERROR; } } + return TSDB_CODE_SUCCESS; } int32_t convertNcharToDouble(const void *inData, void *outData) { @@ -180,7 +181,10 @@ int32_t getVectorBigintValue_BOOL(void *src, int32_t index, int64_t *res) { } int32_t getVectorBigintValue_JSON(void *src, int32_t index, int64_t *res) { - ASSERT(!colDataIsNull_var(((SColumnInfoData *)src), index)); + if (colDataIsNull_var(((SColumnInfoData *)src), index)) { + sclError("getVectorBigintValue_JSON get json data null with index %d", index); + SCL_ERR_RET(TSDB_CODE_SCALAR_CONVERT_ERROR); + } char *data = colDataGetVarData((SColumnInfoData *)src, index); double out = 0; if (*data == TSDB_DATA_TYPE_NULL) { @@ -192,46 +196,47 @@ int32_t getVectorBigintValue_JSON(void *src, int32_t index, int64_t *res) { *res = 0; SCL_ERR_RET(TSDB_CODE_QRY_JSON_NOT_SUPPORT_ERROR); } else { - convertNumberToNumber(data + CHAR_BYTES, &out, *data, TSDB_DATA_TYPE_DOUBLE); + SCL_ERR_RET(convertNumberToNumber(data + CHAR_BYTES, &out, *data, TSDB_DATA_TYPE_DOUBLE)); } *res = (int64_t)out; SCL_RET(TSDB_CODE_SUCCESS); } -_getBigintValue_fn_t getVectorBigintValueFn(int32_t srcType) { - _getBigintValue_fn_t p = NULL; +int32_t getVectorBigintValueFn(int32_t srcType, _getBigintValue_fn_t *p) { + *p = NULL; if (srcType == TSDB_DATA_TYPE_TINYINT) { - p = getVectorBigintValue_TINYINT; + *p = getVectorBigintValue_TINYINT; } else if (srcType == TSDB_DATA_TYPE_UTINYINT) { - p = getVectorBigintValue_UTINYINT; + *p = getVectorBigintValue_UTINYINT; } else if (srcType == TSDB_DATA_TYPE_SMALLINT) { - p = getVectorBigintValue_SMALLINT; + *p = getVectorBigintValue_SMALLINT; } else if (srcType == TSDB_DATA_TYPE_USMALLINT) { - p = getVectorBigintValue_USMALLINT; + *p = getVectorBigintValue_USMALLINT; } else if (srcType == TSDB_DATA_TYPE_INT) { - p = getVectorBigintValue_INT; + *p = getVectorBigintValue_INT; } else if (srcType == TSDB_DATA_TYPE_UINT) { - p = getVectorBigintValue_UINT; + *p = getVectorBigintValue_UINT; } else if (srcType == TSDB_DATA_TYPE_BIGINT) { - p = getVectorBigintValue_BIGINT; + *p = getVectorBigintValue_BIGINT; } else if (srcType == TSDB_DATA_TYPE_UBIGINT) { - p = getVectorBigintValue_UBIGINT; + *p = getVectorBigintValue_UBIGINT; } else if (srcType == TSDB_DATA_TYPE_FLOAT) { - p = getVectorBigintValue_FLOAT; + *p = getVectorBigintValue_FLOAT; } else if (srcType == TSDB_DATA_TYPE_DOUBLE) { - p = getVectorBigintValue_DOUBLE; + *p = getVectorBigintValue_DOUBLE; } else if (srcType == TSDB_DATA_TYPE_TIMESTAMP) { - p = getVectorBigintValue_BIGINT; + *p = getVectorBigintValue_BIGINT; } else if (srcType == TSDB_DATA_TYPE_BOOL) { - p = getVectorBigintValue_BOOL; + *p = getVectorBigintValue_BOOL; } else if (srcType == TSDB_DATA_TYPE_JSON) { - p = getVectorBigintValue_JSON; + *p = getVectorBigintValue_JSON; } else if (srcType == TSDB_DATA_TYPE_NULL) { - p = NULL; + *p = NULL; } else { - ASSERT(0); + sclError("getVectorBigintValueFn invalid srcType : %d", srcType); + return TSDB_CODE_SCALAR_CONVERT_ERROR; } - return p; + return TSDB_CODE_SUCCESS; } static FORCE_INLINE int32_t varToTimestamp(char *buf, SScalarParam *pOut, int32_t rowIndex, int32_t *overflow) { @@ -467,7 +472,7 @@ static FORCE_INLINE int32_t varToGeometry(char *buf, SScalarParam *pOut, int32_t _return: taosMemoryFree(output); geosFreeBuffer(t); - ASSERT(t == NULL && len == 0); + t = NULL; VarDataLenT dummyHeader = 0; SCL_ERR_RET(colDataSetVal(pOut->columnData, rowIndex, (const char *)&dummyHeader, false)); SCL_RET(code); @@ -525,7 +530,7 @@ int32_t vectorConvertFromVarData(SSclVectorConvCtx *pCtx, int32_t *overflow) { } else if (tTagIsJson(data) || *data == TSDB_DATA_TYPE_NULL) { SCL_ERR_JRET(TSDB_CODE_QRY_JSON_NOT_SUPPORT_ERROR); } else { - convertNumberToNumber(data + CHAR_BYTES, colDataGetNumData(pCtx->pOut->columnData, i), *data, pCtx->outType); + SCL_ERR_JRET(convertNumberToNumber(data + CHAR_BYTES, colDataGetNumData(pCtx->pOut->columnData, i), *data, pCtx->outType)); continue; } } @@ -582,7 +587,7 @@ int32_t getVectorDoubleValue_JSON(void *src, int32_t index, double *out) { } else if (tTagIsJson(data)) { SCL_ERR_RET(TSDB_CODE_QRY_JSON_NOT_SUPPORT_ERROR); } else { - convertNumberToNumber(data + CHAR_BYTES, out, *data, TSDB_DATA_TYPE_DOUBLE); + SCL_ERR_RET(convertNumberToNumber(data + CHAR_BYTES, out, *data, TSDB_DATA_TYPE_DOUBLE)); } SCL_RET(TSDB_CODE_SUCCESS); } @@ -673,7 +678,7 @@ int32_t convertJsonValue(__compar_fn_t *fp, int32_t optr, int8_t typeLeft, int8_ *result = false; return TSDB_CODE_SUCCESS; } else if (typeLeft != type) { - convertNumberToNumber(*pLeftData, pLeftOut, typeLeft, type); + SCL_ERR_RET(convertNumberToNumber(*pLeftData, pLeftOut, typeLeft, type)); *pLeftData = pLeftOut; } @@ -683,7 +688,7 @@ int32_t convertJsonValue(__compar_fn_t *fp, int32_t optr, int8_t typeLeft, int8_ *result = false; return TSDB_CODE_SUCCESS; } else if (typeRight != type) { - convertNumberToNumber(*pRightData, pRightOut, typeRight, type); + SCL_ERR_RET(convertNumberToNumber(*pRightData, pRightOut, typeRight, type)); *pRightData = pRightOut; } } else if (type == TSDB_DATA_TYPE_BINARY || @@ -1130,8 +1135,10 @@ enum { // TODO not correct for descending order scan static int32_t vectorMathAddHelper(SColumnInfoData *pLeftCol, SColumnInfoData *pRightCol, SColumnInfoData *pOutputCol, int32_t numOfRows, int32_t step, int32_t i) { - _getDoubleValue_fn_t getVectorDoubleValueFnLeft = getVectorDoubleValueFn(pLeftCol->info.type); - _getDoubleValue_fn_t getVectorDoubleValueFnRight = getVectorDoubleValueFn(pRightCol->info.type); + _getDoubleValue_fn_t getVectorDoubleValueFnLeft; + _getDoubleValue_fn_t getVectorDoubleValueFnRight; + SCL_ERR_RET(getVectorDoubleValueFn(pLeftCol->info.type, &getVectorDoubleValueFnLeft)); + SCL_ERR_RET(getVectorDoubleValueFn(pRightCol->info.type, &getVectorDoubleValueFnRight)); double *output = (double *)pOutputCol->pData; @@ -1155,9 +1162,10 @@ static int32_t vectorMathAddHelper(SColumnInfoData *pLeftCol, SColumnInfoData *p static int32_t vectorMathTsAddHelper(SColumnInfoData *pLeftCol, SColumnInfoData *pRightCol, SColumnInfoData *pOutputCol, int32_t numOfRows, int32_t step, int32_t i) { - _getBigintValue_fn_t getVectorBigintValueFnLeft = getVectorBigintValueFn(pLeftCol->info.type); - _getBigintValue_fn_t getVectorBigintValueFnRight = getVectorBigintValueFn(pRightCol->info.type); - + _getBigintValue_fn_t getVectorBigintValueFnLeft; + _getBigintValue_fn_t getVectorBigintValueFnRight; + SCL_ERR_RET(getVectorBigintValueFn(pLeftCol->info.type, &getVectorBigintValueFnLeft)); + SCL_ERR_RET(getVectorBigintValueFn(pRightCol->info.type, &getVectorBigintValueFnRight)); int64_t *output = (int64_t *)pOutputCol->pData; if (IS_HELPER_NULL(pRightCol, 0)) { // Set pLeft->numOfRows NULL value @@ -1230,8 +1238,10 @@ int32_t vectorMathAdd(SScalarParam *pLeft, SScalarParam *pRight, SScalarParam *p (GET_PARAM_TYPE(pRight) == TSDB_DATA_TYPE_TIMESTAMP && GET_PARAM_TYPE(pLeft) == TSDB_DATA_TYPE_BOOL)) { // timestamp plus duration int64_t *output = (int64_t *)pOutputCol->pData; - _getBigintValue_fn_t getVectorBigintValueFnLeft = getVectorBigintValueFn(pLeftCol->info.type); - _getBigintValue_fn_t getVectorBigintValueFnRight = getVectorBigintValueFn(pRightCol->info.type); + _getBigintValue_fn_t getVectorBigintValueFnLeft; + _getBigintValue_fn_t getVectorBigintValueFnRight; + SCL_ERR_JRET(getVectorBigintValueFn(pLeftCol->info.type, &getVectorBigintValueFnLeft)); + SCL_ERR_JRET(getVectorBigintValueFn(pRightCol->info.type, &getVectorBigintValueFnRight)); if (pLeft->numOfRows == 1 && pRight->numOfRows == 1) { if (GET_PARAM_TYPE(pLeft) == TSDB_DATA_TYPE_TIMESTAMP) { @@ -1258,9 +1268,10 @@ int32_t vectorMathAdd(SScalarParam *pLeft, SScalarParam *pRight, SScalarParam *p } } else { double *output = (double *)pOutputCol->pData; - _getDoubleValue_fn_t getVectorDoubleValueFnLeft = getVectorDoubleValueFn(pLeftCol->info.type); - _getDoubleValue_fn_t getVectorDoubleValueFnRight = getVectorDoubleValueFn(pRightCol->info.type); - + _getDoubleValue_fn_t getVectorDoubleValueFnLeft; + _getDoubleValue_fn_t getVectorDoubleValueFnRight; + SCL_ERR_JRET(getVectorDoubleValueFn(pLeftCol->info.type, &getVectorDoubleValueFnLeft)); + SCL_ERR_JRET(getVectorDoubleValueFn(pRightCol->info.type, &getVectorDoubleValueFnRight)); if (pLeft->numOfRows == pRight->numOfRows) { for (; i < pRight->numOfRows && i >= 0; i += step, output += 1) { if (IS_NULL) { @@ -1289,8 +1300,10 @@ _return: // TODO not correct for descending order scan static int32_t vectorMathSubHelper(SColumnInfoData *pLeftCol, SColumnInfoData *pRightCol, SColumnInfoData *pOutputCol, int32_t numOfRows, int32_t step, int32_t factor, int32_t i) { - _getDoubleValue_fn_t getVectorDoubleValueFnLeft = getVectorDoubleValueFn(pLeftCol->info.type); - _getDoubleValue_fn_t getVectorDoubleValueFnRight = getVectorDoubleValueFn(pRightCol->info.type); + _getDoubleValue_fn_t getVectorDoubleValueFnLeft; + _getDoubleValue_fn_t getVectorDoubleValueFnRight; + SCL_ERR_RET(getVectorDoubleValueFn(pLeftCol->info.type, &getVectorDoubleValueFnLeft)); + SCL_ERR_RET(getVectorDoubleValueFn(pRightCol->info.type, &getVectorDoubleValueFnRight)); double *output = (double *)pOutputCol->pData; @@ -1314,8 +1327,10 @@ static int32_t vectorMathSubHelper(SColumnInfoData *pLeftCol, SColumnInfoData *p static int32_t vectorMathTsSubHelper(SColumnInfoData *pLeftCol, SColumnInfoData *pRightCol, SColumnInfoData *pOutputCol, int32_t numOfRows, int32_t step, int32_t factor, int32_t i) { - _getBigintValue_fn_t getVectorBigintValueFnLeft = getVectorBigintValueFn(pLeftCol->info.type); - _getBigintValue_fn_t getVectorBigintValueFnRight = getVectorBigintValueFn(pRightCol->info.type); + _getBigintValue_fn_t getVectorBigintValueFnLeft; + _getBigintValue_fn_t getVectorBigintValueFnRight; + SCL_ERR_RET(getVectorBigintValueFn(pLeftCol->info.type, &getVectorBigintValueFnLeft)); + SCL_ERR_RET(getVectorBigintValueFn(pRightCol->info.type, &getVectorBigintValueFnRight)); int64_t *output = (int64_t *)pOutputCol->pData; @@ -1357,8 +1372,10 @@ int32_t vectorMathSub(SScalarParam *pLeft, SScalarParam *pRight, SScalarParam *p (GET_PARAM_TYPE(pRight) == TSDB_DATA_TYPE_TIMESTAMP && GET_PARAM_TYPE(pLeft) == TSDB_DATA_TYPE_BIGINT)) { // timestamp minus duration int64_t *output = (int64_t *)pOutputCol->pData; - _getBigintValue_fn_t getVectorBigintValueFnLeft = getVectorBigintValueFn(pLeftCol->info.type); - _getBigintValue_fn_t getVectorBigintValueFnRight = getVectorBigintValueFn(pRightCol->info.type); + _getBigintValue_fn_t getVectorBigintValueFnLeft; + _getBigintValue_fn_t getVectorBigintValueFnRight; + SCL_ERR_JRET(getVectorBigintValueFn(pLeftCol->info.type, &getVectorBigintValueFnLeft)); + SCL_ERR_JRET(getVectorBigintValueFn(pRightCol->info.type, &getVectorBigintValueFnRight)); if (pLeft->numOfRows == 1 && pRight->numOfRows == 1) { SCL_ERR_JRET(vectorMathTsSubHelper(pLeftCol, pRightCol, pOutputCol, pLeft->numOfRows, step, 1, i)); @@ -1381,8 +1398,10 @@ int32_t vectorMathSub(SScalarParam *pLeft, SScalarParam *pRight, SScalarParam *p } } else { double *output = (double *)pOutputCol->pData; - _getDoubleValue_fn_t getVectorDoubleValueFnLeft = getVectorDoubleValueFn(pLeftCol->info.type); - _getDoubleValue_fn_t getVectorDoubleValueFnRight = getVectorDoubleValueFn(pRightCol->info.type); + _getDoubleValue_fn_t getVectorDoubleValueFnLeft; + _getDoubleValue_fn_t getVectorDoubleValueFnRight; + SCL_ERR_JRET(getVectorDoubleValueFn(pLeftCol->info.type, &getVectorDoubleValueFnLeft)); + SCL_ERR_JRET(getVectorDoubleValueFn(pRightCol->info.type, &getVectorDoubleValueFnRight)); if (pLeft->numOfRows == pRight->numOfRows) { for (; i < pRight->numOfRows && i >= 0; i += step, output += 1) { @@ -1412,8 +1431,10 @@ _return: // TODO not correct for descending order scan static int32_t vectorMathMultiplyHelper(SColumnInfoData *pLeftCol, SColumnInfoData *pRightCol, SColumnInfoData *pOutputCol, int32_t numOfRows, int32_t step, int32_t i) { - _getDoubleValue_fn_t getVectorDoubleValueFnLeft = getVectorDoubleValueFn(pLeftCol->info.type); - _getDoubleValue_fn_t getVectorDoubleValueFnRight = getVectorDoubleValueFn(pRightCol->info.type); + _getDoubleValue_fn_t getVectorDoubleValueFnLeft; + _getDoubleValue_fn_t getVectorDoubleValueFnRight; + SCL_ERR_RET(getVectorDoubleValueFn(pLeftCol->info.type, &getVectorDoubleValueFnLeft)); + SCL_ERR_RET(getVectorDoubleValueFn(pRightCol->info.type, &getVectorDoubleValueFnRight)); double *output = (double *)pOutputCol->pData; @@ -1449,8 +1470,10 @@ int32_t vectorMathMultiply(SScalarParam *pLeft, SScalarParam *pRight, SScalarPar SCL_ERR_JRET(vectorConvertVarToDouble(pLeft, &leftConvert, &pLeftCol)); SCL_ERR_JRET(vectorConvertVarToDouble(pRight, &rightConvert, &pRightCol)); - _getDoubleValue_fn_t getVectorDoubleValueFnLeft = getVectorDoubleValueFn(pLeftCol->info.type); - _getDoubleValue_fn_t getVectorDoubleValueFnRight = getVectorDoubleValueFn(pRightCol->info.type); + _getDoubleValue_fn_t getVectorDoubleValueFnLeft; + _getDoubleValue_fn_t getVectorDoubleValueFnRight; + SCL_ERR_JRET(getVectorDoubleValueFn(pLeftCol->info.type, &getVectorDoubleValueFnLeft)); + SCL_ERR_JRET(getVectorDoubleValueFn(pRightCol->info.type, &getVectorDoubleValueFnRight)); double *output = (double *)pOutputCol->pData; if (pLeft->numOfRows == pRight->numOfRows) { @@ -1491,8 +1514,10 @@ int32_t vectorMathDivide(SScalarParam *pLeft, SScalarParam *pRight, SScalarParam SCL_ERR_JRET(vectorConvertVarToDouble(pLeft, &leftConvert, &pLeftCol)); SCL_ERR_JRET(vectorConvertVarToDouble(pRight, &rightConvert, &pRightCol)); - _getDoubleValue_fn_t getVectorDoubleValueFnLeft = getVectorDoubleValueFn(pLeftCol->info.type); - _getDoubleValue_fn_t getVectorDoubleValueFnRight = getVectorDoubleValueFn(pRightCol->info.type); + _getDoubleValue_fn_t getVectorDoubleValueFnLeft; + _getDoubleValue_fn_t getVectorDoubleValueFnRight; + SCL_ERR_JRET(getVectorDoubleValueFn(pLeftCol->info.type, &getVectorDoubleValueFnLeft)); + SCL_ERR_JRET(getVectorDoubleValueFn(pRightCol->info.type, &getVectorDoubleValueFnRight)); double *output = (double *)pOutputCol->pData; if (pLeft->numOfRows == pRight->numOfRows) { @@ -1573,8 +1598,10 @@ int32_t vectorMathRemainder(SScalarParam *pLeft, SScalarParam *pRight, SScalarPa SCL_ERR_JRET(vectorConvertVarToDouble(pLeft, &leftConvert, &pLeftCol)); SCL_ERR_JRET(vectorConvertVarToDouble(pRight, &rightConvert, &pRightCol)); - _getDoubleValue_fn_t getVectorDoubleValueFnLeft = getVectorDoubleValueFn(pLeftCol->info.type); - _getDoubleValue_fn_t getVectorDoubleValueFnRight = getVectorDoubleValueFn(pRightCol->info.type); + _getDoubleValue_fn_t getVectorDoubleValueFnLeft; + _getDoubleValue_fn_t getVectorDoubleValueFnRight; + SCL_ERR_JRET(getVectorDoubleValueFn(pLeftCol->info.type, &getVectorDoubleValueFnLeft)); + SCL_ERR_JRET(getVectorDoubleValueFn(pRightCol->info.type, &getVectorDoubleValueFnRight)); double *output = (double *)pOutputCol->pData; @@ -1661,7 +1688,8 @@ int32_t vectorMathMinus(SScalarParam *pLeft, SScalarParam *pRight, SScalarParam SColumnInfoData *pLeftCol = NULL; SCL_ERR_JRET(vectorConvertVarToDouble(pLeft, &leftConvert, &pLeftCol)); - _getDoubleValue_fn_t getVectorDoubleValueFnLeft = getVectorDoubleValueFn(pLeftCol->info.type); + _getDoubleValue_fn_t getVectorDoubleValueFnLeft; + SCL_ERR_JRET(getVectorDoubleValueFn(pLeftCol->info.type, &getVectorDoubleValueFnLeft)); double *output = (double *)pOutputCol->pData; for (; i < pLeft->numOfRows && i >= 0; i += step, output += 1) { @@ -1692,15 +1720,20 @@ int32_t vectorAssign(SScalarParam *pLeft, SScalarParam *pRight, SScalarParam *pO } } - ASSERT(pRight->numOfQualified == 1 || pRight->numOfQualified == 0); + if (pRight->numOfQualified != 1 && pRight->numOfQualified != 0) { + sclError("vectorAssign: invalid qualified number %d", pRight->numOfQualified); + SCL_ERR_RET(TSDB_CODE_APP_ERROR); + } pOut->numOfQualified = pRight->numOfQualified * pOut->numOfRows; return TSDB_CODE_SUCCESS; } static int32_t vectorBitAndHelper(SColumnInfoData *pLeftCol, SColumnInfoData *pRightCol, SColumnInfoData *pOutputCol, int32_t numOfRows, int32_t step, int32_t i) { - _getBigintValue_fn_t getVectorBigintValueFnLeft = getVectorBigintValueFn(pLeftCol->info.type); - _getBigintValue_fn_t getVectorBigintValueFnRight = getVectorBigintValueFn(pRightCol->info.type); + _getBigintValue_fn_t getVectorBigintValueFnLeft; + _getBigintValue_fn_t getVectorBigintValueFnRight; + SCL_ERR_RET(getVectorBigintValueFn(pLeftCol->info.type, &getVectorBigintValueFnLeft)); + SCL_ERR_RET(getVectorBigintValueFn(pRightCol->info.type, &getVectorBigintValueFnRight)); int64_t *output = (int64_t *)pOutputCol->pData; @@ -1736,8 +1769,10 @@ int32_t vectorBitAnd(SScalarParam *pLeft, SScalarParam *pRight, SScalarParam *pO SCL_ERR_JRET(vectorConvertVarToDouble(pLeft, &leftConvert, &pLeftCol)); SCL_ERR_JRET(vectorConvertVarToDouble(pRight, &rightConvert, &pRightCol)); - _getBigintValue_fn_t getVectorBigintValueFnLeft = getVectorBigintValueFn(pLeftCol->info.type); - _getBigintValue_fn_t getVectorBigintValueFnRight = getVectorBigintValueFn(pRightCol->info.type); + _getBigintValue_fn_t getVectorBigintValueFnLeft; + _getBigintValue_fn_t getVectorBigintValueFnRight; + SCL_ERR_JRET(getVectorBigintValueFn(pLeftCol->info.type, &getVectorBigintValueFnLeft)); + SCL_ERR_JRET(getVectorBigintValueFn(pRightCol->info.type, &getVectorBigintValueFnRight)); int64_t *output = (int64_t *)pOutputCol->pData; if (pLeft->numOfRows == pRight->numOfRows) { @@ -1766,8 +1801,10 @@ _return: static int32_t vectorBitOrHelper(SColumnInfoData *pLeftCol, SColumnInfoData *pRightCol, SColumnInfoData *pOutputCol, int32_t numOfRows, int32_t step, int32_t i) { - _getBigintValue_fn_t getVectorBigintValueFnLeft = getVectorBigintValueFn(pLeftCol->info.type); - _getBigintValue_fn_t getVectorBigintValueFnRight = getVectorBigintValueFn(pRightCol->info.type); + _getBigintValue_fn_t getVectorBigintValueFnLeft; + _getBigintValue_fn_t getVectorBigintValueFnRight; + SCL_ERR_RET(getVectorBigintValueFn(pLeftCol->info.type, &getVectorBigintValueFnLeft)); + SCL_ERR_RET(getVectorBigintValueFn(pRightCol->info.type, &getVectorBigintValueFnRight)); int64_t *output = (int64_t *)pOutputCol->pData; @@ -1803,8 +1840,10 @@ int32_t vectorBitOr(SScalarParam *pLeft, SScalarParam *pRight, SScalarParam *pOu SCL_ERR_JRET(vectorConvertVarToDouble(pLeft, &leftConvert, &pLeftCol)); SCL_ERR_JRET(vectorConvertVarToDouble(pRight, &rightConvert, &pRightCol)); - _getBigintValue_fn_t getVectorBigintValueFnLeft = getVectorBigintValueFn(pLeftCol->info.type); - _getBigintValue_fn_t getVectorBigintValueFnRight = getVectorBigintValueFn(pRightCol->info.type); + _getBigintValue_fn_t getVectorBigintValueFnLeft; + _getBigintValue_fn_t getVectorBigintValueFnRight; + SCL_ERR_JRET(getVectorBigintValueFn(pLeftCol->info.type, &getVectorBigintValueFnLeft)); + SCL_ERR_JRET(getVectorBigintValueFn(pRightCol->info.type, &getVectorBigintValueFnRight)); int64_t *output = (int64_t *)pOutputCol->pData; if (pLeft->numOfRows == pRight->numOfRows) { @@ -1892,7 +1931,8 @@ int32_t doVectorCompareImpl(SScalarParam *pLeft, SScalarParam *pRight, SScalarPa &leftOut, &rightOut, &isJsonnull, &freeLeft, &freeRight, &result)); if (isJsonnull) { - ASSERT(0); + sclError("doVectorCompareImpl: invalid json null value"); + SCL_ERR_RET(TSDB_CODE_APP_ERROR); } if (!pLeftData || !pRightData) { diff --git a/source/libs/scalar/test/filter/CMakeLists.txt b/source/libs/scalar/test/filter/CMakeLists.txt index 94af1eb6f0..44a0395286 100644 --- a/source/libs/scalar/test/filter/CMakeLists.txt +++ b/source/libs/scalar/test/filter/CMakeLists.txt @@ -1,7 +1,7 @@ MESSAGE(STATUS "build filter unit test") -IF(NOT TD_DARWIN) +IF(TD_DARWIN) # GoogleTest requires at least C++11 SET(CMAKE_CXX_STANDARD 11) AUX_SOURCE_DIRECTORY(${CMAKE_CURRENT_SOURCE_DIR} SOURCE_LIST) diff --git a/source/libs/scalar/test/filter/filterTests.cpp b/source/libs/scalar/test/filter/filterTests.cpp index 33d2c1e2ef..b970bf5297 100644 --- a/source/libs/scalar/test/filter/filterTests.cpp +++ b/source/libs/scalar/test/filter/filterTests.cpp @@ -113,8 +113,7 @@ int32_t flttMakeColumnNode(SNode **pNode, SSDataBlock **block, int32_t dataType, if (NULL == *block) { SSDataBlock *res = NULL; - int32_t code = createDataBlock(&res); - ASSERT(code == 0); + FLT_ERR_RET(createDataBlock(&res)); for (int32_t i = 0; i < 2; ++i) { SColumnInfoData idata = createColumnInfoData(TSDB_DATA_TYPE_NULL, 10, 1 + i); diff --git a/source/libs/scalar/test/scalar/scalarTests.cpp b/source/libs/scalar/test/scalar/scalarTests.cpp index 032b197046..c29f7b8a5b 100644 --- a/source/libs/scalar/test/scalar/scalarTests.cpp +++ b/source/libs/scalar/test/scalar/scalarTests.cpp @@ -115,7 +115,10 @@ int32_t scltAppendReservedSlot(SArray *pBlockList, int16_t *dataBlockId, int16_t res->info.capacity = rows; res->info.rows = rows; SColumnInfoData *p = static_cast(taosArrayGet(res->pDataBlock, 0)); - ASSERT(p->pData != NULL && p->nullbitmap != NULL); + if (p->pData == NULL || p->nullbitmap == NULL) { + sclError("data block is not initialized since pData or nullbitmap is NULL"); + SCL_ERR_RET(TSDB_CODE_APP_ERROR); + } (void)taosArrayPush(pBlockList, &res); *dataBlockId = taosArrayGetSize(pBlockList) - 1; @@ -189,8 +192,7 @@ int32_t scltMakeColumnNode(SNode **pNode, SSDataBlock **block, int32_t dataType, if (NULL == *block) { SSDataBlock *res = NULL; - int32_t code = createDataBlock(&res); - ASSERT(code == 0); + SCL_ERR_RET(createDataBlock(&res)); for (int32_t i = 0; i < 2; ++i) { SColumnInfoData idata = createColumnInfoData(TSDB_DATA_TYPE_INT, 10, i + 1); @@ -1422,8 +1424,10 @@ int32_t makeCalculate(void *json, void *key, int32_t rightType, void *rightData, opType == OP_TYPE_NMATCH) { (void)printf("op:%s,3result:%d,except:%f\n", operatorTypeStr(opType), *((bool *)colDataGetData(column, 0)), exceptValue); - assert(*(bool *)colDataGetData(column, 0) == exceptValue); -// ASSERT_EQ((int) *((bool *)colDataGetData(column, 0)), (int)exceptValue); + if(*(bool *)colDataGetData(column, 0) != exceptValue) { + (void)printf("expect value %d, but got %d\n", *((bool *)colDataGetData(column, 0)), exceptValue); + SCL_ERR_RET(TSDB_CODE_FAILED); + } } taosArrayDestroyEx(blockList, scltFreeDataBlock); diff --git a/source/util/src/terror.c b/source/util/src/terror.c index 396abf21a7..c04e34b443 100644 --- a/source/util/src/terror.c +++ b/source/util/src/terror.c @@ -478,6 +478,9 @@ TAOS_DEFINE_ERROR(TSDB_CODE_QRY_INVALID_WINDOW_CONDITION, "The time pseudo colum TAOS_DEFINE_ERROR(TSDB_CODE_QRY_EXECUTOR_INTERNAL_ERROR, "Executor internal error") TAOS_DEFINE_ERROR(TSDB_CODE_QRY_INVALID_JOIN_CONDITION, "Not supported join on condition") TAOS_DEFINE_ERROR(TSDB_CODE_QRY_FILTER_NOT_SUPPORT_TYPE, "Not supported range type") +TAOS_DEFINE_ERROR(TSDB_CODE_QRY_FILTER_WRONG_OPTR_TYPE, "Wrong operator type") +TAOS_DEFINE_ERROR(TSDB_CODE_QRY_FILTER_RANGE_ERROR, "Wrong filter range") +TAOS_DEFINE_ERROR(TSDB_CODE_QRY_FILTER_INVALID_TYPE, "Invalid filter type") // grant TAOS_DEFINE_ERROR(TSDB_CODE_GRANT_EXPIRED, "License expired") From 6055b9172ec4e1192c077b7de1f57438ef444388 Mon Sep 17 00:00:00 2001 From: kailixu Date: Tue, 20 Aug 2024 09:03:57 +0800 Subject: [PATCH 09/28] fix: memory leak of geos --- source/libs/geometry/src/geosWrapper.c | 51 +++++++------------------- source/util/src/tgeosctx.c | 4 +- 2 files changed, 15 insertions(+), 40 deletions(-) diff --git a/source/libs/geometry/src/geosWrapper.c b/source/libs/geometry/src/geosWrapper.c index 4f3f7d75c2..7372521276 100644 --- a/source/libs/geometry/src/geosWrapper.c +++ b/source/libs/geometry/src/geosWrapper.c @@ -37,10 +37,7 @@ int32_t initCtxMakePoint() { int32_t code = TSDB_CODE_FAILED; SGeosContext *geosCtx = getThreadLocalGeosCtx(); - if (!geosCtx) { - code = TSDB_CODE_OUT_OF_MEMORY; - return code; - } + if (!geosCtx) return TSDB_CODE_OUT_OF_MEMORY; if (geosCtx->handle == NULL) { geosCtx->handle = GEOS_init_r(); @@ -67,10 +64,7 @@ int32_t doMakePoint(double x, double y, unsigned char **outputGeom, size_t *size int32_t code = TSDB_CODE_FAILED; SGeosContext *geosCtx = getThreadLocalGeosCtx(); - if (!geosCtx) { - code = TSDB_CODE_OUT_OF_MEMORY; - return code; - } + if (!geosCtx) return TSDB_CODE_OUT_OF_MEMORY; GEOSGeometry *geom = NULL; unsigned char *wkb = NULL; @@ -177,10 +171,7 @@ int32_t initCtxGeomFromText() { int32_t code = TSDB_CODE_FAILED; SGeosContext *geosCtx = getThreadLocalGeosCtx(); - if (!geosCtx) { - code = TSDB_CODE_OUT_OF_MEMORY; - return code; - } + if (!geosCtx) return TSDB_CODE_OUT_OF_MEMORY; if (geosCtx->handle == NULL) { geosCtx->handle = GEOS_init_r(); @@ -218,10 +209,7 @@ int32_t doGeomFromText(const char *inputWKT, unsigned char **outputGeom, size_t int32_t code = TSDB_CODE_FAILED; SGeosContext *geosCtx = getThreadLocalGeosCtx(); - if (!geosCtx) { - code = TSDB_CODE_OUT_OF_MEMORY; - return code; - } + if (!geosCtx) return TSDB_CODE_OUT_OF_MEMORY; GEOSGeometry *geom = NULL; unsigned char *wkb = NULL; @@ -258,10 +246,7 @@ int32_t initCtxAsText() { int32_t code = TSDB_CODE_FAILED; SGeosContext *geosCtx = getThreadLocalGeosCtx(); - if (!geosCtx) { - code = TSDB_CODE_OUT_OF_MEMORY; - return code; - } + if (!geosCtx) return TSDB_CODE_OUT_OF_MEMORY; if (geosCtx->handle == NULL) { geosCtx->handle = GEOS_init_r(); @@ -299,10 +284,7 @@ int32_t doAsText(const unsigned char *inputGeom, size_t size, char **outputWKT) int32_t code = TSDB_CODE_FAILED; SGeosContext *geosCtx = getThreadLocalGeosCtx(); - if (!geosCtx) { - code = TSDB_CODE_OUT_OF_MEMORY; - return code; - } + if (!geosCtx) return TSDB_CODE_OUT_OF_MEMORY; GEOSGeometry *geom = NULL; char *wkt = NULL; @@ -335,10 +317,7 @@ int32_t initCtxRelationFunc() { int32_t code = TSDB_CODE_FAILED; SGeosContext *geosCtx = getThreadLocalGeosCtx(); - if (!geosCtx) { - code = TSDB_CODE_OUT_OF_MEMORY; - return code; - } + if (!geosCtx) return TSDB_CODE_OUT_OF_MEMORY; if (geosCtx->handle == NULL) { geosCtx->handle = GEOS_init_r(); @@ -365,9 +344,7 @@ int32_t doGeosRelation(const GEOSGeometry *geom1, const GEOSPreparedGeometry *pr _geosPreparedRelationFunc_t swappedPreparedRelationFn) { SGeosContext *geosCtx = getThreadLocalGeosCtx(); - if (!geosCtx) { - return TSDB_CODE_OUT_OF_MEMORY; - } + if (!geosCtx) return TSDB_CODE_OUT_OF_MEMORY; if (!preparedGeom1) { if (!swapped) { @@ -429,11 +406,6 @@ int32_t doContainsProperly(const GEOSGeometry *geom1, const GEOSPreparedGeometry // need to call destroyGeometry(outputGeom, outputPreparedGeom) later int32_t readGeometry(const unsigned char *input, GEOSGeometry **outputGeom, const GEOSPreparedGeometry **outputPreparedGeom) { - SGeosContext *geosCtx = getThreadLocalGeosCtx(); - if (!geosCtx) { - return TSDB_CODE_OUT_OF_MEMORY; - } - ASSERT(outputGeom); // it is not allowed if outputGeom is NULL *outputGeom = NULL; @@ -445,6 +417,10 @@ int32_t readGeometry(const unsigned char *input, GEOSGeometry **outputGeom, return TSDB_CODE_SUCCESS; } + SGeosContext *geosCtx = getThreadLocalGeosCtx(); + + if (!geosCtx) return TSDB_CODE_OUT_OF_MEMORY; + *outputGeom = GEOSWKBReader_read_r(geosCtx->handle, geosCtx->WKBReader, varDataVal(input), varDataLen(input)); if (*outputGeom == NULL) { return TSDB_CODE_FUNC_FUNTION_PARA_VALUE; @@ -461,7 +437,8 @@ int32_t readGeometry(const unsigned char *input, GEOSGeometry **outputGeom, } void destroyGeometry(GEOSGeometry **geom, const GEOSPreparedGeometry **preparedGeom) { - SGeosContext *geosCtx = getThreadLocalGeosCtx(); + SGeosContext *geosCtx = acquireThreadLocalGeosCtx(); + if (!geosCtx) return; if (preparedGeom && *preparedGeom) { GEOSPreparedGeom_destroy_r(geosCtx->handle, *preparedGeom); diff --git a/source/util/src/tgeosctx.c b/source/util/src/tgeosctx.c index 473b7539fc..5d47452fda 100644 --- a/source/util/src/tgeosctx.c +++ b/source/util/src/tgeosctx.c @@ -31,9 +31,7 @@ static threadlocal SGeosContext *tlGeosCtx = NULL; SGeosContext *acquireThreadLocalGeosCtx() { return tlGeosCtx; } SGeosContext *getThreadLocalGeosCtx() { - if (tlGeosCtx) { - return tlGeosCtx; - } + if (tlGeosCtx) return tlGeosCtx; taosWLockLatch(&sGeosPool.lock); if (sGeosPool.size >= sGeosPool.capacity) { From a2217973aeb70a8d7651263c7f11c6dd6e0c7a99 Mon Sep 17 00:00:00 2001 From: kailixu Date: Tue, 20 Aug 2024 09:07:29 +0800 Subject: [PATCH 10/28] fix: memory leak of geos --- source/libs/scalar/src/sclvector.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/libs/scalar/src/sclvector.c b/source/libs/scalar/src/sclvector.c index daf44ec527..e20b0cb6fc 100644 --- a/source/libs/scalar/src/sclvector.c +++ b/source/libs/scalar/src/sclvector.c @@ -445,7 +445,7 @@ static FORCE_INLINE int32_t varToGeometry(char *buf, SScalarParam *pOut, int32_t sclError("failed to init geometry ctx, %s", getGeosErrMsg(code)); SCL_ERR_JRET(TSDB_CODE_APP_ERROR); } - if ((code = doGeomFromText(buf, &t, &len))) { + if ((code = doGeomFromText(buf, &t, &len)) != 0) { sclInfo("failed to convert text to geometry, %s", getGeosErrMsg(code)); SCL_ERR_JRET(TSDB_CODE_SCALAR_CONVERT_ERROR); } From 2f4d0815920a46e45f38fb1e927528e4ff5ca07f Mon Sep 17 00:00:00 2001 From: kailixu Date: Tue, 20 Aug 2024 09:08:22 +0800 Subject: [PATCH 11/28] fix: memory leak of geos --- source/libs/scalar/src/sclvector.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/libs/scalar/src/sclvector.c b/source/libs/scalar/src/sclvector.c index e20b0cb6fc..9bac02e5f9 100644 --- a/source/libs/scalar/src/sclvector.c +++ b/source/libs/scalar/src/sclvector.c @@ -446,7 +446,7 @@ static FORCE_INLINE int32_t varToGeometry(char *buf, SScalarParam *pOut, int32_t SCL_ERR_JRET(TSDB_CODE_APP_ERROR); } if ((code = doGeomFromText(buf, &t, &len)) != 0) { - sclInfo("failed to convert text to geometry, %s", getGeosErrMsg(code)); + sclError("failed to convert text to geometry, %s", getGeosErrMsg(code)); SCL_ERR_JRET(TSDB_CODE_SCALAR_CONVERT_ERROR); } From 0e25059039a154f16e67cb15a6514fca8ec023a2 Mon Sep 17 00:00:00 2001 From: Shungang Li Date: Tue, 20 Aug 2024 09:17:50 +0800 Subject: [PATCH 12/28] fix: s3 protocolG/uriStyleG init --- source/common/src/cos.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/source/common/src/cos.c b/source/common/src/cos.c index a5a278e82e..6e9c7dd50d 100644 --- a/source/common/src/cos.c +++ b/source/common/src/cos.c @@ -56,10 +56,8 @@ int32_t s3Begin() { } for (int i = 0; i < tsS3EpNum; i++) { - protocolG[i] = !tsS3Https[i]; - if (tsS3Oss[i]) { - uriStyleG[i] = S3UriStyleVirtualHost; - } + protocolG[i] = tsS3Https[i] ? S3ProtocolHTTPS : S3ProtocolHTTP; + uriStyleG[i] = tsS3Oss[i] ? S3UriStyleVirtualHost : S3UriStylePath; } TAOS_RETURN(TSDB_CODE_SUCCESS); From 1f1a2d9f6c7f464ad31d941817c4f369609eee05 Mon Sep 17 00:00:00 2001 From: Shengliang Guan Date: Tue, 20 Aug 2024 09:54:26 +0800 Subject: [PATCH 13/28] fix: handle error code --- include/libs/sync/sync.h | 30 ++++++++++----------- source/libs/sync/inc/syncRaftCfg.h | 1 - source/libs/sync/inc/syncUtil.h | 2 +- source/libs/sync/src/syncMain.c | 26 +++++++++++++----- source/libs/sync/src/syncRaftCfg.c | 42 +++++++++++++++++------------- 5 files changed, 60 insertions(+), 41 deletions(-) diff --git a/include/libs/sync/sync.h b/include/libs/sync/sync.h index 1fb077e3ca..07d56f9b07 100644 --- a/include/libs/sync/sync.h +++ b/include/libs/sync/sync.h @@ -156,11 +156,11 @@ typedef struct SSnapshotParam { typedef struct SSnapshot { int32_t type; - SSyncTLV* data; + SSyncTLV* data; ESyncFsmState state; - SyncIndex lastApplyIndex; - SyncTerm lastApplyTerm; - SyncIndex lastConfigIndex; + SyncIndex lastApplyIndex; + SyncTerm lastApplyTerm; + SyncIndex lastConfigIndex; } SSnapshot; typedef struct SSnapshotMeta { @@ -263,16 +263,16 @@ typedef struct SSyncState { int64_t startTimeMs; } SSyncState; -int32_t syncInit(); -void syncCleanUp(); -int64_t syncOpen(SSyncInfo* pSyncInfo, int32_t vnodeVersion); -int32_t syncStart(int64_t rid); -void syncStop(int64_t rid); -void syncPreStop(int64_t rid); -void syncPostStop(int64_t rid); -int32_t syncPropose(int64_t rid, SRpcMsg* pMsg, bool isWeak, int64_t* seq); -int32_t syncCheckMember(int64_t rid); -int32_t syncIsCatchUp(int64_t rid); +int32_t syncInit(); +void syncCleanUp(); +int64_t syncOpen(SSyncInfo* pSyncInfo, int32_t vnodeVersion); +int32_t syncStart(int64_t rid); +void syncStop(int64_t rid); +void syncPreStop(int64_t rid); +void syncPostStop(int64_t rid); +int32_t syncPropose(int64_t rid, SRpcMsg* pMsg, bool isWeak, int64_t* seq); +int32_t syncCheckMember(int64_t rid); +int32_t syncIsCatchUp(int64_t rid); ESyncRole syncGetRole(int64_t rid); int64_t syncGetTerm(int64_t rid); int32_t syncProcessMsg(int64_t rid, SRpcMsg* pMsg); @@ -296,7 +296,7 @@ int32_t syncGetAssignedLogSynced(int64_t rid); void syncGetRetryEpSet(int64_t rid, SEpSet* pEpSet); const char* syncStr(ESyncState state); -int32_t syncNodeGetConfig(int64_t rid, SSyncCfg *cfg); +int32_t syncNodeGetConfig(int64_t rid, SSyncCfg* cfg); // util int32_t syncSnapInfoDataRealloc(SSnapshot* pSnap, int32_t size); diff --git a/source/libs/sync/inc/syncRaftCfg.h b/source/libs/sync/inc/syncRaftCfg.h index 4f03a60fbc..2c1626c4e8 100644 --- a/source/libs/sync/inc/syncRaftCfg.h +++ b/source/libs/sync/inc/syncRaftCfg.h @@ -24,7 +24,6 @@ extern "C" { int32_t syncWriteCfgFile(SSyncNode *pNode); int32_t syncReadCfgFile(SSyncNode *pNode); -int32_t syncAddCfgIndex(SSyncNode *pNode, SyncIndex cfgIndex); #ifdef __cplusplus } diff --git a/source/libs/sync/inc/syncUtil.h b/source/libs/sync/inc/syncUtil.h index 555607d40c..2b5b818da3 100644 --- a/source/libs/sync/inc/syncUtil.h +++ b/source/libs/sync/inc/syncUtil.h @@ -107,7 +107,7 @@ void syncLogRecvAppendEntries(SSyncNode* pSyncNode, const SyncAppendEntries* pMs void syncLogSendAppendEntries(SSyncNode* pSyncNode, const SyncAppendEntries* pMsg, const char* s); void syncLogRecvRequestVote(SSyncNode* pSyncNode, const SyncRequestVote* pMsg, int32_t voteGranted, const char* s); -void syncLogSendRequestVote(SSyncNode* pNode, const SyncRequestVote* pMsg, const char* s); +void syncLogSendRequestVote(SSyncNode* pSyncNode, const SyncRequestVote* pMsg, const char* s); void syncLogRecvRequestVoteReply(SSyncNode* pSyncNode, const SyncRequestVoteReply* pMsg, const char* s); void syncLogSendRequestVoteReply(SSyncNode* pSyncNode, const SyncRequestVoteReply* pMsg, const char* s); diff --git a/source/libs/sync/src/syncMain.c b/source/libs/sync/src/syncMain.c index 5465007b18..8b8a9e1279 100644 --- a/source/libs/sync/src/syncMain.c +++ b/source/libs/sync/src/syncMain.c @@ -50,7 +50,7 @@ static int32_t syncHbTimerStart(SSyncNode* pSyncNode, SSyncTimer* pSyncTimer); static int32_t syncHbTimerStop(SSyncNode* pSyncNode, SSyncTimer* pSyncTimer); static int32_t syncNodeUpdateNewConfigIndex(SSyncNode* ths, SSyncCfg* pNewCfg); static bool syncNodeInConfig(SSyncNode* pSyncNode, const SSyncCfg* config); -static void syncNodeDoConfigChange(SSyncNode* pSyncNode, SSyncCfg* newConfig, SyncIndex lastConfigChangeIndex); +static int32_t syncNodeDoConfigChange(SSyncNode* pSyncNode, SSyncCfg* newConfig, SyncIndex lastConfigChangeIndex); static bool syncNodeIsOptimizedOneReplica(SSyncNode* ths, SRpcMsg* pMsg); static bool syncNodeCanChange(SSyncNode* pSyncNode); @@ -182,7 +182,12 @@ int32_t syncReconfig(int64_t rid, SSyncCfg* pNewCfg) { } TAOS_CHECK_RETURN(syncNodeUpdateNewConfigIndex(pSyncNode, pNewCfg)); - syncNodeDoConfigChange(pSyncNode, pNewCfg, pNewCfg->lastIndex); + + if (syncNodeDoConfigChange(pSyncNode, pNewCfg, pNewCfg->lastIndex) != 0) { + code = TSDB_CODE_SYN_NEW_CONFIG_ERROR; + sError("vgId:%d, failed to reconfig since do change error", pSyncNode->vgId); + TAOS_RETURN(code); + } if (pSyncNode->state == TAOS_SYNC_STATE_LEADER || pSyncNode->state == TAOS_SYNC_STATE_ASSIGNED_LEADER) { // TODO check return value @@ -1015,7 +1020,7 @@ SSyncNode* syncNodeOpen(SSyncInfo* pSyncInfo, int32_t vnodeVersion) { if (!taosDirExist((char*)(pSyncInfo->path))) { if (taosMkDir(pSyncInfo->path) != 0) { terrno = TAOS_SYSTEM_ERROR(errno); - sError("failed to create dir:%s since %s", pSyncInfo->path, terrstr()); + sError("vgId:%d, failed to create dir:%s since %s", pSyncInfo->vgId, pSyncInfo->path, terrstr()); goto _error; } } @@ -1766,11 +1771,11 @@ static bool syncIsConfigChanged(const SSyncCfg* pOldCfg, const SSyncCfg* pNewCfg return false; } -void syncNodeDoConfigChange(SSyncNode* pSyncNode, SSyncCfg* pNewConfig, SyncIndex lastConfigChangeIndex) { +int32_t syncNodeDoConfigChange(SSyncNode* pSyncNode, SSyncCfg* pNewConfig, SyncIndex lastConfigChangeIndex) { SSyncCfg oldConfig = pSyncNode->raftCfg.cfg; if (!syncIsConfigChanged(&oldConfig, pNewConfig)) { sInfo("vgId:1, sync not reconfig since not changed"); - return; + return 0; } pSyncNode->raftCfg.cfg = *pNewConfig; @@ -1809,7 +1814,15 @@ void syncNodeDoConfigChange(SSyncNode* pSyncNode, SSyncCfg* pNewConfig, SyncInde } // add last config index - (void)syncAddCfgIndex(pSyncNode, lastConfigChangeIndex); + SRaftCfg* pCfg = &pSyncNode->raftCfg; + if (pCfg->configIndexCount >= MAX_CONFIG_INDEX_COUNT) { + sNError(pSyncNode, "failed to add cfg index:%d since out of range", pCfg->configIndexCount); + terrno = TSDB_CODE_OUT_OF_RANGE; + return -1; + } + + pCfg->configIndexArr[pCfg->configIndexCount] = lastConfigChangeIndex; + pCfg->configIndexCount++; if (IamInNew) { //----------------------------------------- @@ -1924,6 +1937,7 @@ void syncNodeDoConfigChange(SSyncNode* pSyncNode, SSyncCfg* pNewConfig, SyncInde _END: // log end config change sNInfo(pSyncNode, "end do config change, from %d to %d", oldConfig.totalReplicaNum, pNewConfig->totalReplicaNum); + return 0; } // raft state change -------------- diff --git a/source/libs/sync/src/syncRaftCfg.c b/source/libs/sync/src/syncRaftCfg.c index b0e6abffc6..82cc86ed86 100644 --- a/source/libs/sync/src/syncRaftCfg.c +++ b/source/libs/sync/src/syncRaftCfg.c @@ -18,7 +18,7 @@ #include "syncUtil.h" #include "tjson.h" -const char *syncRoleToStr(ESyncRole role) { +static const char *syncRoleToStr(ESyncRole role) { switch (role) { case TAOS_SYNC_ROLE_VOTER: return "true"; @@ -29,15 +29,14 @@ const char *syncRoleToStr(ESyncRole role) { } } -const ESyncRole syncStrToRole(char *str) { +static const ESyncRole syncStrToRole(char *str) { if (strcmp(str, "true") == 0) { return TAOS_SYNC_ROLE_VOTER; - } - if (strcmp(str, "false") == 0) { + } else if (strcmp(str, "false") == 0) { return TAOS_SYNC_ROLE_LEARNER; + } else { + return TAOS_SYNC_ROLE_ERROR; } - - return TAOS_SYNC_ROLE_ERROR; } static int32_t syncEncodeSyncCfg(const void *pObj, SJson *pJson) { @@ -52,10 +51,12 @@ static int32_t syncEncodeSyncCfg(const void *pObj, SJson *pJson) { if (nodeInfo == NULL) { TAOS_CHECK_EXIT(TSDB_CODE_OUT_OF_MEMORY); } + if ((code = tjsonAddItemToObject(pJson, "nodeInfo", nodeInfo)) < 0) { tjsonDelete(nodeInfo); TAOS_CHECK_EXIT(code); } + for (int32_t i = 0; i < pCfg->totalReplicaNum; ++i) { SJson *info = tjsonCreateObject(); if (info == NULL) { @@ -68,20 +69,25 @@ static int32_t syncEncodeSyncCfg(const void *pObj, SJson *pJson) { TAOS_CHECK_GOTO(tjsonAddStringToObject(info, "isReplica", syncRoleToStr(pCfg->nodeInfo[i].nodeRole)), NULL, _err); TAOS_CHECK_GOTO(tjsonAddItemToArray(nodeInfo, info), NULL, _err); continue; + _err: tjsonDelete(info); break; } + _exit: if (code < 0) { sError("failed to encode sync cfg at line %d since %s", lino, tstrerror(code)); } + TAOS_RETURN(code); } static int32_t syncEncodeRaftCfg(const void *pObj, SJson *pJson) { SRaftCfg *pCfg = (SRaftCfg *)pObj; - int32_t code = 0, lino = 0; + int32_t code = 0; + int32_t lino = 0; + TAOS_CHECK_EXIT(tjsonAddObject(pJson, "SSyncCfg", syncEncodeSyncCfg, (void *)&pCfg->cfg)); TAOS_CHECK_EXIT(tjsonAddDoubleToObject(pJson, "isStandBy", pCfg->isStandBy)); TAOS_CHECK_EXIT(tjsonAddDoubleToObject(pJson, "snapshotStrategy", pCfg->snapshotStrategy)); @@ -93,10 +99,12 @@ static int32_t syncEncodeRaftCfg(const void *pObj, SJson *pJson) { if (configIndexArr == NULL) { TAOS_CHECK_EXIT(TSDB_CODE_OUT_OF_MEMORY); } + if ((code = tjsonAddItemToObject(pJson, "configIndexArr", configIndexArr)) < 0) { tjsonDelete(configIndexArr); TAOS_CHECK_EXIT(code); } + for (int32_t i = 0; i < pCfg->configIndexCount; ++i) { SJson *configIndex = tjsonCreateObject(); if (configIndex == NULL) { @@ -105,14 +113,17 @@ static int32_t syncEncodeRaftCfg(const void *pObj, SJson *pJson) { TAOS_CHECK_EXIT(tjsonAddIntegerToObject(configIndex, "index", pCfg->configIndexArr[i])); TAOS_CHECK_EXIT(tjsonAddItemToArray(configIndexArr, configIndex)); continue; + _err: tjsonDelete(configIndex); break; } + _exit: if (code < 0) { sError("failed to encode raft cfg at line %d since %s", lino, tstrerror(code)); } + TAOS_RETURN(code); } @@ -124,11 +135,13 @@ int32_t syncWriteCfgFile(SSyncNode *pNode) { const char *realfile = pNode->configPath; SRaftCfg *pCfg = &pNode->raftCfg; char file[PATH_MAX] = {0}; + (void)snprintf(file, sizeof(file), "%s.bak", realfile); if ((pJson = tjsonCreateObject()) == NULL) { TAOS_CHECK_EXIT(TSDB_CODE_OUT_OF_MEMORY); } + TAOS_CHECK_EXIT(tjsonAddObject(pJson, "RaftCfg", syncEncodeRaftCfg, pCfg)); buffer = tjsonToString(pJson); if (buffer == NULL) { @@ -145,6 +158,7 @@ int32_t syncWriteCfgFile(SSyncNode *pNode) { if (taosWriteFile(pFile, buffer, len) <= 0) { TAOS_CHECK_EXIT(TAOS_SYSTEM_ERROR(errno)); } + if (taosFsyncFile(pFile) < 0) { TAOS_CHECK_EXIT(TAOS_SYSTEM_ERROR(errno)); } @@ -165,6 +179,7 @@ _exit: if (code != 0) { sError("vgId:%d, failed to write sync cfg file:%s since %s", pNode->vgId, realfile, tstrerror(code)); } + TAOS_RETURN(code); } @@ -232,6 +247,7 @@ static int32_t syncDecodeRaftCfg(const SJson *pJson, void *pObj) { tjsonGetNumberValue(configIndex, "index", pCfg->configIndexArr[i], code); if (code < 0) return TSDB_CODE_INVALID_JSON_FORMAT; } + return 0; } @@ -292,16 +308,6 @@ _OVER: if (code != 0) { sError("vgId:%d, failed to read sync cfg file:%s since %s", pNode->vgId, file, tstrerror(code)); } + TAOS_RETURN(code); } - -int32_t syncAddCfgIndex(SSyncNode *pNode, SyncIndex cfgIndex) { - SRaftCfg *pCfg = &pNode->raftCfg; - if (pCfg->configIndexCount >= MAX_CONFIG_INDEX_COUNT) { - return TSDB_CODE_OUT_OF_RANGE; - } - - pCfg->configIndexArr[pCfg->configIndexCount] = cfgIndex; - pCfg->configIndexCount++; - return 0; -} \ No newline at end of file From 66878fd040922b8881501d04768d879c9f5316b0 Mon Sep 17 00:00:00 2001 From: kailixu Date: Tue, 20 Aug 2024 10:10:42 +0800 Subject: [PATCH 14/28] fix: memory leak of geos --- source/util/src/tgeosctx.c | 45 ++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/source/util/src/tgeosctx.c b/source/util/src/tgeosctx.c index 5d47452fda..42cde5b8c7 100644 --- a/source/util/src/tgeosctx.c +++ b/source/util/src/tgeosctx.c @@ -14,14 +14,16 @@ */ #include "tgeosctx.h" +#include "tarray.h" #include "tdef.h" #include "tlockfree.h" #include "tlog.h" +#define GEOS_POOL_CAPACITY 64 typedef struct { - SGeosContext *pool; - int32_t capacity; - int32_t size; + SArray *poolArray; // totalSize: (GEOS_POOL_CAPACITY * (taosArrayGetSize(poolArray) - 1)) + size + SGeosContext *pool; // current SGeosContext pool + int32_t size; // size of current SGeosContext pool, size <= GEOS_POOL_CAPACITY SRWLatch lock; } SGeosContextPool; @@ -34,16 +36,26 @@ SGeosContext *getThreadLocalGeosCtx() { if (tlGeosCtx) return tlGeosCtx; taosWLockLatch(&sGeosPool.lock); - if (sGeosPool.size >= sGeosPool.capacity) { - sGeosPool.capacity += 128; - void *tmp = taosMemoryRealloc(sGeosPool.pool, sGeosPool.capacity * sizeof(SGeosContext)); - if (!tmp) { + if (!sGeosPool.pool || sGeosPool.size >= GEOS_POOL_CAPACITY) { + if (!(sGeosPool.pool = (SGeosContext *)taosMemoryCalloc(GEOS_POOL_CAPACITY, sizeof(SGeosContext)))) { taosWUnLockLatch(&sGeosPool.lock); - terrno = TSDB_CODE_OUT_OF_MEMORY; return NULL; } - sGeosPool.pool = tmp; - TAOS_MEMSET(sGeosPool.pool + sGeosPool.size, 0, (sGeosPool.capacity - sGeosPool.size) * sizeof(SGeosContext)); + if (!sGeosPool.poolArray) { + if (!(sGeosPool.poolArray = taosArrayInit(16, POINTER_BYTES))) { + taosMemoryFree(sGeosPool.pool); + sGeosPool.pool = NULL; + taosWUnLockLatch(&sGeosPool.lock); + return NULL; + } + } + if (!taosArrayPush(sGeosPool.poolArray, &sGeosPool.pool)) { + taosMemoryFree(sGeosPool.pool); + sGeosPool.pool = NULL; + taosWUnLockLatch(&sGeosPool.lock); + return NULL; + } + sGeosPool.size = 0; } tlGeosCtx = sGeosPool.pool + sGeosPool.size; ++sGeosPool.size; @@ -91,9 +103,14 @@ static void destroyGeosCtx(SGeosContext *pCtx) { void taosGeosDestroy() { uInfo("geos is cleaned up"); - if (!sGeosPool.pool) return; - for (int32_t i = 0; i < sGeosPool.size; ++i) { - destroyGeosCtx(sGeosPool.pool + i); + int32_t size = taosArrayGetSize(sGeosPool.poolArray); + for (int32_t i = 0; i < size; ++i) { + SGeosContext *pool = *(SGeosContext **)TARRAY_GET_ELEM(sGeosPool.poolArray, i); + for (int32_t j = 0; j < GEOS_POOL_CAPACITY; ++j) { + destroyGeosCtx(pool + j); + } + taosMemoryFree(pool); } - taosMemoryFreeClear(sGeosPool.pool); + taosArrayDestroy(sGeosPool.poolArray); + sGeosPool.poolArray = NULL; } \ No newline at end of file From 1bff0e89f4f3b7b12b4cca070a23a557c341b44c Mon Sep 17 00:00:00 2001 From: 54liuyao <54liuyao> Date: Tue, 20 Aug 2024 10:15:52 +0800 Subject: [PATCH 15/28] fix issue --- source/libs/parser/src/parTranslater.c | 58 ++++++++++++++++++++++---- tests/script/tsim/show/showalive.sim | 44 ++++++++++++++++++- 2 files changed, 92 insertions(+), 10 deletions(-) diff --git a/source/libs/parser/src/parTranslater.c b/source/libs/parser/src/parTranslater.c index a262acb2ec..05cceb656e 100755 --- a/source/libs/parser/src/parTranslater.c +++ b/source/libs/parser/src/parTranslater.c @@ -12628,6 +12628,27 @@ static int32_t createParOperatorNode(EOperatorType opType, const char* pLeftCol, return TSDB_CODE_SUCCESS; } +static int32_t createIsOperatorNode(EOperatorType opType, const char* pColName, SNode** pOp) { + SOperatorNode* pOper = NULL; + int32_t code = nodesMakeNode(QUERY_NODE_OPERATOR, (SNode**)&pOper); + if (NULL == pOper) { + return code; + } + + pOper->opType = opType; + code = nodesMakeNode(QUERY_NODE_COLUMN, (SNode**)&pOper->pLeft); + if (TSDB_CODE_SUCCESS != code) { + nodesDestroyNode((SNode*)pOper); + return code; + } + pOper->pRight = NULL; + + snprintf(((SColumnNode*)pOper->pLeft)->colName, sizeof(((SColumnNode*)pOper->pLeft)->colName), "%s", pColName); + + *pOp = (SNode*)pOper; + return TSDB_CODE_SUCCESS; +} + static const char* getTbNameColName(ENodeType type) { const char* colName; switch (type) { @@ -15035,7 +15056,7 @@ static int32_t rewriteShowAliveStmt(STranslateContext* pCxt, SQuery* pQuery) { // pWhenThenlist and pElse need to free - // case when (v1_status = "leader" or v2_status = "lead er" or v3_status = "leader" or v4_status = "leader") then 1 + // case when (v1_status = "leader" or v2_status = "leader" or v3_status = "leader" or v4_status = "leader") then 1 // else 0 end SNode* pCaseWhen = NULL; code = createParCaseWhenNode(NULL, pWhenThenlist, pElse, NULL, &pCaseWhen); @@ -15190,23 +15211,42 @@ static int32_t rewriteShowAliveStmt(STranslateContext* pCxt, SQuery* pQuery) { return code; } - // pSubSelect, pTemp1, pTempVal need to free - - pThen = NULL; - code = nodesMakeValueNodeFromInt32(1, &pThen); + SNode* pCondIsNULL = NULL; + code = createIsOperatorNode(OP_TYPE_IS_NULL, pSumColAlias, &pCondIsNULL); if (TSDB_CODE_SUCCESS != code) { nodesDestroyNode((SNode*)pSubSelect); nodesDestroyNode(pTemp1); nodesDestroyNode(pTempVal); return code; } - // pSubSelect, pTemp1, pThen, pTempVal need to free - pWhenThen = NULL; - code = createParWhenThenNode(pTemp1, pThen, &pWhenThen); + SNode* pCondFull1 = NULL; + code = createLogicCondNode(&pTemp1, &pCondIsNULL, &pCondFull1, LOGIC_COND_TYPE_OR); if (TSDB_CODE_SUCCESS != code) { nodesDestroyNode((SNode*)pSubSelect); nodesDestroyNode(pTemp1); + nodesDestroyNode(pTempVal); + nodesDestroyNode(pCondIsNULL); + return code; + } + + // pSubSelect, pCondFull1, pTempVal need to free + + pThen = NULL; + code = nodesMakeValueNodeFromInt32(1, &pThen); + if (TSDB_CODE_SUCCESS != code) { + nodesDestroyNode((SNode*)pSubSelect); + nodesDestroyNode(pCondFull1); + nodesDestroyNode(pTempVal); + return code; + } + // pSubSelect, pCondFull1, pThen, pTempVal need to free + + pWhenThen = NULL; + code = createParWhenThenNode(pCondFull1, pThen, &pWhenThen); + if (TSDB_CODE_SUCCESS != code) { + nodesDestroyNode((SNode*)pSubSelect); + nodesDestroyNode(pCondFull1); nodesDestroyNode(pThen); nodesDestroyNode(pTempVal); return code; @@ -15289,7 +15329,7 @@ static int32_t rewriteShowAliveStmt(STranslateContext* pCxt, SQuery* pQuery) { } // pSubSelect, pWhenThenlist need to free - // case when leader_col = count_col and count_col > 0 then 1 when leader_col < count_col and count_col > 0 then 2 else + // case when leader_col = count_col and leader_col > 0 then 1 when leader_col < count_col and leader_col > 0 then 2 else // 0 end as status pElse = NULL; code = nodesMakeValueNodeFromInt32(0, &pElse); diff --git a/tests/script/tsim/show/showalive.sim b/tests/script/tsim/show/showalive.sim index 4cad1da01d..72fad47f57 100644 --- a/tests/script/tsim/show/showalive.sim +++ b/tests/script/tsim/show/showalive.sim @@ -21,6 +21,30 @@ sql create dnode $hostname4 port 7500 sleep 1000 +$loop_count = 0 + +loop00: + +sleep 1000 + +$loop_count = $loop_count + 1 +if $loop_count == 20 then + return -1 +endi + +print 0 show cluster alive; +sql show cluster alive; + +print res------------------------ +print $data00 $data01 +print $data10 $data11 + +if $data00 != 1 then + print =====data00=$data00 + goto loop00 +endi + + print =============== create database, stable, table sql create database test vgroups 6; sql use test; @@ -46,6 +70,15 @@ endi print show cluster alive; sql show cluster alive; +print res------------------------ +print $data00 $data01 +print $data10 $data11 + +if $rows != 1 then + print =====rows=$rows + goto loop0 +endi + if $data00 != 1 then print =====data00=$data00 goto loop0 @@ -54,6 +87,15 @@ endi print show test.alive; sql show test.alive; +print res------------------------ +print $data00 $data01 +print $data10 $data11 + +if $rows != 1 then + print =====rows=$rows + goto loop0 +endi + if $data00 != 1 then print =====data00=$data00 goto loop0 @@ -164,4 +206,4 @@ endi system sh/exec.sh -n dnode1 -s stop -x SIGINT system sh/exec.sh -n dnode2 -s stop -x SIGINT system sh/exec.sh -n dnode3 -s stop -x SIGINT -system sh/exec.sh -n dnode4 -s stop -x SIGINT +system sh/exec.sh -n dnode4 -s stop -x SIGINT \ No newline at end of file From 5e77f6f6ca873e0b1aa308e5721a383fdec1f7d6 Mon Sep 17 00:00:00 2001 From: kailixu Date: Tue, 20 Aug 2024 10:18:14 +0800 Subject: [PATCH 16/28] fix: memory leak of geos --- source/util/src/tgeosctx.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/source/util/src/tgeosctx.c b/source/util/src/tgeosctx.c index 42cde5b8c7..99655ed7f7 100644 --- a/source/util/src/tgeosctx.c +++ b/source/util/src/tgeosctx.c @@ -43,15 +43,13 @@ SGeosContext *getThreadLocalGeosCtx() { } if (!sGeosPool.poolArray) { if (!(sGeosPool.poolArray = taosArrayInit(16, POINTER_BYTES))) { - taosMemoryFree(sGeosPool.pool); - sGeosPool.pool = NULL; + taosMemoryFreeClear(sGeosPool.pool); taosWUnLockLatch(&sGeosPool.lock); return NULL; } } if (!taosArrayPush(sGeosPool.poolArray, &sGeosPool.pool)) { - taosMemoryFree(sGeosPool.pool); - sGeosPool.pool = NULL; + taosMemoryFreeClear(sGeosPool.pool); taosWUnLockLatch(&sGeosPool.lock); return NULL; } From e15bd712740de4df8bf7e5778fbb69935e4a1859 Mon Sep 17 00:00:00 2001 From: 54liuyao <54liuyao> Date: Tue, 20 Aug 2024 10:38:00 +0800 Subject: [PATCH 17/28] fix issue --- source/util/src/tscalablebf.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/source/util/src/tscalablebf.c b/source/util/src/tscalablebf.c index 1d6ef29987..ffcfdfeaf1 100644 --- a/source/util/src/tscalablebf.c +++ b/source/util/src/tscalablebf.c @@ -87,6 +87,7 @@ int32_t tScalableBfPutNoCheck(SScalableBf* pSBf, const void* keyBuf, uint32_t le pSBf->status = SBF_INVALID; if (code == TSDB_CODE_OUT_OF_BUFFER) { code = TSDB_CODE_SUCCESS; + return code; } QUERY_CHECK_CODE(code, lino, _error); } @@ -126,6 +127,8 @@ int32_t tScalableBfPut(SScalableBf* pSBf, const void* keyBuf, uint32_t len, int3 pSBf->status = SBF_INVALID; if (code == TSDB_CODE_OUT_OF_BUFFER) { code = TSDB_CODE_SUCCESS; + (*winRes) = TSDB_CODE_FAILED; + goto _end; } QUERY_CHECK_CODE(code, lino, _end); } From 7073f204ac4594d7271efc6f8e82594bf2bbc8f9 Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Tue, 20 Aug 2024 11:00:02 +0800 Subject: [PATCH 18/28] refactor: do some internal refactor. --- source/dnode/vnode/src/tsdb/tsdbMergeTree.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source/dnode/vnode/src/tsdb/tsdbMergeTree.c b/source/dnode/vnode/src/tsdb/tsdbMergeTree.c index 160ff2e13c..2288e8bbce 100644 --- a/source/dnode/vnode/src/tsdb/tsdbMergeTree.c +++ b/source/dnode/vnode/src/tsdb/tsdbMergeTree.c @@ -903,6 +903,7 @@ int32_t tLDataIterNextRow(SLDataIter *pIter, const char *idStr, bool* hasNext) { pIter->rInfo.row = tsdbRowFromBlockData(pBlockData, pIter->iRow); _exit: + tsdbError("failed to exec stt-file nextIter, lino:%d, code:%s, %s", lino, tstrerror(code), idStr); *hasNext = (code == TSDB_CODE_SUCCESS) && (pIter->pSttBlk != NULL) && (pBlockData != NULL); return code; } From bbdd1f655b62698733e99f0f1188d9761c68b70f Mon Sep 17 00:00:00 2001 From: Haojun Liao Date: Tue, 20 Aug 2024 11:04:44 +0800 Subject: [PATCH 19/28] fix(stream): send kill checkpoint trans to mnode when trying to close vnode. --- source/dnode/vnode/src/tq/tq.c | 4 ++-- source/libs/stream/src/streamMeta.c | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/source/dnode/vnode/src/tq/tq.c b/source/dnode/vnode/src/tq/tq.c index a70a04f23d..5fc550da32 100644 --- a/source/dnode/vnode/src/tq/tq.c +++ b/source/dnode/vnode/src/tq/tq.c @@ -1204,11 +1204,11 @@ int32_t tqProcessTaskCheckPointSourceReq(STQ* pTq, SRpcMsg* pMsg, SRpcMsg* pRsp) } if (req.mndTrigger) { - qInfo("s-task:%s (vgId:%d) level:%d receive checkpoint-source msg chkpt:%" PRId64 ", transId:%d, ", pTask->id.idStr, + tqInfo("s-task:%s (vgId:%d) level:%d receive checkpoint-source msg chkpt:%" PRId64 ", transId:%d, ", pTask->id.idStr, vgId, pTask->info.taskLevel, req.checkpointId, req.transId); } else { const char* pPrevStatus = streamTaskGetStatusStr(streamTaskGetPrevStatus(pTask)); - qInfo("s-task:%s (vgId:%d) level:%d receive checkpoint-source msg chkpt:%" PRId64 + tqInfo("s-task:%s (vgId:%d) level:%d receive checkpoint-source msg chkpt:%" PRId64 ", transId:%d after transfer-state, prev status:%s", pTask->id.idStr, vgId, pTask->info.taskLevel, req.checkpointId, req.transId, pPrevStatus); } diff --git a/source/libs/stream/src/streamMeta.c b/source/libs/stream/src/streamMeta.c index a9976760b6..b6be1d04ca 100644 --- a/source/libs/stream/src/streamMeta.c +++ b/source/libs/stream/src/streamMeta.c @@ -1260,7 +1260,9 @@ void streamMetaUpdateStageRole(SStreamMeta* pMeta, int64_t stage, bool isLeader) pMeta->stage = stage; // mark the sign to send msg before close all tasks - if ((!isLeader) && (pMeta->role == NODE_ROLE_LEADER)) { + // 1. for leader vnode, always send msg before closing + // 2. for follower vnode, if it's is changed from leader, also sending msg before closing. + if (pMeta->role == NODE_ROLE_LEADER) { pMeta->sendMsgBeforeClosing = true; } From 6470e5f6e3b3a7318ca1fce15aed9685496949f9 Mon Sep 17 00:00:00 2001 From: Shengliang Guan Date: Tue, 20 Aug 2024 12:24:01 +0800 Subject: [PATCH 20/28] refact: refact function syncUtilNodeInfo2RaftId --- source/dnode/mnode/impl/src/mndSync.c | 2 +- source/libs/sync/src/syncMain.c | 52 +++++++++++---------------- source/libs/sync/src/syncUtil.c | 12 +++---- 3 files changed, 27 insertions(+), 39 deletions(-) diff --git a/source/dnode/mnode/impl/src/mndSync.c b/source/dnode/mnode/impl/src/mndSync.c index cf7769b932..3c5724dde3 100644 --- a/source/dnode/mnode/impl/src/mndSync.c +++ b/source/dnode/mnode/impl/src/mndSync.c @@ -486,7 +486,7 @@ int32_t mndInitSync(SMnode *pMnode) { int32_t code = 0; (void)tsem_init(&pMgmt->syncSem, 0, 0); - pMgmt->sync = syncOpen(&syncInfo, true); + pMgmt->sync = syncOpen(&syncInfo, 1); // always check if (pMgmt->sync <= 0) { if (terrno != 0) code = terrno; mError("failed to open sync since %s", tstrerror(code)); diff --git a/source/libs/sync/src/syncMain.c b/source/libs/sync/src/syncMain.c index 8b8a9e1279..ffd180ee01 100644 --- a/source/libs/sync/src/syncMain.c +++ b/source/libs/sync/src/syncMain.c @@ -1113,37 +1113,6 @@ SSyncNode* syncNodeOpen(SSyncInfo* pSyncInfo, int32_t vnodeVersion) { goto _error; } - // init internal - pSyncNode->myNodeInfo = pSyncNode->raftCfg.cfg.nodeInfo[pSyncNode->raftCfg.cfg.myIndex]; - if (!syncUtilNodeInfo2RaftId(&pSyncNode->myNodeInfo, pSyncNode->vgId, &pSyncNode->myRaftId)) { - terrno = TSDB_CODE_SYN_INTERNAL_ERROR; - sError("vgId:%d, failed to determine my raft member id", pSyncNode->vgId); - goto _error; - } - - pSyncNode->arbTerm = -1; - (void)taosThreadMutexInit(&pSyncNode->arbTokenMutex, NULL); - syncUtilGenerateArbToken(pSyncNode->myNodeInfo.nodeId, pSyncInfo->vgId, pSyncNode->arbToken); - sInfo("vgId:%d, arb token:%s", pSyncNode->vgId, pSyncNode->arbToken); - - // init peersNum, peers, peersId - pSyncNode->peersNum = pSyncNode->raftCfg.cfg.totalReplicaNum - 1; - int32_t j = 0; - for (int32_t i = 0; i < pSyncNode->raftCfg.cfg.totalReplicaNum; ++i) { - if (i != pSyncNode->raftCfg.cfg.myIndex) { - pSyncNode->peersNodeInfo[j] = pSyncNode->raftCfg.cfg.nodeInfo[i]; - syncUtilNodeInfo2EpSet(&pSyncNode->peersNodeInfo[j], &pSyncNode->peersEpset[j]); - j++; - } - } - for (int32_t i = 0; i < pSyncNode->peersNum; ++i) { - if (!syncUtilNodeInfo2RaftId(&pSyncNode->peersNodeInfo[i], pSyncNode->vgId, &pSyncNode->peersId[i])) { - terrno = TSDB_CODE_SYN_INTERNAL_ERROR; - sError("vgId:%d, failed to determine raft member id, peer:%d", pSyncNode->vgId, i); - goto _error; - } - } - // init replicaNum, replicasId pSyncNode->replicaNum = pSyncNode->raftCfg.cfg.replicaNum; pSyncNode->totalReplicaNum = pSyncNode->raftCfg.cfg.totalReplicaNum; @@ -1155,6 +1124,27 @@ SSyncNode* syncNodeOpen(SSyncInfo* pSyncInfo, int32_t vnodeVersion) { } } + // init internal + pSyncNode->myNodeInfo = pSyncNode->raftCfg.cfg.nodeInfo[pSyncNode->raftCfg.cfg.myIndex]; + pSyncNode->myRaftId = pSyncNode->replicasId[pSyncNode->raftCfg.cfg.myIndex]; + + // init peersNum, peers, peersId + pSyncNode->peersNum = pSyncNode->raftCfg.cfg.totalReplicaNum - 1; + int32_t j = 0; + for (int32_t i = 0; i < pSyncNode->raftCfg.cfg.totalReplicaNum; ++i) { + if (i != pSyncNode->raftCfg.cfg.myIndex) { + pSyncNode->peersNodeInfo[j] = pSyncNode->raftCfg.cfg.nodeInfo[i]; + pSyncNode->peersId[j] = pSyncNode->replicasId[i]; + syncUtilNodeInfo2EpSet(&pSyncNode->peersNodeInfo[j], &pSyncNode->peersEpset[j]); + j++; + } + } + + pSyncNode->arbTerm = -1; + (void)taosThreadMutexInit(&pSyncNode->arbTokenMutex, NULL); + syncUtilGenerateArbToken(pSyncNode->myNodeInfo.nodeId, pSyncInfo->vgId, pSyncNode->arbToken); + sInfo("vgId:%d, generate arb token:%s", pSyncNode->vgId, pSyncNode->arbToken); + // init raft algorithm pSyncNode->pFsm = pSyncInfo->pFsm; pSyncInfo->pFsm = NULL; diff --git a/source/libs/sync/src/syncUtil.c b/source/libs/sync/src/syncUtil.c index ca879f70d9..69abbcdea7 100644 --- a/source/libs/sync/src/syncUtil.c +++ b/source/libs/sync/src/syncUtil.c @@ -43,13 +43,11 @@ void syncUtilNodeInfo2EpSet(const SNodeInfo* pInfo, SEpSet* pEpSet) { bool syncUtilNodeInfo2RaftId(const SNodeInfo* pInfo, SyncGroupId vgId, SRaftId* raftId) { uint32_t ipv4 = 0xFFFFFFFF; - sDebug("vgId:%d, resolve sync addr from fqdn, dnode:%d cluster:%" PRId64 " fqdn:%s port:%u", vgId, pInfo->nodeId, - pInfo->clusterId, pInfo->nodeFqdn, pInfo->nodePort); + sDebug("vgId:%d, resolve sync addr from fqdn, ep:%s:%u", vgId, pInfo->nodeFqdn, pInfo->nodePort); for (int32_t i = 0; i < tsResolveFQDNRetryTime; i++) { int32_t code = taosGetIpv4FromFqdn(pInfo->nodeFqdn, &ipv4); if (code) { - sError("vgId:%d, failed to resolve sync addr, dnode:%d fqdn:%s, wait one second", vgId, pInfo->nodeId, - pInfo->nodeFqdn); + sError("vgId:%d, failed to resolve sync addr, dnode:%d fqdn:%s, retry", vgId, pInfo->nodeId, pInfo->nodeFqdn); taosSsleep(1); } else { break; @@ -57,7 +55,7 @@ bool syncUtilNodeInfo2RaftId(const SNodeInfo* pInfo, SyncGroupId vgId, SRaftId* } if (ipv4 == 0xFFFFFFFF || ipv4 == 1) { - sError("vgId:%d, failed to resolve sync addr, fqdn:%s", vgId, pInfo->nodeFqdn); + sError("vgId:%d, failed to resolve sync addr, dnode:%d fqdn:%s", vgId, pInfo->nodeId, pInfo->nodeFqdn); terrno = TSDB_CODE_TSC_INVALID_FQDN; return false; } @@ -67,8 +65,8 @@ bool syncUtilNodeInfo2RaftId(const SNodeInfo* pInfo, SyncGroupId vgId, SRaftId* raftId->addr = SYNC_ADDR(pInfo); raftId->vgId = vgId; - sInfo("vgId:%d, sync addr:%" PRIu64 " is resolved, dnode:%d cluster:%" PRId64 " fqdn:%s port:%u ip:%s ipv4:%u", vgId, - raftId->addr, pInfo->nodeId, pInfo->clusterId, pInfo->nodeFqdn, pInfo->nodePort, ipbuf, ipv4); + sInfo("vgId:%d, sync addr:%" PRIu64 " is resolved, ep:%s:%u ip:%s ipv4:%u dnode:%d cluster:%" PRId64, vgId, + raftId->addr, pInfo->nodeFqdn, pInfo->nodePort, ipbuf, ipv4, pInfo->nodeId, pInfo->clusterId); return true; } From 54dbf92517ccd514a9e0a3c29fc4b52e65d65f4e Mon Sep 17 00:00:00 2001 From: 54liuyao <54liuyao> Date: Tue, 20 Aug 2024 13:28:53 +0800 Subject: [PATCH 21/28] fix mem leak for max delay --- source/libs/executor/src/streamtimewindowoperator.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/source/libs/executor/src/streamtimewindowoperator.c b/source/libs/executor/src/streamtimewindowoperator.c index 5c12db1ab9..9147ff9b27 100644 --- a/source/libs/executor/src/streamtimewindowoperator.c +++ b/source/libs/executor/src/streamtimewindowoperator.c @@ -439,6 +439,11 @@ void destroyFlusedPos(void* pRes) { } } +void destroyFlusedppPos(void* ppRes) { + void *pRes = *(void **)ppRes; + destroyFlusedPos(pRes); +} + void clearGroupResInfo(SGroupResInfo* pGroupResInfo) { if (pGroupResInfo->freeItem) { int32_t size = taosArrayGetSize(pGroupResInfo->pRows); @@ -1920,6 +1925,7 @@ int32_t createStreamFinalIntervalOperatorInfo(SOperatorInfo* downstream, SPhysiN code = initAggSup(&pOperator->exprSupp, &pInfo->aggSup, pExprInfo, numOfCols, keyBufSize, pTaskInfo->id.str, pInfo->pState, &pTaskInfo->storageAPI.functionStore); QUERY_CHECK_CODE(code, lino, _error); + tSimpleHashSetFreeFp(pInfo->aggSup.pResultRowHashTable, destroyFlusedppPos); code = initExecTimeWindowInfo(&pInfo->twAggSup.timeWindowData, &pTaskInfo->window); QUERY_CHECK_CODE(code, lino, _error); @@ -5283,6 +5289,7 @@ int32_t createStreamIntervalOperatorInfo(SOperatorInfo* downstream, SPhysiNode* code = initAggSup(pSup, &pInfo->aggSup, pExprInfo, numOfCols, keyBufSize, pTaskInfo->id.str, pInfo->pState, &pTaskInfo->storageAPI.functionStore); QUERY_CHECK_CODE(code, lino, _error); + tSimpleHashSetFreeFp(pInfo->aggSup.pResultRowHashTable, destroyFlusedppPos); if (pIntervalPhyNode->window.pExprs != NULL) { int32_t numOfScalar = 0; From 7ac14bca97d841839bf299cdc994b512c149a110 Mon Sep 17 00:00:00 2001 From: kailixu Date: Tue, 20 Aug 2024 14:06:35 +0800 Subject: [PATCH 22/28] fix: memory leak of geos --- source/util/src/tgeosctx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source/util/src/tgeosctx.c b/source/util/src/tgeosctx.c index 99655ed7f7..82a360edd1 100644 --- a/source/util/src/tgeosctx.c +++ b/source/util/src/tgeosctx.c @@ -104,7 +104,8 @@ void taosGeosDestroy() { int32_t size = taosArrayGetSize(sGeosPool.poolArray); for (int32_t i = 0; i < size; ++i) { SGeosContext *pool = *(SGeosContext **)TARRAY_GET_ELEM(sGeosPool.poolArray, i); - for (int32_t j = 0; j < GEOS_POOL_CAPACITY; ++j) { + int32_t poolSize = i == size - 1 ? sGeosPool.size : GEOS_POOL_CAPACITY; + for (int32_t j = 0; j < poolSize; ++j) { destroyGeosCtx(pool + j); } taosMemoryFree(pool); From 7759e6aea41fd2cd57f27a1b5095d1e0b98c1c6e Mon Sep 17 00:00:00 2001 From: sima Date: Tue, 20 Aug 2024 14:12:30 +0800 Subject: [PATCH 23/28] fix:[TD-31558] Handle error of tDecodeSSchemaWrapper. --- source/dnode/vnode/src/meta/metaQuery.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/source/dnode/vnode/src/meta/metaQuery.c b/source/dnode/vnode/src/meta/metaQuery.c index bee4727260..3b10182c90 100644 --- a/source/dnode/vnode/src/meta/metaQuery.c +++ b/source/dnode/vnode/src/meta/metaQuery.c @@ -694,9 +694,13 @@ int32_t metaGetTbTSchemaEx(SMeta *pMeta, tb_uid_t suid, tb_uid_t uid, int32_t sv SSchemaWrapper *pSchemaWrapper = &schema; tDecoderInit(&dc, pData, nData); - (void)tDecodeSSchemaWrapper(&dc, pSchemaWrapper); + code = tDecodeSSchemaWrapper(&dc, pSchemaWrapper); tDecoderClear(&dc); tdbFree(pData); + if (TSDB_CODE_SUCCESS != code) { + taosMemoryFree(pSchemaWrapper->pSchema); + goto _exit; + } // convert STSchema *pTSchema = tBuildTSchema(pSchemaWrapper->pSchema, pSchemaWrapper->nCols, pSchemaWrapper->version); From b70ad8d3bb5cd2030051c8254c478754519444ed Mon Sep 17 00:00:00 2001 From: dapan1121 Date: Tue, 20 Aug 2024 14:13:06 +0800 Subject: [PATCH 24/28] fix: return code check issue --- source/libs/executor/src/dynqueryctrloperator.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/libs/executor/src/dynqueryctrloperator.c b/source/libs/executor/src/dynqueryctrloperator.c index 02932cd278..772222f9ed 100644 --- a/source/libs/executor/src/dynqueryctrloperator.c +++ b/source/libs/executor/src/dynqueryctrloperator.c @@ -320,7 +320,7 @@ static int32_t buildMergeJoinOperatorParam(SOperatorParam** ppRes, bool initPara return code; } (*ppRes)->pChildren = taosArrayInit(2, POINTER_BYTES); - if (NULL == *ppRes) { + if (NULL == (*ppRes)->pChildren) { code = terrno; freeOperatorParam(pChild0, OP_GET_PARAM); freeOperatorParam(pChild1, OP_GET_PARAM); From 557dbd883880fac40d815ec6b3f546b6596fdfbd Mon Sep 17 00:00:00 2001 From: dapan1121 Date: Tue, 20 Aug 2024 14:53:28 +0800 Subject: [PATCH 25/28] fix: data sink memory leak --- source/libs/executor/src/dataDispatcher.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/source/libs/executor/src/dataDispatcher.c b/source/libs/executor/src/dataDispatcher.c index 3964422411..f616cb05da 100644 --- a/source/libs/executor/src/dataDispatcher.c +++ b/source/libs/executor/src/dataDispatcher.c @@ -318,6 +318,7 @@ int32_t createDataDispatcher(SDataSinkManager* pManager, const SDataSinkNode* pD dispatcher->sink.fGetCacheSize = getCacheSize; dispatcher->pManager = pManager; + pManager = NULL; dispatcher->pSchema = pDataSink->pInputDataBlockDesc; dispatcher->status = DS_BUF_EMPTY; dispatcher->queryEnd = false; @@ -336,6 +337,9 @@ int32_t createDataDispatcher(SDataSinkManager* pManager, const SDataSinkNode* pD return TSDB_CODE_SUCCESS; _return: + + taosMemoryFree(pManager); + if (dispatcher) { dsDestroyDataSinker(dispatcher); } From 9900643657b94237c35aeab683c6e74861e8f48b Mon Sep 17 00:00:00 2001 From: sima Date: Tue, 20 Aug 2024 15:33:28 +0800 Subject: [PATCH 26/28] enh:[TD-31564] Remove ASSERT in client. --- source/client/src/clientImpl.c | 30 ++++++++++++++++++------------ source/client/src/clientMain.c | 27 ++++++++++++++------------- 2 files changed, 32 insertions(+), 25 deletions(-) diff --git a/source/client/src/clientImpl.c b/source/client/src/clientImpl.c index e12c761fcc..563d70ab08 100644 --- a/source/client/src/clientImpl.c +++ b/source/client/src/clientImpl.c @@ -166,7 +166,10 @@ int32_t taos_connect_internal(const char* ip, const char* user, const char* pass pInst = &p; } else { - ASSERTS((*pInst) && (*pInst)->pAppHbMgr, "*pInst:%p, pAppHgMgr:%p", *pInst, (*pInst) ? (*pInst)->pAppHbMgr : NULL); + if (NULL == *pInst || NULL == (*pInst)->pAppHbMgr) { + tscError("*pInst:%p, pAppHgMgr:%p", *pInst, (*pInst) ? (*pInst)->pAppHbMgr : NULL); + TSC_ERR_JRET(TSDB_CODE_TSC_INTERNAL_ERROR); + } // reset to 0 in case of conn with duplicated user key but its user has ever been dropped. atomic_store_8(&(*pInst)->pAppHbMgr->connHbFlag, 0); } @@ -2036,9 +2039,9 @@ static int32_t estimateJsonLen(SReqResultInfo* pResultInfo, int32_t numOfCols, i // | version | total length | total rows | total columns | flag seg| block group id | column schema | each column // length | int32_t cols = *(int32_t*)(p + sizeof(int32_t) * 3); - if (ASSERT(numOfCols == cols)) { + if (numOfCols != cols) { tscError("estimateJsonLen error: numOfCols:%d != cols:%d", numOfCols, cols); - return -1; + return TSDB_CODE_TSC_INTERNAL_ERROR; } int32_t len = getVersion1BlockMetaSize(p, numOfCols); @@ -2123,7 +2126,7 @@ static int32_t doConvertJson(SReqResultInfo* pResultInfo, int32_t numOfCols, int int32_t totalLen = 0; int32_t cols = *(int32_t*)(p + sizeof(int32_t) * 3); - if (ASSERT(numOfCols == cols)) { + if (numOfCols != cols) { tscError("doConvertJson error: numOfCols:%d != cols:%d", numOfCols, cols); return TSDB_CODE_TSC_INTERNAL_ERROR; } @@ -2148,7 +2151,7 @@ static int32_t doConvertJson(SReqResultInfo* pResultInfo, int32_t numOfCols, int for (int32_t i = 0; i < numOfCols; ++i) { int32_t colLen = (blockVersion == BLOCK_VERSION_1) ? htonl(colLength[i]) : colLength[i]; int32_t colLen1 = (blockVersion == BLOCK_VERSION_1) ? htonl(colLength1[i]) : colLength1[i]; - if (ASSERT(colLen < dataLen)) { + if (colLen >= dataLen) { tscError("doConvertJson error: colLen:%d >= dataLen:%d", colLen, dataLen); return TSDB_CODE_TSC_INTERNAL_ERROR; } @@ -2236,7 +2239,7 @@ static int32_t doConvertJson(SReqResultInfo* pResultInfo, int32_t numOfCols, int int32_t setResultDataPtr(SReqResultInfo* pResultInfo, TAOS_FIELD* pFields, int32_t numOfCols, int32_t numOfRows, bool convertUcs4) { - if (ASSERT(numOfCols > 0 && pFields != NULL && pResultInfo != NULL)) { + if (numOfCols <= 0 || pFields == NULL || pResultInfo == NULL) { tscError("setResultDataPtr paras error"); return TSDB_CODE_TSC_INTERNAL_ERROR; } @@ -2269,7 +2272,7 @@ int32_t setResultDataPtr(SReqResultInfo* pResultInfo, TAOS_FIELD* pFields, int32 int32_t cols = *(int32_t*)p; p += sizeof(int32_t); - if (ASSERT(rows == numOfRows && cols == numOfCols)) { + if (rows != numOfRows || cols != numOfCols) { tscError("setResultDataPtr paras error:rows;%d numOfRows:%d cols:%d numOfCols:%d", rows, numOfRows, cols, numOfCols); return TSDB_CODE_TSC_INTERNAL_ERROR; @@ -2288,8 +2291,6 @@ int32_t setResultDataPtr(SReqResultInfo* pResultInfo, TAOS_FIELD* pFields, int32 int32_t bytes = *(int32_t*)p; p += sizeof(int32_t); - - /*ASSERT(type == pFields[i].type && bytes == pFields[i].bytes);*/ } int32_t* colLength = (int32_t*)p; @@ -2411,18 +2412,23 @@ int32_t setQueryResultFromRsp(SReqResultInfo* pResultInfo, const SRetrieveTableR if (pRsp->compressed && compLen < rawLen) { int32_t len = tsDecompressString(pStart, compLen, 1, pResultInfo->decompBuf, rawLen, ONE_STAGE_COMP, NULL, 0); - ASSERT(len == rawLen); if (len < 0) { tscError("tsDecompressString failed"); return terrno ? terrno : TSDB_CODE_FAILED; } - + if (len != rawLen) { + tscError("tsDecompressString failed, len:%d != rawLen:%d", len, rawLen); + return TSDB_CODE_TSC_INTERNAL_ERROR; + } pResultInfo->pData = pResultInfo->decompBuf; pResultInfo->payloadLen = rawLen; } else { pResultInfo->pData = pStart; pResultInfo->payloadLen = htonl(pRsp->compLen); - ASSERT(pRsp->compLen == pRsp->payloadLen); + if (pRsp->compLen != pRsp->payloadLen) { + tscError("pRsp->compLen:%d != pRsp->payloadLen:%d", pRsp->compLen, pRsp->payloadLen); + return TSDB_CODE_TSC_INTERNAL_ERROR; + } } } diff --git a/source/client/src/clientMain.c b/source/client/src/clientMain.c index d007dae7f7..de56a4844a 100644 --- a/source/client/src/clientMain.c +++ b/source/client/src/clientMain.c @@ -477,7 +477,6 @@ TAOS_ROW taos_fetch_row(TAOS_RES *res) { } else if (TD_RES_TMQ_META(res) || TD_RES_TMQ_BATCH_META(res)) { return NULL; } else { - // assert to avoid un-initialization error tscError("invalid result passed to taos_fetch_row"); terrno = TSDB_CODE_TSC_INTERNAL_ERROR; return NULL; @@ -557,12 +556,14 @@ int taos_print_row(char *str, TAOS_ROW row, TAOS_FIELD *fields, int num_fields) case TSDB_DATA_TYPE_GEOMETRY: { int32_t charLen = varDataLen((char *)row[i] - VARSTR_HEADER_SIZE); if (fields[i].type == TSDB_DATA_TYPE_BINARY || fields[i].type == TSDB_DATA_TYPE_VARBINARY || fields[i].type == TSDB_DATA_TYPE_GEOMETRY) { - if (ASSERT(charLen <= fields[i].bytes && charLen >= 0)) { + if (charLen > fields[i].bytes || charLen < 0) { tscError("taos_print_row error binary. charLen:%d, fields[i].bytes:%d", charLen, fields[i].bytes); + break; } } else { - if (ASSERT(charLen <= fields[i].bytes * TSDB_NCHAR_SIZE && charLen >= 0)) { + if (charLen > fields[i].bytes * TSDB_NCHAR_SIZE || charLen < 0) { tscError("taos_print_row error. charLen:%d, fields[i].bytes:%d", charLen, fields[i].bytes); + break; } } @@ -1315,11 +1316,11 @@ void restartAsyncQuery(SRequestObj *pRequest, int32_t code) { } void taos_fetch_rows_a(TAOS_RES *res, __taos_async_fn_t fp, void *param) { - if (ASSERT(res != NULL && fp != NULL)) { + if (res == NULL || fp == NULL) { tscError("taos_fetch_rows_a invalid paras"); return; } - if (ASSERT(TD_RES_QUERY(res))) { + if (!TD_RES_QUERY(res)) { tscError("taos_fetch_rows_a res is NULL"); return; } @@ -1334,12 +1335,12 @@ void taos_fetch_rows_a(TAOS_RES *res, __taos_async_fn_t fp, void *param) { } void taos_fetch_raw_block_a(TAOS_RES *res, __taos_async_fn_t fp, void *param) { - if (ASSERT(res != NULL && fp != NULL)) { - tscError("taos_fetch_rows_a invalid paras"); + if (res == NULL || fp == NULL) { + tscError("taos_fetch_raw_block_a invalid paras"); return; } - if (ASSERT(TD_RES_QUERY(res))) { - tscError("taos_fetch_rows_a res is NULL"); + if (!TD_RES_QUERY(res)) { + tscError("taos_fetch_raw_block_a res is NULL"); return; } SRequestObj *pRequest = res; @@ -1353,12 +1354,12 @@ void taos_fetch_raw_block_a(TAOS_RES *res, __taos_async_fn_t fp, void *param) { } const void *taos_get_raw_block(TAOS_RES *res) { - if (ASSERT(res != NULL)) { - tscError("taos_fetch_rows_a invalid paras"); + if (res == NULL) { + tscError("taos_get_raw_block invalid paras"); return NULL; } - if (ASSERT(TD_RES_QUERY(res))) { - tscError("taos_fetch_rows_a res is NULL"); + if (!TD_RES_QUERY(res)) { + tscError("taos_get_raw_block res is NULL"); return NULL; } SRequestObj *pRequest = res; From 9f8f7591696558b4ba433e03c71ee33f477d3dff Mon Sep 17 00:00:00 2001 From: chenhaoran Date: Tue, 20 Aug 2024 21:10:14 +0800 Subject: [PATCH 27/28] tetst:update windows test case in full ci --- tests/script/win-test-file | 14 +++-- tests/system-test/win-test-file | 108 +++++++++++++++++++++++++++++++- 2 files changed, 117 insertions(+), 5 deletions(-) diff --git a/tests/script/win-test-file b/tests/script/win-test-file index acc4c74d21..8c96722c9f 100644 --- a/tests/script/win-test-file +++ b/tests/script/win-test-file @@ -1,3 +1,5 @@ +./test.sh -f tsim/query/timeline.sim +./test.sh -f tsim/join/join.sim ./test.sh -f tsim/tmq/basic2Of2ConsOverlap.sim ./test.sh -f tsim/parser/where.sim ./test.sh -f tsim/parser/join_manyblocks.sim @@ -150,6 +152,7 @@ ./test.sh -f tsim/parser/join_multivnode.sim ./test.sh -f tsim/parser/join.sim ./test.sh -f tsim/parser/last_cache.sim +./test.sh -f tsim/parser/last_both.sim ./test.sh -f tsim/parser/last_groupby.sim ./test.sh -f tsim/parser/lastrow.sim ./test.sh -f tsim/parser/lastrow2.sim @@ -192,6 +195,7 @@ ./test.sh -f tsim/query/session.sim ./test.sh -f tsim/query/join_interval.sim ./test.sh -f tsim/query/join_pk.sim +./test.sh -f tsim/query/join_order.sim ./test.sh -f tsim/query/count_spread.sim ./test.sh -f tsim/query/unionall_as_table.sim ./test.sh -f tsim/query/multi_order_by.sim @@ -214,6 +218,9 @@ ./test.sh -f tsim/query/bug3398.sim ./test.sh -f tsim/query/explain_tsorder.sim ./test.sh -f tsim/query/apercentile.sim +./test.sh -f tsim/query/query_count0.sim +./test.sh -f tsim/query/query_count_sliding0.sim +./test.sh -f tsim/query/union_precision.sim ./test.sh -f tsim/qnode/basic1.sim ./test.sh -f tsim/snode/basic1.sim ./test.sh -f tsim/mnode/basic1.sim @@ -287,9 +294,6 @@ ./test.sh -f tsim/sma/tsmaCreateInsertQuery.sim ./test.sh -f tsim/sma/rsmaCreateInsertQuery.sim ./test.sh -f tsim/sma/rsmaCreateInsertQueryDelete.sim - -### refactor stream backend, open case after rsma refactored -#./test.sh -f tsim/sma/rsmaPersistenceRecovery.sim ./test.sh -f tsim/sync/vnodesnapshot-rsma-test.sim ./test.sh -f tsim/valgrind/checkError1.sim ./test.sh -f tsim/valgrind/checkError2.sim @@ -326,6 +330,7 @@ ./test.sh -f tsim/compress/commitlog.sim ./test.sh -f tsim/compress/compress2.sim ./test.sh -f tsim/compress/compress.sim +./test.sh -f tsim/compress/compress_col.sim ./test.sh -f tsim/compress/uncompress.sim ./test.sh -f tsim/compute/avg.sim ./test.sh -f tsim/compute/block_dist.sim @@ -401,8 +406,9 @@ ./test.sh -f tsim/tag/tbNameIn.sim ./test.sh -f tmp/monitor.sim ./test.sh -f tsim/tagindex/add_index.sim -./test.sh -f tsim/tagindex/indexOverflow.sim ./test.sh -f tsim/tagindex/sma_and_tag_index.sim +./test.sh -f tsim/tagindex/indexOverflow.sim ./test.sh -f tsim/view/view.sim ./test.sh -f tsim/query/cache_last.sim ./test.sh -f tsim/query/const.sim +./test.sh -f tsim/query/nestedJoinView.sim diff --git a/tests/system-test/win-test-file b/tests/system-test/win-test-file index 69688e7450..41eb28e071 100644 --- a/tests/system-test/win-test-file +++ b/tests/system-test/win-test-file @@ -1,5 +1,13 @@ +python3 ./test.py -f 2-query/pk_error.py +python3 ./test.py -f 2-query/pk_func.py +python3 ./test.py -f 2-query/pk_varchar.py +python3 ./test.py -f 2-query/pk_func_group.py +python3 ./test.py -f 2-query/partition_expr.py +python3 ./test.py -f 2-query/project_group.py python3 ./test.py -f 2-query/tbname_vgroup.py +python3 ./test.py -f 2-query/count_interval.py python3 ./test.py -f 2-query/compact-col.py +python3 ./test.py -f 2-query/tms_memleak.py python3 ./test.py -f 2-query/stbJoin.py python3 ./test.py -f 2-query/stbJoin.py -Q 2 python3 ./test.py -f 2-query/stbJoin.py -Q 3 @@ -8,6 +16,8 @@ python3 ./test.py -f 2-query/hint.py python3 ./test.py -f 2-query/hint.py -Q 2 python3 ./test.py -f 2-query/hint.py -Q 3 python3 ./test.py -f 2-query/hint.py -Q 4 +python3 ./test.py -f 2-query/para_tms.py +python3 ./test.py -f 2-query/para_tms2.py python3 ./test.py -f 2-query/nestedQuery.py python3 ./test.py -f 2-query/nestedQuery_str.py python3 ./test.py -f 2-query/nestedQuery_math.py @@ -52,6 +62,20 @@ python3 ./test.py -f 2-query/last_cache_scan.py python3 ./test.py -f 2-query/last_cache_scan.py -Q 2 python3 ./test.py -f 2-query/last_cache_scan.py -Q 3 python3 ./test.py -f 2-query/last_cache_scan.py -Q 4 +python3 ./test.py -f 2-query/tbname.py +python3 ./test.py -f 2-query/tbname.py -Q 2 +python3 ./test.py -f 2-query/tbname.py -Q 3 +python3 ./test.py -f 2-query/tbname.py -Q 4 +python3 ./test.py -f 2-query/tsma.py +python3 ./test.py -f 2-query/tsma.py -R +python3 ./test.py -f 2-query/tsma.py -Q 2 +python3 ./test.py -f 2-query/tsma.py -Q 3 +python3 ./test.py -f 2-query/tsma.py -Q 4 +python3 ./test.py -f 2-query/tsma2.py +python3 ./test.py -f 2-query/tsma2.py -R +python3 ./test.py -f 2-query/tsma2.py -Q 2 +python3 ./test.py -f 2-query/tsma2.py -Q 3 +python3 ./test.py -f 2-query/tsma2.py -Q 4 python3 ./test.py -f 7-tmq/tmqShow.py python3 ./test.py -f 7-tmq/tmqDropStb.py python3 ./test.py -f 7-tmq/subscribeStb0.py @@ -67,11 +91,14 @@ python3 ./test.py -f 7-tmq/tmqClientConsLog.py python3 ./test.py -f 7-tmq/tmqMaxGroupIds.py python3 ./test.py -f 7-tmq/tmqConsumeDiscontinuousData.py python3 ./test.py -f 7-tmq/tmqOffset.py +python3 ./test.py -f 7-tmq/tmq_primary_key.py python3 ./test.py -f 7-tmq/tmqDropConsumer.py python3 ./test.py -f 1-insert/insert_stb.py python3 ./test.py -f 1-insert/delete_stable.py +python3 ./test.py -f 1-insert/stt_blocks_check.py python3 ./test.py -f 2-query/out_of_order.py -Q 3 python3 ./test.py -f 2-query/out_of_order.py +python3 ./test.py -f 2-query/agg_null.py python3 ./test.py -f 2-query/insert_null_none.py python3 ./test.py -f 2-query/insert_null_none.py -R python3 ./test.py -f 2-query/insert_null_none.py -Q 2 @@ -108,6 +135,18 @@ python3 ./test.py -f 2-query/match.py python3 ./test.py -f 2-query/match.py -Q 2 python3 ./test.py -f 2-query/match.py -Q 3 python3 ./test.py -f 2-query/match.py -Q 4 +python3 ./test.py -f 2-query/td-28068.py +python3 ./test.py -f 2-query/td-28068.py -Q 2 +python3 ./test.py -f 2-query/td-28068.py -Q 3 +python3 ./test.py -f 2-query/td-28068.py -Q 4 +python3 ./test.py -f 2-query/agg_group_AlwaysReturnValue.py +python3 ./test.py -f 2-query/agg_group_AlwaysReturnValue.py -Q 2 +python3 ./test.py -f 2-query/agg_group_AlwaysReturnValue.py -Q 3 +python3 ./test.py -f 2-query/agg_group_AlwaysReturnValue.py -Q 4 +python3 ./test.py -f 2-query/agg_group_NotReturnValue.py +python3 ./test.py -f 2-query/agg_group_NotReturnValue.py -Q 2 +python3 ./test.py -f 2-query/agg_group_NotReturnValue.py -Q 3 +python3 ./test.py -f 2-query/agg_group_NotReturnValue.py -Q 4 python3 ./test.py -f 3-enterprise/restore/restoreDnode.py -N 5 -M 3 -i False python3 ./test.py -f 3-enterprise/restore/restoreVnode.py -N 5 -M 3 -i False python3 ./test.py -f 3-enterprise/restore/restoreMnode.py -N 5 -M 3 -i False @@ -158,7 +197,8 @@ python3 ./test.py -f 7-tmq/tmqDropNtb-snapshot1.py python3 ./test.py -f 7-tmq/stbTagFilter-1ctb.py python3 ./test.py -f 7-tmq/dataFromTsdbNWal.py python3 ./test.py -f 7-tmq/dataFromTsdbNWal-multiCtb.py -# python3 ./test.py -f 7-tmq/tmq_taosx.py +python3 ./test.py -f 7-tmq/tmq_taosx.py +python3 ./test.py -f 7-tmq/tmq_ts4563.py python3 ./test.py -f 7-tmq/tmq_replay.py python3 ./test.py -f 7-tmq/tmqSeekAndCommit.py python3 ./test.py -f 7-tmq/tmq_offset.py @@ -168,15 +208,21 @@ python3 ./test.py -f 7-tmq/stbTagFilter-multiCtb.py python3 ./test.py -f 7-tmq/tmqSubscribeStb-r3.py -N 5 python3 ./test.py -f 7-tmq/tmq3mnodeSwitch.py -N 6 -M 3 -i True python3 ./test.py -f 7-tmq/tmq3mnodeSwitch.py -N 6 -M 3 -n 3 -i True +python3 test.py -f 7-tmq/tmqVnodeTransform-db-removewal.py -N 2 -n 1 +python3 test.py -f 7-tmq/tmqVnodeTransform-stb-removewal.py -N 6 -n 3 python3 test.py -f 7-tmq/tmqVnodeTransform-stb.py -N 2 -n 1 python3 test.py -f 7-tmq/tmqVnodeTransform-stb.py -N 6 -n 3 python3 test.py -f 7-tmq/tmqVnodeSplit-stb-select.py -N 2 -n 1 python3 test.py -f 7-tmq/tmqVnodeSplit-stb-select-duplicatedata.py -N 3 -n 3 python3 test.py -f 7-tmq/tmqVnodeSplit-stb-select-duplicatedata-false.py -N 3 -n 3 python3 test.py -f 7-tmq/tmqVnodeSplit-stb-select.py -N 3 -n 3 +python3 test.py -f 7-tmq/tmqVnodeSplit-stb-select-false.py -N 3 -n 3 python3 test.py -f 7-tmq/tmqVnodeSplit-stb.py -N 3 -n 3 +python3 test.py -f 7-tmq/tmqVnodeSplit-stb-false.py -N 3 -n 3 python3 test.py -f 7-tmq/tmqVnodeSplit-column.py -N 3 -n 3 +python3 test.py -f 7-tmq/tmqVnodeSplit-column-false.py -N 3 -n 3 python3 test.py -f 7-tmq/tmqVnodeSplit-db.py -N 3 -n 3 +python3 test.py -f 7-tmq/tmqVnodeSplit-db-false.py -N 3 -n 3 python3 test.py -f 7-tmq/tmqVnodeReplicate.py -M 3 -N 3 -n 3 python3 ./test.py -f 99-TDcase/TD-19201.py python3 ./test.py -f 99-TDcase/TD-21561.py @@ -184,6 +230,7 @@ python3 ./test.py -f 99-TDcase/TS-3404.py python3 ./test.py -f 99-TDcase/TS-3581.py python3 ./test.py -f 99-TDcase/TS-3311.py python3 ./test.py -f 99-TDcase/TS-3821.py +python3 ./test.py -f 99-TDcase/TS-5130.py python3 ./test.py -f 0-others/balance_vgroups_r1.py -N 6 python3 ./test.py -f 0-others/taosShell.py python3 ./test.py -f 0-others/taosShellError.py @@ -217,6 +264,10 @@ python3 ./test.py -f 0-others/splitVGroupWal.py -N 3 -n 3 python3 ./test.py -f 0-others/timeRangeWise.py -N 3 python3 ./test.py -f 0-others/delete_check.py python3 ./test.py -f 0-others/test_hot_refresh_configurations.py +python3 ./test.py -f 0-others/empty_identifier.py +python3 ./test.py -f 1-insert/composite_primary_key_create.py +python3 ./test.py -f 1-insert/composite_primary_key_insert.py +python3 ./test.py -f 1-insert/composite_primary_key_delete.py python3 ./test.py -f 1-insert/insert_double.py python3 ./test.py -f 1-insert/alter_database.py python3 ./test.py -f 1-insert/alter_replica.py -N 3 @@ -270,7 +321,10 @@ python3 ./test.py -f 1-insert/test_ts4219.py python3 ./test.py -f 1-insert/ts-4272.py python3 ./test.py -f 1-insert/test_ts4295.py python3 ./test.py -f 1-insert/test_td27388.py +python3 ./test.py -f 1-insert/test_ts4479.py +python3 ./test.py -f 1-insert/test_td29793.py python3 ./test.py -f 1-insert/insert_timestamp.py +python3 ./test.py -f 1-insert/test_td29157.py python3 ./test.py -f 0-others/show.py python3 ./test.py -f 0-others/show_tag_index.py python3 ./test.py -f 0-others/information_schema.py @@ -308,6 +362,11 @@ python3 ./test.py -f 2-query/concat_ws2.py python3 ./test.py -f 2-query/concat_ws2.py -R python3 ./test.py -f 2-query/cos.py python3 ./test.py -f 2-query/cos.py -R +python3 ./test.py -f 2-query/group_partition.py +python3 ./test.py -f 2-query/group_partition.py -R +python3 ./test.py -f 2-query/group_partition.py -Q 2 +python3 ./test.py -f 2-query/group_partition.py -Q 3 +python3 ./test.py -f 2-query/group_partition.py -Q 4 python3 ./test.py -f 2-query/count_partition.py python3 ./test.py -f 2-query/count_partition.py -R python3 ./test.py -f 2-query/count.py @@ -361,6 +420,40 @@ python3 ./test.py -f 2-query/last_row.py python3 ./test.py -f 2-query/last_row.py -R python3 ./test.py -f 2-query/last.py python3 ./test.py -f 2-query/last.py -R +python3 ./test.py -f 2-query/last_and_last_row.py +python3 ./test.py -f 2-query/last_and_last_row.py -R +python3 ./test.py -f 2-query/last_and_last_row.py -Q 2 +python3 ./test.py -f 2-query/last_and_last_row.py -Q 3 +python3 ./test.py -f 2-query/last_and_last_row.py -Q 4 +python3 ./test.py -f 2-query/last+last_row.py +python3 ./test.py -f 2-query/last+last_row.py -Q 2 +python3 ./test.py -f 2-query/last+last_row.py -Q 3 +python3 ./test.py -f 2-query/last+last_row.py -Q 4 +python3 ./test.py -f 2-query/primary_ts_base_1.py +python3 ./test.py -f 2-query/primary_ts_base_1.py -R +python3 ./test.py -f 2-query/primary_ts_base_1.py -Q 2 +python3 ./test.py -f 2-query/primary_ts_base_1.py -Q 3 +python3 ./test.py -f 2-query/primary_ts_base_1.py -Q 4 +python3 ./test.py -f 2-query/primary_ts_base_2.py +python3 ./test.py -f 2-query/primary_ts_base_2.py -R +python3 ./test.py -f 2-query/primary_ts_base_2.py -Q 2 +python3 ./test.py -f 2-query/primary_ts_base_2.py -Q 3 +python3 ./test.py -f 2-query/primary_ts_base_2.py -Q 4 +python3 ./test.py -f 2-query/primary_ts_base_3.py +python3 ./test.py -f 2-query/primary_ts_base_3.py -R +python3 ./test.py -f 2-query/primary_ts_base_3.py -Q 2 +python3 ./test.py -f 2-query/primary_ts_base_3.py -Q 3 +python3 ./test.py -f 2-query/primary_ts_base_3.py -Q 4 +python3 ./test.py -f 2-query/primary_ts_base_4.py +python3 ./test.py -f 2-query/primary_ts_base_4.py -R +python3 ./test.py -f 2-query/primary_ts_base_4.py -Q 2 +python3 ./test.py -f 2-query/primary_ts_base_4.py -Q 3 +python3 ./test.py -f 2-query/primary_ts_base_4.py -Q 4 +python3 ./test.py -f 2-query/primary_ts_base_5.py +python3 ./test.py -f 2-query/primary_ts_base_5.py -R +python3 ./test.py -f 2-query/primary_ts_base_5.py -Q 2 +python3 ./test.py -f 2-query/primary_ts_base_5.py -Q 3 +python3 ./test.py -f 2-query/primary_ts_base_5.py -Q 4 python3 ./test.py -f 2-query/leastsquares.py python3 ./test.py -f 2-query/leastsquares.py -R python3 ./test.py -f 2-query/length.py @@ -368,6 +461,8 @@ python3 ./test.py -f 2-query/length.py -R python3 ./test.py -f 2-query/limit.py python3 ./test.py -f 2-query/log.py python3 ./test.py -f 2-query/log.py -R +python3 ./test.py -f 2-query/logical_operators.py +python3 ./test.py -f 2-query/logical_operators.py -R python3 ./test.py -f 2-query/lower.py python3 ./test.py -f 2-query/lower.py -R python3 ./test.py -f 2-query/ltrim.py @@ -376,6 +471,8 @@ python3 ./test.py -f 2-query/mavg.py python3 ./test.py -f 2-query/mavg.py -R python3 ./test.py -f 2-query/max_partition.py python3 ./test.py -f 2-query/max_partition.py -R +python3 ./test.py -f 2-query/partition_limit_interval.py +python3 ./test.py -f 2-query/partition_limit_interval.py -R python3 ./test.py -f 2-query/max_min_last_interval.py python3 ./test.py -f 2-query/last_row_interval.py python3 ./test.py -f 2-query/max.py @@ -478,6 +575,8 @@ python3 ./test.py -f 2-query/json_tag.py python3 ./test.py -f 2-query/nestedQueryInterval.py python3 ./test.py -f 2-query/systable_func.py python3 ./test.py -f 2-query/test_ts4382.py +python3 ./test.py -f 2-query/test_ts4403.py +python3 ./test.py -f 2-query/test_td28163.py python3 ./test.py -f 2-query/stablity.py python3 ./test.py -f 2-query/stablity_1.py python3 ./test.py -f 2-query/elapsed.py @@ -486,6 +585,10 @@ python3 ./test.py -f 2-query/function_diff.py python3 ./test.py -f 2-query/tagFilter.py python3 ./test.py -f 2-query/projectionDesc.py python3 ./test.py -f 2-query/ts_3405_3398_3423.py -N 3 -n 3 +python3 ./test.py -f 2-query/ts-4348-td-27939.py +python3 ./test.py -f 2-query/backslash_g.py +python3 ./test.py -f 2-query/test_ts4467.py +python3 ./test.py -f 2-query/geometry.py python3 ./test.py -f 2-query/queryQnode.py python3 ./test.py -f 6-cluster/5dnode1mnode.py python3 ./test.py -f 6-cluster/5dnode2mnode.py -N 5 @@ -612,6 +715,7 @@ python3 ./test.py -f 2-query/irate.py -Q 2 python3 ./test.py -f 2-query/function_null.py -Q 2 python3 ./test.py -f 2-query/count_partition.py -Q 2 python3 ./test.py -f 2-query/max_partition.py -Q 2 +python3 ./test.py -f 2-query/partition_limit_interval.py -Q 2 python3 ./test.py -f 2-query/max_min_last_interval.py -Q 2 python3 ./test.py -f 2-query/last_row_interval.py -Q 2 python3 ./test.py -f 2-query/last_row.py -Q 2 @@ -707,6 +811,7 @@ python3 ./test.py -f 2-query/irate.py -Q 3 python3 ./test.py -f 2-query/function_null.py -Q 3 python3 ./test.py -f 2-query/count_partition.py -Q 3 python3 ./test.py -f 2-query/max_partition.py -Q 3 +python3 ./test.py -f 2-query/partition_limit_interval.py -Q 3 python3 ./test.py -f 2-query/max_min_last_interval.py -Q 3 python3 ./test.py -f 2-query/last_row_interval.py -Q 3 python3 ./test.py -f 2-query/last_row.py -Q 3 @@ -802,6 +907,7 @@ python3 ./test.py -f 2-query/irate.py -Q 4 python3 ./test.py -f 2-query/function_null.py -Q 4 python3 ./test.py -f 2-query/count_partition.py -Q 4 python3 ./test.py -f 2-query/max_partition.py -Q 4 +python3 ./test.py -f 2-query/partition_limit_interval.py -Q 4 python3 ./test.py -f 2-query/max_min_last_interval.py -Q 4 python3 ./test.py -f 2-query/last_row_interval.py -Q 4 python3 ./test.py -f 2-query/last_row.py -Q 4 From 87e56d3d67f394394759efd066e7925451267d4b Mon Sep 17 00:00:00 2001 From: Hongze Cheng Date: Wed, 21 Aug 2024 10:23:22 +0800 Subject: [PATCH 28/28] Revert "fix: memory leak of geos" --- include/util/tgeosctx.h | 8 +- source/client/src/clientMain.c | 2 - source/dnode/mgmt/node_mgmt/src/dmMgmt.c | 2 - source/libs/executor/src/sysscanoperator.c | 2 +- source/libs/geometry/src/geosWrapper.c | 28 +----- source/libs/parser/src/parInsertSql.c | 4 +- source/libs/scalar/src/sclvector.c | 8 +- source/util/src/tgeosctx.c | 111 +++++---------------- source/util/src/tsched.c | 2 + source/util/src/tworker.c | 2 + tools/shell/src/shellEngine.c | 5 +- 11 files changed, 45 insertions(+), 129 deletions(-) diff --git a/include/util/tgeosctx.h b/include/util/tgeosctx.h index a4355db29a..267ba9e049 100644 --- a/include/util/tgeosctx.h +++ b/include/util/tgeosctx.h @@ -32,16 +32,14 @@ typedef struct SGeosContext { GEOSWKBReader *WKBReader; GEOSWKBWriter *WKBWriter; - pcre2_code *WKTRegex; + pcre2_code *WKTRegex; pcre2_match_data *WKTMatchData; char errMsg[512]; } SGeosContext; -SGeosContext *acquireThreadLocalGeosCtx(); -SGeosContext *getThreadLocalGeosCtx(); -const char *getGeosErrMsg(int32_t code); -void taosGeosDestroy(); +SGeosContext* getThreadLocalGeosCtx(); +void destroyThreadLocalGeosCtx(); #ifdef __cplusplus } diff --git a/source/client/src/clientMain.c b/source/client/src/clientMain.c index c0eaf27077..d007dae7f7 100644 --- a/source/client/src/clientMain.c +++ b/source/client/src/clientMain.c @@ -23,7 +23,6 @@ #include "query.h" #include "scheduler.h" #include "tdatablock.h" -#include "tgeosctx.h" #include "tglobal.h" #include "tmsg.h" #include "tref.h" @@ -87,7 +86,6 @@ void taos_cleanup(void) { tscDebug("rpc cleanup"); taosConvDestroy(); - taosGeosDestroy(); tmqMgmtClose(); diff --git a/source/dnode/mgmt/node_mgmt/src/dmMgmt.c b/source/dnode/mgmt/node_mgmt/src/dmMgmt.c index ae6efa7af4..fdce9fd4c9 100644 --- a/source/dnode/mgmt/node_mgmt/src/dmMgmt.c +++ b/source/dnode/mgmt/node_mgmt/src/dmMgmt.c @@ -19,7 +19,6 @@ #include "index.h" #include "qworker.h" #include "tcompression.h" -#include "tgeosctx.h" #include "tglobal.h" #include "tgrant.h" #include "tstream.h" @@ -122,7 +121,6 @@ void dmCleanupDnode(SDnode *pDnode) { streamMetaCleanup(); indexCleanup(); taosConvDestroy(); - taosGeosDestroy(); // compress destroy tsCompressExit(); diff --git a/source/libs/executor/src/sysscanoperator.c b/source/libs/executor/src/sysscanoperator.c index 082d4e7789..d8a2331980 100644 --- a/source/libs/executor/src/sysscanoperator.c +++ b/source/libs/executor/src/sysscanoperator.c @@ -979,7 +979,7 @@ static int32_t sysTableGetGeomText(char* iGeom, int32_t nGeom, char** output, in if (TSDB_CODE_SUCCESS != (code = initCtxAsText()) || TSDB_CODE_SUCCESS != (code = doAsText(iGeom, nGeom, &outputWKT))) { - qError("geo text for systable failed:%s", getGeosErrMsg(code)); + qError("geo text for systable failed:%s", getThreadLocalGeosCtx()->errMsg); *output = NULL; *nOutput = 0; return code; diff --git a/source/libs/geometry/src/geosWrapper.c b/source/libs/geometry/src/geosWrapper.c index 7372521276..dde34edc91 100644 --- a/source/libs/geometry/src/geosWrapper.c +++ b/source/libs/geometry/src/geosWrapper.c @@ -23,8 +23,7 @@ typedef char (*_geosPreparedRelationFunc_t)(GEOSContextHandle_t handle, const GE void geosFreeBuffer(void *buffer) { if (buffer) { - SGeosContext *pCtx = acquireThreadLocalGeosCtx(); - if (pCtx) GEOSFree_r(pCtx->handle, buffer); + GEOSFree_r(getThreadLocalGeosCtx()->handle, buffer); } } @@ -37,8 +36,6 @@ int32_t initCtxMakePoint() { int32_t code = TSDB_CODE_FAILED; SGeosContext *geosCtx = getThreadLocalGeosCtx(); - if (!geosCtx) return TSDB_CODE_OUT_OF_MEMORY; - if (geosCtx->handle == NULL) { geosCtx->handle = GEOS_init_r(); if (geosCtx->handle == NULL) { @@ -64,8 +61,6 @@ int32_t doMakePoint(double x, double y, unsigned char **outputGeom, size_t *size int32_t code = TSDB_CODE_FAILED; SGeosContext *geosCtx = getThreadLocalGeosCtx(); - if (!geosCtx) return TSDB_CODE_OUT_OF_MEMORY; - GEOSGeometry *geom = NULL; unsigned char *wkb = NULL; @@ -171,8 +166,6 @@ int32_t initCtxGeomFromText() { int32_t code = TSDB_CODE_FAILED; SGeosContext *geosCtx = getThreadLocalGeosCtx(); - if (!geosCtx) return TSDB_CODE_OUT_OF_MEMORY; - if (geosCtx->handle == NULL) { geosCtx->handle = GEOS_init_r(); if (geosCtx->handle == NULL) { @@ -209,8 +202,6 @@ int32_t doGeomFromText(const char *inputWKT, unsigned char **outputGeom, size_t int32_t code = TSDB_CODE_FAILED; SGeosContext *geosCtx = getThreadLocalGeosCtx(); - if (!geosCtx) return TSDB_CODE_OUT_OF_MEMORY; - GEOSGeometry *geom = NULL; unsigned char *wkb = NULL; @@ -246,8 +237,6 @@ int32_t initCtxAsText() { int32_t code = TSDB_CODE_FAILED; SGeosContext *geosCtx = getThreadLocalGeosCtx(); - if (!geosCtx) return TSDB_CODE_OUT_OF_MEMORY; - if (geosCtx->handle == NULL) { geosCtx->handle = GEOS_init_r(); if (geosCtx->handle == NULL) { @@ -284,8 +273,6 @@ int32_t doAsText(const unsigned char *inputGeom, size_t size, char **outputWKT) int32_t code = TSDB_CODE_FAILED; SGeosContext *geosCtx = getThreadLocalGeosCtx(); - if (!geosCtx) return TSDB_CODE_OUT_OF_MEMORY; - GEOSGeometry *geom = NULL; char *wkt = NULL; @@ -317,8 +304,6 @@ int32_t initCtxRelationFunc() { int32_t code = TSDB_CODE_FAILED; SGeosContext *geosCtx = getThreadLocalGeosCtx(); - if (!geosCtx) return TSDB_CODE_OUT_OF_MEMORY; - if (geosCtx->handle == NULL) { geosCtx->handle = GEOS_init_r(); if (geosCtx->handle == NULL) { @@ -344,8 +329,6 @@ int32_t doGeosRelation(const GEOSGeometry *geom1, const GEOSPreparedGeometry *pr _geosPreparedRelationFunc_t swappedPreparedRelationFn) { SGeosContext *geosCtx = getThreadLocalGeosCtx(); - if (!geosCtx) return TSDB_CODE_OUT_OF_MEMORY; - if (!preparedGeom1) { if (!swapped) { ASSERT(relationFn); @@ -406,6 +389,8 @@ int32_t doContainsProperly(const GEOSGeometry *geom1, const GEOSPreparedGeometry // need to call destroyGeometry(outputGeom, outputPreparedGeom) later int32_t readGeometry(const unsigned char *input, GEOSGeometry **outputGeom, const GEOSPreparedGeometry **outputPreparedGeom) { + SGeosContext *geosCtx = getThreadLocalGeosCtx(); + ASSERT(outputGeom); // it is not allowed if outputGeom is NULL *outputGeom = NULL; @@ -417,10 +402,6 @@ int32_t readGeometry(const unsigned char *input, GEOSGeometry **outputGeom, return TSDB_CODE_SUCCESS; } - SGeosContext *geosCtx = getThreadLocalGeosCtx(); - - if (!geosCtx) return TSDB_CODE_OUT_OF_MEMORY; - *outputGeom = GEOSWKBReader_read_r(geosCtx->handle, geosCtx->WKBReader, varDataVal(input), varDataLen(input)); if (*outputGeom == NULL) { return TSDB_CODE_FUNC_FUNTION_PARA_VALUE; @@ -437,8 +418,7 @@ int32_t readGeometry(const unsigned char *input, GEOSGeometry **outputGeom, } void destroyGeometry(GEOSGeometry **geom, const GEOSPreparedGeometry **preparedGeom) { - SGeosContext *geosCtx = acquireThreadLocalGeosCtx(); - if (!geosCtx) return; + SGeosContext *geosCtx = getThreadLocalGeosCtx(); if (preparedGeom && *preparedGeom) { GEOSPreparedGeom_destroy_r(geosCtx->handle, *preparedGeom); diff --git a/source/libs/parser/src/parInsertSql.c b/source/libs/parser/src/parInsertSql.c index aa6116287e..cb94cd42f7 100644 --- a/source/libs/parser/src/parInsertSql.c +++ b/source/libs/parser/src/parInsertSql.c @@ -655,7 +655,7 @@ static int32_t parseTagToken(const char** end, SToken* pToken, SSchema* pSchema, code = parseGeometry(pToken, &output, &size); if (code != TSDB_CODE_SUCCESS) { - code = buildSyntaxErrMsg(pMsgBuf, getGeosErrMsg(code), pToken->z); + code = buildSyntaxErrMsg(pMsgBuf, getThreadLocalGeosCtx()->errMsg, pToken->z); } else if (size + VARSTR_HEADER_SIZE > pSchema->bytes) { // Too long values will raise the invalid sql error message code = generateSyntaxErrMsg(pMsgBuf, TSDB_CODE_PAR_VALUE_TOO_LONG, pSchema->name); @@ -1646,7 +1646,7 @@ static int32_t parseValueTokenImpl(SInsertParseContext* pCxt, const char** pSql, code = parseGeometry(pToken, &output, &size); if (code != TSDB_CODE_SUCCESS) { - code = buildSyntaxErrMsg(&pCxt->msg, getGeosErrMsg(code), pToken->z); + code = buildSyntaxErrMsg(&pCxt->msg, getThreadLocalGeosCtx()->errMsg, pToken->z); } // Too long values will raise the invalid sql error message else if (size + VARSTR_HEADER_SIZE > pSchema->bytes) { diff --git a/source/libs/scalar/src/sclvector.c b/source/libs/scalar/src/sclvector.c index bc8a2ae233..5556108a52 100644 --- a/source/libs/scalar/src/sclvector.c +++ b/source/libs/scalar/src/sclvector.c @@ -446,12 +446,12 @@ static FORCE_INLINE int32_t varToGeometry(char *buf, SScalarParam *pOut, int32_t unsigned char *t = NULL; char *output = NULL; - if ((code = initCtxGeomFromText()) != 0) { - sclError("failed to init geometry ctx, %s", getGeosErrMsg(code)); + if (initCtxGeomFromText()) { + sclError("failed to init geometry ctx, %s", getThreadLocalGeosCtx()->errMsg); SCL_ERR_JRET(TSDB_CODE_APP_ERROR); } - if ((code = doGeomFromText(buf, &t, &len)) != 0) { - sclError("failed to convert text to geometry, %s", getGeosErrMsg(code)); + if (doGeomFromText(buf, &t, &len)) { + sclInfo("failed to convert text to geometry, %s", getThreadLocalGeosCtx()->errMsg); SCL_ERR_JRET(TSDB_CODE_SCALAR_CONVERT_ERROR); } diff --git a/source/util/src/tgeosctx.c b/source/util/src/tgeosctx.c index 82a360edd1..a05734c911 100644 --- a/source/util/src/tgeosctx.c +++ b/source/util/src/tgeosctx.c @@ -14,102 +14,39 @@ */ #include "tgeosctx.h" -#include "tarray.h" #include "tdef.h" -#include "tlockfree.h" -#include "tlog.h" -#define GEOS_POOL_CAPACITY 64 -typedef struct { - SArray *poolArray; // totalSize: (GEOS_POOL_CAPACITY * (taosArrayGetSize(poolArray) - 1)) + size - SGeosContext *pool; // current SGeosContext pool - int32_t size; // size of current SGeosContext pool, size <= GEOS_POOL_CAPACITY - SRWLatch lock; -} SGeosContextPool; +static threadlocal SGeosContext tlGeosCtx = {0}; -static SGeosContextPool sGeosPool = {0}; -static threadlocal SGeosContext *tlGeosCtx = NULL; +SGeosContext* getThreadLocalGeosCtx() { return &tlGeosCtx; } -SGeosContext *acquireThreadLocalGeosCtx() { return tlGeosCtx; } - -SGeosContext *getThreadLocalGeosCtx() { - if (tlGeosCtx) return tlGeosCtx; - - taosWLockLatch(&sGeosPool.lock); - if (!sGeosPool.pool || sGeosPool.size >= GEOS_POOL_CAPACITY) { - if (!(sGeosPool.pool = (SGeosContext *)taosMemoryCalloc(GEOS_POOL_CAPACITY, sizeof(SGeosContext)))) { - taosWUnLockLatch(&sGeosPool.lock); - return NULL; - } - if (!sGeosPool.poolArray) { - if (!(sGeosPool.poolArray = taosArrayInit(16, POINTER_BYTES))) { - taosMemoryFreeClear(sGeosPool.pool); - taosWUnLockLatch(&sGeosPool.lock); - return NULL; - } - } - if (!taosArrayPush(sGeosPool.poolArray, &sGeosPool.pool)) { - taosMemoryFreeClear(sGeosPool.pool); - taosWUnLockLatch(&sGeosPool.lock); - return NULL; - } - sGeosPool.size = 0; +void destroyThreadLocalGeosCtx() { + if (tlGeosCtx.WKTReader) { + GEOSWKTReader_destroy_r(tlGeosCtx.handle, tlGeosCtx.WKTReader); + tlGeosCtx.WKTReader = NULL; } - tlGeosCtx = sGeosPool.pool + sGeosPool.size; - ++sGeosPool.size; - taosWUnLockLatch(&sGeosPool.lock); - return tlGeosCtx; -} + if (tlGeosCtx.WKTWriter) { + GEOSWKTWriter_destroy_r(tlGeosCtx.handle, tlGeosCtx.WKTWriter); + tlGeosCtx.WKTWriter = NULL; + } -const char *getGeosErrMsg(int32_t code) { return tlGeosCtx ? tlGeosCtx->errMsg : (code != 0 ? tstrerror(code) : ""); } + if (tlGeosCtx.WKBReader) { + GEOSWKBReader_destroy_r(tlGeosCtx.handle, tlGeosCtx.WKBReader); + tlGeosCtx.WKBReader = NULL; + } -static void destroyGeosCtx(SGeosContext *pCtx) { - if (pCtx) { - if (pCtx->WKTReader) { - GEOSWKTReader_destroy_r(pCtx->handle, pCtx->WKTReader); - pCtx->WKTReader = NULL; - } + if (tlGeosCtx.WKBWriter) { + GEOSWKBWriter_destroy_r(tlGeosCtx.handle, tlGeosCtx.WKBWriter); + tlGeosCtx.WKBWriter = NULL; + } - if (pCtx->WKTWriter) { - GEOSWKTWriter_destroy_r(pCtx->handle, pCtx->WKTWriter); - pCtx->WKTWriter = NULL; - } + if (tlGeosCtx.WKTRegex) { + destroyRegexes(tlGeosCtx.WKTRegex, tlGeosCtx.WKTMatchData); + } - if (pCtx->WKBReader) { - GEOSWKBReader_destroy_r(pCtx->handle, pCtx->WKBReader); - pCtx->WKBReader = NULL; - } - - if (pCtx->WKBWriter) { - GEOSWKBWriter_destroy_r(pCtx->handle, pCtx->WKBWriter); - pCtx->WKBWriter = NULL; - } - - if (pCtx->WKTRegex) { - destroyRegexes(pCtx->WKTRegex, pCtx->WKTMatchData); - pCtx->WKTRegex = NULL; - pCtx->WKTMatchData = NULL; - } - - if (pCtx->handle) { - GEOS_finish_r(pCtx->handle); - pCtx->handle = NULL; - } + if (tlGeosCtx.handle) { + GEOS_finish_r(tlGeosCtx.handle); + tlGeosCtx.handle = NULL; } } - -void taosGeosDestroy() { - uInfo("geos is cleaned up"); - int32_t size = taosArrayGetSize(sGeosPool.poolArray); - for (int32_t i = 0; i < size; ++i) { - SGeosContext *pool = *(SGeosContext **)TARRAY_GET_ELEM(sGeosPool.poolArray, i); - int32_t poolSize = i == size - 1 ? sGeosPool.size : GEOS_POOL_CAPACITY; - for (int32_t j = 0; j < poolSize; ++j) { - destroyGeosCtx(pool + j); - } - taosMemoryFree(pool); - } - taosArrayDestroy(sGeosPool.poolArray); - sGeosPool.poolArray = NULL; -} \ No newline at end of file diff --git a/source/util/src/tsched.c b/source/util/src/tsched.c index 55a927f340..6779e8dee5 100644 --- a/source/util/src/tsched.c +++ b/source/util/src/tsched.c @@ -178,6 +178,8 @@ void *taosProcessSchedQueue(void *scheduler) { (*(msg.tfp))(msg.ahandle, msg.thandle); } + destroyThreadLocalGeosCtx(); + return NULL; } diff --git a/source/util/src/tworker.c b/source/util/src/tworker.c index ebec134c91..b2064d6787 100644 --- a/source/util/src/tworker.c +++ b/source/util/src/tworker.c @@ -105,6 +105,7 @@ static void *tQWorkerThreadFp(SQueueWorker *worker) { taosUpdateItemSize(qinfo.queue, 1); } + destroyThreadLocalGeosCtx(); DestoryThreadLocalRegComp(); return NULL; @@ -664,6 +665,7 @@ static void *tQueryAutoQWorkerThreadFp(SQueryAutoQWorker *worker) { } } + destroyThreadLocalGeosCtx(); DestoryThreadLocalRegComp(); return NULL; diff --git a/tools/shell/src/shellEngine.c b/tools/shell/src/shellEngine.c index 2c8330c433..0ccbd683dc 100644 --- a/tools/shell/src/shellEngine.c +++ b/tools/shell/src/shellEngine.c @@ -611,14 +611,14 @@ void shellPrintGeometry(const unsigned char *val, int32_t length, int32_t width) code = initCtxAsText(); if (code != TSDB_CODE_SUCCESS) { - shellPrintString(getGeosErrMsg(code), width); + shellPrintString(getThreadLocalGeosCtx()->errMsg, width); return; } char *outputWKT = NULL; code = doAsText(val, length, &outputWKT); if (code != TSDB_CODE_SUCCESS) { - shellPrintString(getGeosErrMsg(code), width); // should NOT happen + shellPrintString(getThreadLocalGeosCtx()->errMsg, width); // should NOT happen return; } @@ -1282,6 +1282,7 @@ void *shellThreadLoop(void *arg) { taosResetTerminalMode(); } while (shellRunCommand(command, true) == 0); + destroyThreadLocalGeosCtx(); taosMemoryFreeClear(command); shellWriteHistory(); shellExit();