From 22a2a6602858803cd32553e971fecc3052766ece Mon Sep 17 00:00:00 2001 From: Hongze Cheng Date: Thu, 5 Aug 2021 14:29:15 +0800 Subject: [PATCH 1/6] [TD-5812] : fix possible coredump change schema --- src/tsdb/inc/tsdbMeta.h | 8 ++--- src/tsdb/src/tsdbMeta.c | 77 ++++++++++++++++++++++++++++------------- 2 files changed, 55 insertions(+), 30 deletions(-) diff --git a/src/tsdb/inc/tsdbMeta.h b/src/tsdb/inc/tsdbMeta.h index 9a8de01f71..29d2b6595d 100644 --- a/src/tsdb/inc/tsdbMeta.h +++ b/src/tsdb/inc/tsdbMeta.h @@ -24,8 +24,7 @@ typedef struct STable { tstr* name; // NOTE: there a flexible string here uint64_t suid; struct STable* pSuper; // super table pointer - uint8_t numOfSchemas; - STSchema* schema[TSDB_MAX_TABLE_SCHEMAS]; + SArray* schema; STSchema* tagSchema; SKVRow tagVal; SSkipList* pIndex; // For TSDB_SUPER_TABLE, it is the skiplist index @@ -107,10 +106,9 @@ static FORCE_INLINE STSchema* tsdbGetTableSchemaImpl(STable* pTable, bool lock, if (lock) TSDB_RLOCK_TABLE(pDTable); if (_version < 0) { // get the latest version of schema - pTSchema = pDTable->schema[pDTable->numOfSchemas - 1]; + pTSchema = *(STSchema **)taosArrayGetLast(pTable->schema); } else { // get the schema with version - void* ptr = taosbsearch(&_version, pDTable->schema, pDTable->numOfSchemas, sizeof(STSchema*), - tsdbCompareSchemaVersion, TD_EQ); + void* ptr = taosArraySearch(pTable->schema, &_version, tsdbCompareSchemaVersion, TD_EQ); if (ptr == NULL) { terrno = TSDB_CODE_TDB_IVD_TB_SCHEMA_VERSION; goto _exit; diff --git a/src/tsdb/src/tsdbMeta.c b/src/tsdb/src/tsdbMeta.c index 619b32b3d9..3fb5739641 100644 --- a/src/tsdb/src/tsdbMeta.c +++ b/src/tsdb/src/tsdbMeta.c @@ -44,6 +44,8 @@ static int tsdbRemoveTableFromStore(STsdbRepo *pRepo, STable *pTable); static int tsdbRmTableFromMeta(STsdbRepo *pRepo, STable *pTable); static int tsdbAdjustMetaTables(STsdbRepo *pRepo, int tid); static int tsdbCheckTableTagVal(SKVRow *pKVRow, STSchema *pSchema); +static int tsdbAddSchema(STable *pTable, STSchema *pSchema); +static void tsdbFreeTableSchema(STable *pTable); // ------------------ OUTER FUNCTIONS ------------------ int tsdbCreateTable(STsdbRepo *repo, STableCfg *pCfg) { @@ -723,17 +725,10 @@ void tsdbUpdateTableSchema(STsdbRepo *pRepo, STable *pTable, STSchema *pSchema, STsdbMeta *pMeta = pRepo->tsdbMeta; STable *pCTable = (TABLE_TYPE(pTable) == TSDB_CHILD_TABLE) ? pTable->pSuper : pTable; - ASSERT(schemaVersion(pSchema) > schemaVersion(pCTable->schema[pCTable->numOfSchemas - 1])); + ASSERT(schemaVersion(pSchema) > schemaVersion(*(STSchema **)taosArrayGetLast(pTable->schema))); TSDB_WLOCK_TABLE(pCTable); - if (pCTable->numOfSchemas < TSDB_MAX_TABLE_SCHEMAS) { - pCTable->schema[pCTable->numOfSchemas++] = pSchema; - } else { - ASSERT(pCTable->numOfSchemas == TSDB_MAX_TABLE_SCHEMAS); - tdFreeSchema(pCTable->schema[0]); - memmove(pCTable->schema, pCTable->schema + 1, sizeof(STSchema *) * (TSDB_MAX_TABLE_SCHEMAS - 1)); - pCTable->schema[pCTable->numOfSchemas - 1] = pSchema; - } + tsdbAddSchema(pCTable, pSchema); if (schemaNCols(pSchema) > pMeta->maxCols) pMeta->maxCols = schemaNCols(pSchema); if (schemaTLen(pSchema) > pMeta->maxRowBytes) pMeta->maxRowBytes = schemaTLen(pSchema); @@ -829,9 +824,7 @@ static STable *tsdbCreateTableFromCfg(STableCfg *pCfg, bool isSuper, STable *pST TABLE_TID(pTable) = -1; TABLE_SUID(pTable) = -1; pTable->pSuper = NULL; - pTable->numOfSchemas = 1; - pTable->schema[0] = tdDupSchema(pCfg->schema); - if (pTable->schema[0] == NULL) { + if (tsdbAddSchema(pTable, tdDupSchema(pCfg->schema)) < 0) { terrno = TSDB_CODE_TDB_OUT_OF_MEMORY; goto _err; } @@ -842,7 +835,8 @@ static STable *tsdbCreateTableFromCfg(STableCfg *pCfg, bool isSuper, STable *pST } pTable->tagVal = NULL; STColumn *pCol = schemaColAt(pTable->tagSchema, DEFAULT_TAG_INDEX_COLUMN); - pTable->pIndex = tSkipListCreate(TSDB_SUPER_TABLE_SL_LEVEL, colType(pCol), (uint8_t)(colBytes(pCol)), NULL, SL_ALLOW_DUP_KEY, getTagIndexKey); + pTable->pIndex = tSkipListCreate(TSDB_SUPER_TABLE_SL_LEVEL, colType(pCol), (uint8_t)(colBytes(pCol)), NULL, + SL_ALLOW_DUP_KEY, getTagIndexKey); if (pTable->pIndex == NULL) { terrno = TSDB_CODE_TDB_OUT_OF_MEMORY; goto _err; @@ -871,9 +865,7 @@ static STable *tsdbCreateTableFromCfg(STableCfg *pCfg, bool isSuper, STable *pST } } else { TABLE_SUID(pTable) = -1; - pTable->numOfSchemas = 1; - pTable->schema[0] = tdDupSchema(pCfg->schema); - if (pTable->schema[0] == NULL) { + if (tsdbAddSchema(pTable, tdDupSchema(pCfg->schema)) < 0) { terrno = TSDB_CODE_TDB_OUT_OF_MEMORY; goto _err; } @@ -907,9 +899,7 @@ static void tsdbFreeTable(STable *pTable) { TABLE_UID(pTable)); tfree(TABLE_NAME(pTable)); if (TABLE_TYPE(pTable) != TSDB_CHILD_TABLE) { - for (int i = 0; i < TSDB_MAX_TABLE_SCHEMAS; i++) { - tdFreeSchema(pTable->schema[i]); - } + tsdbFreeTableSchema(pTable); if (TABLE_TYPE(pTable) == TSDB_SUPER_TABLE) { tdFreeSchema(pTable->tagSchema); @@ -1261,9 +1251,10 @@ static int tsdbEncodeTable(void **buf, STable *pTable) { tlen += taosEncodeFixedU64(buf, TABLE_SUID(pTable)); tlen += tdEncodeKVRow(buf, pTable->tagVal); } else { - tlen += taosEncodeFixedU8(buf, pTable->numOfSchemas); - for (int i = 0; i < pTable->numOfSchemas; i++) { - tlen += tdEncodeSchema(buf, pTable->schema[i]); + tlen += taosEncodeFixedU8(buf, taosArrayGetSize(pTable->schema)); + for (int i = 0; i < taosArrayGetSize(pTable->schema); i++) { + STSchema *pSchema = taosArrayGetP(pTable->schema, i); + tlen += tdEncodeSchema(buf, pSchema); } if (TABLE_TYPE(pTable) == TSDB_SUPER_TABLE) { @@ -1294,9 +1285,12 @@ static void *tsdbDecodeTable(void *buf, STable **pRTable) { buf = taosDecodeFixedU64(buf, &TABLE_SUID(pTable)); buf = tdDecodeKVRow(buf, &(pTable->tagVal)); } else { - buf = taosDecodeFixedU8(buf, &(pTable->numOfSchemas)); - for (int i = 0; i < pTable->numOfSchemas; i++) { - buf = tdDecodeSchema(buf, &(pTable->schema[i])); + uint8_t nSchemas; + buf = taosDecodeFixedU8(buf, &nSchemas); + for (int i = 0; i < nSchemas; i++) { + STSchema *pSchema; + buf = tdDecodeSchema(buf, &pSchema); + tsdbAddSchema(pTable, pSchema); } if (TABLE_TYPE(pTable) == TSDB_SUPER_TABLE) { @@ -1458,3 +1452,36 @@ static int tsdbCheckTableTagVal(SKVRow *pKVRow, STSchema *pSchema) { return 0; } + +static int tsdbAddSchema(STable *pTable, STSchema *pSchema) { + ASSERT(TABLE_TYPE(pTable) != TSDB_CHILD_TABLE); + + if (pTable->schema == NULL) { + pTable->schema = taosArrayInit(TSDB_MAX_TABLE_SCHEMAS, sizeof(SSchema *)); + if (pTable->schema == NULL) { + terrno = TAOS_SYSTEM_ERROR(errno); + return -1; + } + } + + ASSERT(taosArrayGetSize(pTable->schema) == 0 || + schemaVersion(pSchema) > schemaVersion(*(STSchema **)taosArrayGetLast(pTable->schema))); + + if (taosArrayPush(pTable->schema, &pSchema) == NULL) { + terrno = TAOS_SYSTEM_ERROR(errno); + return -1; + } + + return 0; +} + +static void tsdbFreeTableSchema(STable *pTable) { + ASSERT(pTable != NULL); + + if (pTable->schema) { + for (size_t i = 0; i < taosArrayGetSize(pTable->schema); i++) { + STSchema *pSchema = taosArrayGetP(pTable->schema, i); + tdFreeSchema(pSchema); + } + } +} \ No newline at end of file From cbcf773962ad5b189f53739529f158b37c481aee Mon Sep 17 00:00:00 2001 From: Hongze Cheng Date: Thu, 5 Aug 2021 15:05:14 +0800 Subject: [PATCH 2/6] fix compile error --- src/tsdb/src/tsdbMeta.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tsdb/src/tsdbMeta.c b/src/tsdb/src/tsdbMeta.c index 3fb5739641..fca1b17ba9 100644 --- a/src/tsdb/src/tsdbMeta.c +++ b/src/tsdb/src/tsdbMeta.c @@ -1251,7 +1251,7 @@ static int tsdbEncodeTable(void **buf, STable *pTable) { tlen += taosEncodeFixedU64(buf, TABLE_SUID(pTable)); tlen += tdEncodeKVRow(buf, pTable->tagVal); } else { - tlen += taosEncodeFixedU8(buf, taosArrayGetSize(pTable->schema)); + tlen += taosEncodeFixedU8(buf, (uint8_t)taosArrayGetSize(pTable->schema)); for (int i = 0; i < taosArrayGetSize(pTable->schema); i++) { STSchema *pSchema = taosArrayGetP(pTable->schema, i); tlen += tdEncodeSchema(buf, pSchema); From d648f557538c7e527fade2c56b7c04d0855a08cd Mon Sep 17 00:00:00 2001 From: Hongze Cheng Date: Thu, 5 Aug 2021 16:57:50 +0800 Subject: [PATCH 3/6] fix memory leak --- src/tsdb/src/tsdbMeta.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/tsdb/src/tsdbMeta.c b/src/tsdb/src/tsdbMeta.c index fca1b17ba9..b2395f8417 100644 --- a/src/tsdb/src/tsdbMeta.c +++ b/src/tsdb/src/tsdbMeta.c @@ -1483,5 +1483,7 @@ static void tsdbFreeTableSchema(STable *pTable) { STSchema *pSchema = taosArrayGetP(pTable->schema, i); tdFreeSchema(pSchema); } + + taosArrayDestroy(pTable->schema); } } \ No newline at end of file From 907c87018130836b7b0ba91c8ab522e950fd0e43 Mon Sep 17 00:00:00 2001 From: Hongze Cheng Date: Thu, 5 Aug 2021 17:28:10 +0800 Subject: [PATCH 4/6] fix coredump --- src/tsdb/inc/tsdbMeta.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tsdb/inc/tsdbMeta.h b/src/tsdb/inc/tsdbMeta.h index 29d2b6595d..51801c843c 100644 --- a/src/tsdb/inc/tsdbMeta.h +++ b/src/tsdb/inc/tsdbMeta.h @@ -106,9 +106,9 @@ static FORCE_INLINE STSchema* tsdbGetTableSchemaImpl(STable* pTable, bool lock, if (lock) TSDB_RLOCK_TABLE(pDTable); if (_version < 0) { // get the latest version of schema - pTSchema = *(STSchema **)taosArrayGetLast(pTable->schema); + pTSchema = *(STSchema **)taosArrayGetLast(pDTable->schema); } else { // get the schema with version - void* ptr = taosArraySearch(pTable->schema, &_version, tsdbCompareSchemaVersion, TD_EQ); + void* ptr = taosArraySearch(pDTable->schema, &_version, tsdbCompareSchemaVersion, TD_EQ); if (ptr == NULL) { terrno = TSDB_CODE_TDB_IVD_TB_SCHEMA_VERSION; goto _exit; From 1d881042d5410ae013c29234a70f6323c553f26a Mon Sep 17 00:00:00 2001 From: Hongze Cheng Date: Thu, 5 Aug 2021 17:32:39 +0800 Subject: [PATCH 5/6] fix another possible coredump --- src/tsdb/src/tsdbMeta.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tsdb/src/tsdbMeta.c b/src/tsdb/src/tsdbMeta.c index b2395f8417..fa125dfa5f 100644 --- a/src/tsdb/src/tsdbMeta.c +++ b/src/tsdb/src/tsdbMeta.c @@ -725,7 +725,7 @@ void tsdbUpdateTableSchema(STsdbRepo *pRepo, STable *pTable, STSchema *pSchema, STsdbMeta *pMeta = pRepo->tsdbMeta; STable *pCTable = (TABLE_TYPE(pTable) == TSDB_CHILD_TABLE) ? pTable->pSuper : pTable; - ASSERT(schemaVersion(pSchema) > schemaVersion(*(STSchema **)taosArrayGetLast(pTable->schema))); + ASSERT(schemaVersion(pSchema) > schemaVersion(*(STSchema **)taosArrayGetLast(pCTable->schema))); TSDB_WLOCK_TABLE(pCTable); tsdbAddSchema(pCTable, pSchema); From b6280cee5822640016732d01594b6ac16df47c83 Mon Sep 17 00:00:00 2001 From: jiajingbin Date: Fri, 6 Aug 2021 03:50:27 -0300 Subject: [PATCH 6/6] [TD-5816]: add testcase alter/alterColMultiTimes.py for TD-5816 fix a small-probability bug for insert/schemalessInsert.py --- tests/pytest/alter/alterColMultiTimes.py | 67 ++++++++++++++++++++++++ tests/pytest/fulltest.sh | 1 + tests/pytest/insert/schemalessInsert.py | 2 +- 3 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 tests/pytest/alter/alterColMultiTimes.py diff --git a/tests/pytest/alter/alterColMultiTimes.py b/tests/pytest/alter/alterColMultiTimes.py new file mode 100644 index 0000000000..173ca8158d --- /dev/null +++ b/tests/pytest/alter/alterColMultiTimes.py @@ -0,0 +1,67 @@ +################################################################### +# Copyright (c) 2016 by TAOS Technologies, Inc. +# All rights reserved. +# +# This file is proprietary and confidential to TAOS Technologies. +# No part of this file may be reproduced, stored, transmitted, +# disclosed or used in any form or by any means other than as +# expressly provided by the written permission from Jianhui Tao +# +################################################################### + +# -*- coding: utf-8 -*- + +import random +import string +from util.log import * +from util.cases import * +from util.sql import * +from util.dnodes import * + +class TDTestCase: + def init(self, conn, logSql): + tdLog.debug("start to execute %s" % __file__) + tdSql.init(conn.cursor(), logSql) + + def genColList(self): + ''' + generate column list + ''' + col_list = list() + for i in range(1, 18): + col_list.append(f'c{i}') + return col_list + + def genIncreaseValue(self, input_value): + ''' + add ', 1' to end of value every loop + ''' + value_list = list(input_value) + value_list.insert(-1, ", 1") + return ''.join(value_list) + + def insertAlter(self): + ''' + after each alter and insert, when execute 'select * from {tbname};' taosd will coredump + ''' + tbname = ''.join(random.choice(string.ascii_letters.lower()) for i in range(7)) + input_value = '(now, 1)' + tdSql.execute(f'create table {tbname} (ts timestamp, c0 int);') + tdSql.execute(f'insert into {tbname} values {input_value};') + for col in self.genColList(): + input_value = self.genIncreaseValue(input_value) + tdSql.execute(f'alter table {tbname} add column {col} int;') + tdSql.execute(f'insert into {tbname} values {input_value};') + tdSql.query(f'select * from {tbname};') + tdSql.checkRows(18) + + def run(self): + tdSql.prepare() + self.insertAlter() + + def stop(self): + tdSql.close() + tdLog.success("%s successfully executed" % __file__) + +tdCases.addWindows(__file__, TDTestCase()) +tdCases.addLinux(__file__, TDTestCase()) diff --git a/tests/pytest/fulltest.sh b/tests/pytest/fulltest.sh index 783ee98da3..d59088574d 100755 --- a/tests/pytest/fulltest.sh +++ b/tests/pytest/fulltest.sh @@ -380,6 +380,7 @@ python3 ./test.py -f query/querySession.py python3 test.py -f alter/alter_create_exception.py python3 ./test.py -f insert/flushwhiledrop.py python3 ./test.py -f insert/schemalessInsert.py +python3 ./test.py -f alter/alterColMultiTimes.py #======================p4-end=============== python3 test.py -f tools/taosdemoAllTest/pytest.py diff --git a/tests/pytest/insert/schemalessInsert.py b/tests/pytest/insert/schemalessInsert.py index 5c93095a1e..88abea477a 100644 --- a/tests/pytest/insert/schemalessInsert.py +++ b/tests/pytest/insert/schemalessInsert.py @@ -705,7 +705,7 @@ class TDTestCase: case no id when stb exist """ self.cleanStb() - input_sql, stb_name = self.genFullTypeSql(t0="f", c0="f") + input_sql, stb_name = self.genFullTypeSql(tb_name="sub_table_0123456", t0="f", c0="f") self.resCmp(input_sql, stb_name) input_sql, stb_name = self.genFullTypeSql(stb_name=stb_name, id_noexist_tag=True, t0="f", c0="f") self.resCmp(input_sql, stb_name, condition='where tbname like "t_%"')