From e2f1b07e705629ad6f8e6bbc6ea080968bcd421f Mon Sep 17 00:00:00 2001 From: Roman Nozdrin Date: Fri, 16 Aug 2019 21:28:07 +0300 Subject: [PATCH] MCOL-3317 Moved fill-next-block from writeRow() into allocRowId. Intro* INSERT statements could face a non-existant block when MCOL-498 feature is enabled. writeRow() guard blocks was supposed to proactively create empty blocks. The pre-patch logic failed when first value in the block has been removed by DELETE and this overwrites the whole valid block with empty magics. This patch moves proactive creation logic into allocRowId(). --- dbcon/ddlpackageproc/droptableprocessor.cpp | 36 ++--- dbcon/mysql/ha_calpont_ddl.cpp | 2 +- writeengine/shared/we_define.h | 1 + writeengine/shared/we_fileop.cpp | 10 +- writeengine/wrapper/we_colop.cpp | 152 ++++++-------------- 5 files changed, 75 insertions(+), 126 deletions(-) diff --git a/dbcon/ddlpackageproc/droptableprocessor.cpp b/dbcon/ddlpackageproc/droptableprocessor.cpp index a12bdb44a..2de7174a0 100644 --- a/dbcon/ddlpackageproc/droptableprocessor.cpp +++ b/dbcon/ddlpackageproc/droptableprocessor.cpp @@ -289,9 +289,9 @@ DropTableProcessor::DDLResult DropTableProcessor::processPackage(ddlpackage::Dro //get a unique number VERBOSE_INFO("Removing the SYSTABLE meta data"); -//#ifdef IDB_DDL_DEBUG +#ifdef IDB_DDL_DEBUG cout << fTxnid.id << " Removing the SYSTABLEs meta data" << endl; -//#endif +#endif bytestream << (ByteStream::byte)WE_SVR_DELETE_SYSTABLE; bytestream << uniqueId; bytestream << (uint32_t) dropTableStmt.fSessionID; @@ -324,9 +324,9 @@ DropTableProcessor::DDLResult DropTableProcessor::processPackage(ddlpackage::Dro try { -// #ifdef IDB_DDL_DEBUG +#ifdef IDB_DDL_DEBUG cout << fTxnid.id << " Drop table sending WE_SVR_DELETE_SYSTABLES to pm " << pmNum << endl; -//#endif +#endif //cout << "deleting systable entries with txnid " << txnID.id << endl; fWEClient->write(bytestream, (uint32_t)pmNum); @@ -356,18 +356,18 @@ DropTableProcessor::DDLResult DropTableProcessor::processPackage(ddlpackage::Dro } catch (runtime_error& ex) //write error { -// #ifdef IDB_DDL_DEBUG +#ifdef IDB_DDL_DEBUG cout << fTxnid.id << " Drop table got exception" << endl; -// #endif +#endif rc = NETWORK_ERROR; errorMsg = ex.what(); } catch (...) { rc = NETWORK_ERROR; -//#ifdef IDB_DDL_DEBUG +#ifdef IDB_DDL_DEBUG cout << fTxnid.id << " Drop table got unknown exception" << endl; -//#endif +#endif } if (rc != 0) @@ -417,9 +417,9 @@ DropTableProcessor::DDLResult DropTableProcessor::processPackage(ddlpackage::Dro try { -//#ifdef IDB_DDL_DEBUG +#ifdef IDB_DDL_DEBUG cout << fTxnid.id << " Drop table sending WE_SVR_DELETE_SYSCOLUMN to pm " << pmNum << endl; -//#endif +#endif fWEClient->write(bytestream, (unsigned)pmNum); while (1) @@ -448,18 +448,18 @@ DropTableProcessor::DDLResult DropTableProcessor::processPackage(ddlpackage::Dro } catch (runtime_error& ex) //write error { -//#ifdef IDB_DDL_DEBUG +#ifdef IDB_DDL_DEBUG cout << fTxnid.id << " Drop table got exception" << endl; -//#endif +#endif rc = NETWORK_ERROR; errorMsg = ex.what(); } catch (...) { rc = NETWORK_ERROR; -// #ifdef IDB_DDL_DEBUG +#ifdef IDB_DDL_DEBUG cout << fTxnid.id << " Drop table got unknown exception" << endl; -//#endif +#endif } if (rc != 0) @@ -612,9 +612,9 @@ DropTableProcessor::DDLResult DropTableProcessor::processPackage(ddlpackage::Dro bytestream << (uint32_t) oidList[i]; } -//#ifdef IDB_DDL_DEBUG +#ifdef IDB_DDL_DEBUG cout << fTxnid.id << " Drop table removing column files" << endl; -//#endif +#endif uint32_t msgRecived = 0; try @@ -686,9 +686,9 @@ DropTableProcessor::DDLResult DropTableProcessor::processPackage(ddlpackage::Dro //Flush primProc cache rc = cacheutils::flushOIDsFromCache( oidList ); //Delete extents from extent map -//#ifdef IDB_DDL_DEBUG +#ifdef IDB_DDL_DEBUG cout << fTxnid.id << " Drop table deleteOIDs" << endl; -//#endif +#endif rc = fDbrm->deleteOIDs(oidList); if (rc != 0) diff --git a/dbcon/mysql/ha_calpont_ddl.cpp b/dbcon/mysql/ha_calpont_ddl.cpp index 1fc07be6d..af51e8a6a 100644 --- a/dbcon/mysql/ha_calpont_ddl.cpp +++ b/dbcon/mysql/ha_calpont_ddl.cpp @@ -2150,7 +2150,7 @@ int ProcessDDLStatement(string& ddlStatement, string& schema, const string& tabl if (b == ddlpackageprocessor::DDLPackageProcessor::WARNING) { rc = 0; - string errmsg ("Error occured during file deletion. Restart DMLProc or use command tool ddlcleanup to clean up. " ); + string errmsg ("Error occured during file deletion. Restart DDLProc or use command tool ddlcleanup to clean up. " ); push_warning(thd, Sql_condition::WARN_LEVEL_WARN, 9999, errmsg.c_str()); } diff --git a/writeengine/shared/we_define.h b/writeengine/shared/we_define.h index 8c3e85fe9..08256a2cb 100644 --- a/writeengine/shared/we_define.h +++ b/writeengine/shared/we_define.h @@ -152,6 +152,7 @@ const int ERR_FILE_STAT = ERR_FILEBASE + 16;// Error getting stats o const int ERR_VB_FILE_NOT_EXIST = ERR_FILEBASE + 17;// Version buffer file not exists const int ERR_FILE_FLUSH = ERR_FILEBASE + 18;// Error flushing file const int ERR_FILE_GLOBBING = ERR_FILEBASE + 19;// Error globbing a file name +const int ERR_FILE_EOF = ERR_FILEBASE + 20;// EOF //-------------------------------------------------------------------------- // XML level error diff --git a/writeengine/shared/we_fileop.cpp b/writeengine/shared/we_fileop.cpp index d25088f77..1b181ac22 100644 --- a/writeengine/shared/we_fileop.cpp +++ b/writeengine/shared/we_fileop.cpp @@ -2603,8 +2603,16 @@ int FileOp::readFile( IDBDataFile* pFile, unsigned char* readBuf, { if ( pFile != NULL ) { - if ( pFile->read( readBuf, readSize ) != readSize ) + int bc = pFile->read( readBuf, readSize ); + if (bc != readSize) + { + // MCOL-498 EOF if a next block is empty + if (bc == 0) + { + return ERR_FILE_EOF; + } return ERR_FILE_READ; + } } else return ERR_FILE_NULL; diff --git a/writeengine/wrapper/we_colop.cpp b/writeengine/wrapper/we_colop.cpp index 00fc25cc8..69e2d6f58 100644 --- a/writeengine/wrapper/we_colop.cpp +++ b/writeengine/wrapper/we_colop.cpp @@ -174,7 +174,23 @@ int ColumnOp::allocRowId(const TxnID& txnid, bool useStartingExtent, } } - RETURN_ON_ERROR(readBlock(column.dataFile.pFile, buf, hwm)); + rc = readBlock(column.dataFile.pFile, buf, hwm); + + // MCOL-498 add a block. + // DRRTUY TODO Given there is no more hwm pre-allocated we + // could extend the file once to accomodate all records. + if (rc != NO_ERROR) + { + if (rc == ERR_FILE_EOF) + { + uint64_t emptyVal = getEmptyRowValue(column.colDataType, column.colWidth); + setEmptyBuf(buf, BYTE_PER_BLOCK, emptyVal, column.colWidth); + RETURN_ON_ERROR(saveBlock(column.dataFile.pFile, buf, hwm)); + } else + { + return rc; + } + } for (j = 0; j < totalRowPerBlock; j++) { @@ -197,8 +213,6 @@ int ColumnOp::allocRowId(const TxnID& txnid, bool useStartingExtent, if ((rowsallocated == 0) && isFirstBatchPm) { TableMetaData::removeTableMetaData(tableOid); - //TableMetaData* tableMetaData= TableMetaData::makeTableMetaData(tableOid); - } //Check if a new extent is needed @@ -335,11 +349,6 @@ int ColumnOp::allocRowId(const TxnID& txnid, bool useStartingExtent, vector lbids; vector colDataTypes; - //BRM::CPInfoList_t cpinfoList; - //BRM::CPInfo cpInfo; - //cpInfo.max = numeric_limits::min(); - //cpInfo.min = numeric_limits::max(); - //cpInfo.seqNum = -1; for ( i = 0; i < extents.size(); i++) { setColParam(newCol, 0, newColStructList[i].colWidth, newColStructList[i].colDataType, newColStructList[i].colType, @@ -352,8 +361,6 @@ int ColumnOp::allocRowId(const TxnID& txnid, bool useStartingExtent, if (rc != NO_ERROR) return rc; - //cpInfo.firstLbid = extents[i].startLbid; - //cpinfoList.push_back(cpInfo); newColStructList[i].fColPartition = partition; newColStructList[i].fColSegment = segment; newColStructList[i].fColDbRoot = dbRoot; @@ -365,7 +372,6 @@ int ColumnOp::allocRowId(const TxnID& txnid, bool useStartingExtent, } //mark the extents to updating -//rc = BRMWrapper::getInstance()->setExtentsMaxMin(cpinfoList); rc = BRMWrapper::getInstance()->markExtentsInvalid(lbids, colDataTypes); if (rc != NO_ERROR) @@ -470,18 +476,22 @@ int ColumnOp::allocRowId(const TxnID& txnid, bool useStartingExtent, //..Search first block of new extent for empty rows rc = readBlock(newCol.dataFile.pFile, buf, newHwm); - if ( rc != NO_ERROR) - return rc; - - // MCOL-498 This must be a first block in a new extent so - // fill the block up to its boundary with empties. Otherwise - // there could be fantom values. + // MCOL-498 add a block. + // DRRTUY TODO Given there is no more hwm pre-allocated we + // could extend the file once to accomodate all records. + if (rc != NO_ERROR) { - uint64_t emptyVal = getEmptyRowValue(column.colDataType, column.colWidth); - setEmptyBuf(buf, BYTE_PER_BLOCK, emptyVal, column.colWidth); + if (rc == ERR_FILE_EOF) + { + uint64_t emptyVal = getEmptyRowValue(newCol.colDataType, newCol.colWidth); + setEmptyBuf(buf, BYTE_PER_BLOCK, emptyVal, newCol.colWidth); + RETURN_ON_ERROR(saveBlock(newCol.dataFile.pFile, buf, newHwm)); + } else + { + return rc; + } } - for (j = 0; j < totalRowPerBlock; j++) { if (isEmptyRow(buf, j, column)) @@ -507,14 +517,27 @@ int ColumnOp::allocRowId(const TxnID& txnid, bool useStartingExtent, { rc = readBlock(newCol.dataFile.pFile, buf, newHwm); - if ( rc != NO_ERROR) - return rc; + // MCOL-498 add a block. + // DRRTUY TODO Given there is no more hwm pre-allocated we + // could extend the file once to accomodate all records. + if (rc != NO_ERROR) + { + if (rc == ERR_FILE_EOF) + { + uint64_t emptyVal = getEmptyRowValue(newCol.colDataType, newCol.colWidth); + setEmptyBuf(buf, BYTE_PER_BLOCK, emptyVal, newCol.colWidth); + RETURN_ON_ERROR(saveBlock(newCol.dataFile.pFile, buf, newHwm)); + } else + { + return rc; + } + } for (j = 0; j < totalRowPerBlock; j++) { if (isEmptyRow(buf, j, column)) { - rowIdArray[counter] = getRowId(newHwm, column.colWidth, j); + rowIdArray[counter] = getRowId(newHwm, newCol.colWidth, j); rowsallocated++; rowsLeft++; counter++; @@ -1542,13 +1565,9 @@ int ColumnOp::writeRow(Column& curCol, uint64_t totalRow, const RID* rowIdArray, unsigned char dataBuf[BYTE_PER_BLOCK]; bool bExit = false, bDataDirty = false; void* pVal = 0; -// void* pOldVal; char charTmpBuf[8]; uint64_t emptyVal; int rc = NO_ERROR; - bool fillUpWEmptyVals = false; - bool firstRowInBlock = false; - bool lastRowInBlock = false; uint16_t rowsInBlock = BYTE_PER_BLOCK / curCol.colWidth; while (!bExit) @@ -1568,18 +1587,8 @@ int ColumnOp::writeRow(Column& curCol, uint64_t totalRow, const RID* rowIdArray, return rc; bDataDirty = false; - // MCOL-498 We got into the next block, so the row is first in that block - // - fill the block up with empty magics. - if ( curDataFbo != -1 && !bDelete ) - fillUpWEmptyVals = true; } - // MCOL-498 CS hasn't touched any block yet, - // but the row filled will be the first in the block. - firstRowInBlock = ( !(curRowId % (rowsInBlock)) ) ? true : false; - if( firstRowInBlock && !bDelete ) - fillUpWEmptyVals = true; - curDataFbo = dataFbo; rc = readBlock(curCol.dataFile.pFile, dataBuf, curDataFbo); @@ -1593,17 +1602,14 @@ int ColumnOp::writeRow(Column& curCol, uint64_t totalRow, const RID* rowIdArray, // How about pVal = valArray + i*curCol.colWidth? switch (curCol.colType) { -// case WriteEngine::WR_LONG : pVal = &((long *) valArray)[i]; break; case WriteEngine::WR_FLOAT : if (!bDelete) pVal = &((float*) valArray)[i]; - //pOldVal = &((float *) oldValArray)[i]; break; case WriteEngine::WR_DOUBLE : if (!bDelete) pVal = &((double*) valArray)[i]; - //pOldVal = &((double *) oldValArray)[i]; break; case WriteEngine::WR_VARBINARY : // treat same as char for now @@ -1615,77 +1621,52 @@ int ColumnOp::writeRow(Column& curCol, uint64_t totalRow, const RID* rowIdArray, memcpy(charTmpBuf, (char*)valArray + i * 8, 8); pVal = charTmpBuf; } - - //pOldVal = (char*)oldValArray + i*8; break; -// case WriteEngine::WR_BIT : pVal = &((bool *) valArray)[i]; break; case WriteEngine::WR_SHORT : if (!bDelete) pVal = &((short*) valArray)[i]; - //pOldVal = &((short *) oldValArray)[i]; break; case WriteEngine::WR_BYTE : if (!bDelete) pVal = &((char*) valArray)[i]; - - //pOldVal = &((char *) oldValArray)[i]; break; case WriteEngine::WR_LONGLONG: if (!bDelete) pVal = &((long long*) valArray)[i]; - - //pOldVal = &((long long *) oldValArray)[i]; break; case WriteEngine::WR_TOKEN: if (!bDelete) pVal = &((Token*) valArray)[i]; - - //pOldVal = &((Token *) oldValArray)[i]; break; case WriteEngine::WR_INT : case WriteEngine::WR_MEDINT : if (!bDelete) pVal = &((int*) valArray)[i]; - - //pOldVal = &((int *) oldValArray)[i]; break; case WriteEngine::WR_USHORT: if (!bDelete) pVal = &((uint16_t*) valArray)[i]; - - //pOldVal = &((uint16_t *) oldValArray)[i]; break; case WriteEngine::WR_UBYTE : if (!bDelete) pVal = &((uint8_t*) valArray)[i]; - - //pOldVal = &((uint8_t *) oldValArray)[i]; break; case WriteEngine::WR_UINT : case WriteEngine::WR_UMEDINT : if (!bDelete) pVal = &((uint32_t*) valArray)[i]; - - //pOldVal = &((uint8_t *) oldValArray)[i]; break; case WriteEngine::WR_ULONGLONG: if (!bDelete) pVal = &((uint64_t*) valArray)[i]; - - //pOldVal = &((uint64_t *) oldValArray)[i]; break; default : if (!bDelete) pVal = &((int*) valArray)[i]; - - //pOldVal = &((int *) oldValArray)[i]; break; } - // This is the stuff to retrieve old value - //memcpy(pOldVal, dataBuf + dataBio, curCol.colWidth); - if (bDelete) { emptyVal = getEmptyRowValue(curCol.colDataType, curCol.colWidth); @@ -1704,52 +1685,11 @@ int ColumnOp::writeRow(Column& curCol, uint64_t totalRow, const RID* rowIdArray, // take care of the cleanup if (bDataDirty && curDataFbo >= 0) { - if ( fillUpWEmptyVals ) - { - emptyVal = getEmptyRowValue(curCol.colDataType, curCol.colWidth); - int writeSize = BYTE_PER_BLOCK - ( dataBio + curCol.colWidth ); - // MCOL-498 Add the check though this is unlikely at the moment of writing. - if ( writeSize ) - setEmptyBuf( dataBuf + dataBio + curCol.colWidth, writeSize, - emptyVal, curCol.colWidth ); - } - rc = saveBlock(curCol.dataFile.pFile, dataBuf, curDataFbo); if ( rc != NO_ERROR) return rc; - // MCOL-498 If it was the last row in a block fill the next block with - // empty vals, otherwise next ColumnOp::allocRowId() - // will fail on the next block. - lastRowInBlock = ( rowsInBlock - ( curRowId % rowsInBlock ) == 1 ) ? true : false; - if ( lastRowInBlock ) - { - if( !fillUpWEmptyVals ) - emptyVal = getEmptyRowValue(curCol.colDataType, curCol.colWidth); - // MCOL-498 Skip if this is the last block in an extent. - if ( curDataFbo % MAX_NBLOCKS != MAX_NBLOCKS - 1 ) - { - rc = saveBlock(curCol.dataFile.pFile, dataBuf, curDataFbo); - if ( rc != NO_ERROR) - return rc; - - curDataFbo += 1; - rc = readBlock(curCol.dataFile.pFile, dataBuf, curDataFbo); - if ( rc != NO_ERROR) - return rc; - - unsigned char zeroSubBlock[BYTE_PER_SUBBLOCK]; - std::memset(zeroSubBlock, 0, BYTE_PER_SUBBLOCK); - // The first subblock is made of 0 - fill the block with empty vals. - if ( !std::memcmp(dataBuf, zeroSubBlock, BYTE_PER_SUBBLOCK) ) - { - setEmptyBuf(dataBuf, BYTE_PER_BLOCK, emptyVal, curCol.colWidth); - rc = saveBlock(curCol.dataFile.pFile, dataBuf, curDataFbo); - } - } - } - } return rc; }