database/openGauss-server
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

!484 MOT code review and memcheck fixes

Browse Source 
Merge pull request !484 from Vinoth Veeraraghavan/master
This commit is contained in:
opengauss-bot
2020-12-13 11:02:24 +08:00
committed by Gitee
parent 4898995c3a d40c5607cb
commit b9ffef274c
19 changed files with 124 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
src/gausskernel/storage/mot/core/src/infra/config/config_tree.h
View File

@ -153,15 +153,6 @@ public:
return m_rootSection;
}
/**
* @brief Retrieves the virtual root section for modification.
* @return The root section.
*/
inline ConfigSection* ModifyRootSection()
{
return &m_rootSection;
}
/**
* @brief Prints the configuration tree to the log.
* @param logLevel The log level used for printing.

59
src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_ctrlfile.cpp
View File

@ -38,48 +38,72 @@ static spin_lock g_ctrlfileLock;
CheckpointControlFile* CheckpointControlFile::GetCtrlFile()
{
if (initialized)
if (initialized) {
return ctrlfileInst;
}
g_ctrlfileLock.lock();
ctrlfileInst = new (std::nothrow) CheckpointControlFile();
if (ctrlfileInst == nullptr) {
MOT_REPORT_ERROR(MOT_ERROR_OOM, "Checkpoint", "Failed to allocate memory for checkpoint control file object");
} else {
if (!ctrlfileInst->Init()) {
MOT_REPORT_ERROR(
MOT_ERROR_INVALID_CFG, "Checkpoint", "Failed to initialize checkpoint control file object");
delete ctrlfileInst;
ctrlfileInst = nullptr;
}
}
g_ctrlfileLock.unlock();
return ctrlfileInst;
}
void CheckpointControlFile::Init()
bool CheckpointControlFile::Init()
{
int fd = -1;
if (initialized)
return;
if (initialized) {
return true;
}
do {
if (GetGlobalConfiguration().m_checkpointDir.length() >= CheckpointUtils::maxPath) {
MOT_REPORT_ERROR(MOT_ERROR_INVALID_CFG,
"Checkpoint",
"Invalid checkpoint_dir configuration, length exceeds max path length");
break;
}
if (!CheckpointUtils::GetWorkingDir(m_fullPath)) {
MOT_LOG_ERROR("Could not obtain working directory");
break;
}
m_fullPath.append("/");
if (!CheckpointUtils::IsDirExists(m_fullPath)) {
MOT_REPORT_ERROR(
MOT_ERROR_INVALID_CFG, "Checkpoint", "Invalid checkpoint_dir configuration, directory doesn't exist");
break;
}
// "/" is already appended in CheckpointUtils::GetWorkingDir.
m_fullPath.append(CTRL_FILE_NAME);
MOT_LOG_TRACE("CheckpointControlFile: Fullpath - '%s'", m_fullPath.c_str());
// try to open an old file
if (!CheckpointUtils::FileExists(m_fullPath)) {
if (!CheckpointUtils::IsFileExists(m_fullPath)) {
MOT_LOG_INFO("CheckpointControlFile: init - a previous checkpoint was not found");
m_controlFile.Init();
m_ctrlFileData.Init();
} else {
int fd = -1;
if (!CheckpointUtils::OpenFileRead(m_fullPath, fd)) {
MOT_LOG_ERROR("CheckpointControlFile: init - could not open control file");
break;
}
if (CheckpointUtils::ReadFile(fd, (char*)&m_controlFile, sizeof(CtrlFileData)) != sizeof(CtrlFileData)) {
if (CheckpointUtils::ReadFile(fd, (char*)&m_ctrlFileData, sizeof(CtrlFileData)) != sizeof(CtrlFileData)) {
MOT_LOG_ERROR("CheckpointControlFile: init - failed to read data from file");
CheckpointUtils::CloseFile(fd);
break;
} else {
MOT_LOG_INFO(
"CheckpointControlFile: init - loaded file: checkpointId %lu", m_controlFile.entry[0].checkpointId);
MOT_LOG_INFO("CheckpointControlFile: init - loaded file: checkpointId %lu",
m_ctrlFileData.entry[0].checkpointId);
}
if (CheckpointUtils::CloseFile(fd)) {
MOT_LOG_ERROR("CheckpointControlFile: init - failed to close file");
@ -87,10 +111,11 @@ void CheckpointControlFile::Init()
}
}
m_valid = true;
initialized = true;
Print();
} while (0);
initialized = true;
Print();
return initialized;
}
bool CheckpointControlFile::Update(uint64_t id, uint64_t lsn, uint64_t lastReplayLsn)
@ -104,11 +129,11 @@ bool CheckpointControlFile::Update(uint64_t id, uint64_t lsn, uint64_t lastRepla
break;
}
m_controlFile.entry[0].checkpointId = id;
m_controlFile.entry[0].lsn = lsn;
m_controlFile.entry[0].lastReplayLsn = lastReplayLsn;
m_ctrlFileData.entry[0].checkpointId = id;
m_ctrlFileData.entry[0].lsn = lsn;
m_ctrlFileData.entry[0].lastReplayLsn = lastReplayLsn;
if (CheckpointUtils::WriteFile(fd, (char*)&m_controlFile, sizeof(CtrlFileData)) != sizeof(CtrlFileData)) {
if (CheckpointUtils::WriteFile(fd, (char*)&m_ctrlFileData, sizeof(CtrlFileData)) != sizeof(CtrlFileData)) {
MOT_LOG_ERROR("CheckpointControlFile::update - failed to write control file");
CheckpointUtils::CloseFile(fd);
break;

14
src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_ctrlfile.h
View File

@ -35,14 +35,12 @@ namespace MOT {
class CheckpointControlFile {
public:
CheckpointControlFile() : m_valid(false)
{
Init();
}
{}
~CheckpointControlFile()
{}
void Init();
bool Init();
struct CtrlFileElem {
CtrlFileElem(uint64_t id = invalidId, uint64_t lsn = invalidId, uint64_t replay = invalidId)
@ -65,17 +63,17 @@ public:
uint64_t GetId() const
{
return m_controlFile.entry[0].checkpointId;
return m_ctrlFileData.entry[0].checkpointId;
}
uint64_t GetLsn() const
{
return m_controlFile.entry[0].lsn;
return m_ctrlFileData.entry[0].lsn;
}
uint64_t GetLastReplayLsn() const
{
return m_controlFile.entry[0].lastReplayLsn;
return m_ctrlFileData.entry[0].lastReplayLsn;
}
/**
@ -118,7 +116,7 @@ private:
std::string m_fullPath;
struct CtrlFileData m_controlFile;
struct CtrlFileData m_ctrlFileData;
bool m_valid;
};

8
src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_manager.cpp
View File

@ -620,16 +620,12 @@ void CheckpointManager::RemoveOldCheckpoints(uint64_t curCheckcpointId)
void CheckpointManager::RemoveCheckpointDir(uint64_t checkpointId)
{
errno_t erc;
char buf[CheckpointUtils::maxPath];
std::string oldCheckpointDir;
if (!CheckpointUtils::SetWorkingDir(oldCheckpointDir, checkpointId)) {
MOT_LOG_ERROR("removeCheckpointDir: failed to set working directory");
return;
}
char* buf = (char*)malloc(CheckpointUtils::maxPath);
if (buf == nullptr) {
MOT_LOG_ERROR("removeCheckpointDir: failed to allocate buffer");
return;
}
DIR* dir = opendir(oldCheckpointDir.c_str());
if (dir != nullptr) {
@ -663,8 +659,6 @@ void CheckpointManager::RemoveCheckpointDir(uint64_t checkpointId)
errno,
gs_strerror(errno));
}
free(buf);
}
bool CheckpointManager::CreateCheckpointMap()

24
src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_utils.cpp
View File

@ -32,12 +32,23 @@ DECLARE_LOGGER(CheckpointUtils, Checkpoint);
namespace CheckpointUtils {
bool FileExists(std::string fileName)
bool IsFileExists(std::string fileName)
{
struct stat buf;
if (stat(fileName.c_str(), &buf) == -1 && errno == ENOENT) {
struct stat statBuf = {0};
if (stat(fileName.c_str(), &statBuf) == -1 && errno == ENOENT) {
return false;
} else if (!S_ISREG(buf.st_mode)) {
} else if (!S_ISREG(statBuf.st_mode)) {
return false;
}
return true;
}
bool IsDirExists(std::string dirName)
{
struct stat statBuf = {0};
if (stat(dirName.c_str(), &statBuf) == -1 && errno == ENOENT) {
return false;
} else if (!S_ISDIR(statBuf.st_mode)) {
return false;
}
return true;
@ -134,8 +145,9 @@ bool GetWorkingDir(std::string& dir)
{
dir.clear();
char cwd[maxPath] = {0};
if (GetGlobalConfiguration().m_checkpointDir.length()) {
errno_t erc = strncpy_s(cwd, maxPath, GetGlobalConfiguration().m_checkpointDir.c_str(), maxPath - 1);
size_t checkpointDirLength = GetGlobalConfiguration().m_checkpointDir.length();
if (checkpointDirLength > 0) {
errno_t erc = strncpy_s(cwd, maxPath, GetGlobalConfiguration().m_checkpointDir.c_str(), checkpointDirLength);
securec_check(erc, "\0", "\0");
} else if (!getcwd(cwd, sizeof(cwd))) {
MOT_REPORT_SYSTEM_ERROR(getcwd, "N/A", "Failed to get current working directory");

11
src/gausskernel/storage/mot/core/src/system/checkpoint/checkpoint_utils.h
View File

@ -39,11 +39,18 @@ namespace MOT {
namespace CheckpointUtils {
/**
* @brief A wrapper function that checks is a file exists
* @brief A wrapper function that checks if a file exists
* @param fileName The file name to check
* @return Boolean value denoting success or failure.
*/
bool FileExists(std::string fileName);
bool IsFileExists(std::string fileName);
/**
* @brief A wrapper function that checks if a dir exists
* @param fileName The directory name to check
* @return Boolean value denoting success or failure.
*/
bool IsDirExists(std::string dirName);
/**
* @brief A wrapper function that opens a file for writing.

3
src/gausskernel/storage/mot/core/src/system/recovery/checkpoint_recovery.cpp
View File

@ -717,7 +717,6 @@ bool CheckpointRecovery::DeserializeInProcessTxns(int fd, uint64_t numEntries)
bool CheckpointRecovery::IsCheckpointValid(uint64_t id)
{
int fd = -1;
std::string fileName;
std::string workingDir;
bool ret = false;
@ -728,7 +727,7 @@ bool CheckpointRecovery::IsCheckpointValid(uint64_t id)
}
CheckpointUtils::MakeEndFilename(fileName, workingDir, id);
if (!CheckpointUtils::FileExists(fileName)) {
if (!CheckpointUtils::IsFileExists(fileName)) {
MOT_LOG_ERROR("IsCheckpointValid: checkpoint id %lu is invalid", id);
break;
}

6
src/gausskernel/storage/mot/core/src/system/recovery/recovery_manager.cpp
View File

@ -312,7 +312,7 @@ RC RecoveryManager::RedoSegment(
bool RecoveryManager::LogStats::FindIdx(uint64_t tableId, uint64_t& id)
{
id = m_numEntries;
std::map<uint64_t, int>::iterator it;
std::map<uint64_t, uint64_t>::iterator it;
std::lock_guard<spin_lock> lock(m_slock);
it = m_idToIdx.find(tableId);
if (it == m_idToIdx.end()) {
@ -321,7 +321,7 @@ bool RecoveryManager::LogStats::FindIdx(uint64_t tableId, uint64_t& id)
return false;
}
m_tableStats.push_back(newEntry);
m_idToIdx.insert(std::pair<int, int>(tableId, m_numEntries));
m_idToIdx.insert(std::pair<uint64_t, uint64_t>(tableId, m_numEntries));
m_numEntries++;
} else {
id = it->second;
@ -332,7 +332,7 @@ bool RecoveryManager::LogStats::FindIdx(uint64_t tableId, uint64_t& id)
void RecoveryManager::LogStats::Print()
{
MOT_LOG_ERROR(">> log recovery stats >>");
for (int i = 0; i < m_numEntries; i++) {
for (uint64_t i = 0; i < m_numEntries; i++) {
MOT_LOG_ERROR("TableId %lu, Inserts: %lu, Updates: %lu, Deletes: %lu",
m_tableStats[i]->m_id,
m_tableStats[i]->m_inserts.load(),

4
src/gausskernel/storage/mot/core/src/system/recovery/recovery_manager.h
View File

@ -245,7 +245,7 @@ public:
*/
bool FindIdx(uint64_t tableId, uint64_t& id);
std::map<uint64_t, int> m_idToIdx;
std::map<uint64_t, uint64_t> m_idToIdx;
std::vector<Entry*> m_tableStats;
@ -253,7 +253,7 @@ public:
spin_lock m_slock;
int m_numEntries;
uint64_t m_numEntries;
};
LogStats* m_logStats;

2
src/gausskernel/storage/mot/core/src/system/statistics/process_statistics.cpp
View File

@ -73,7 +73,7 @@ ProcessStatisticsProvider::ProcessStatisticsProvider()
++m_processorCount;
}
}
fclose(file);
(void)fclose(file);
}
MOT_LOG_DEBUG("Detected %u processors", m_processorCount);
}

1
src/gausskernel/storage/mot/core/src/system/transaction/txn_access.cpp
View File

@ -151,7 +151,6 @@ bool TxnAccess::Init(TxnManager* manager)
}
m_initPhase = CreateRowSet;
m_initPhase = CreateInsertSet;
m_insertManager = new (std::nothrow) TxnInsertAction();
if (m_insertManager == nullptr) {
MOT_REPORT_ERROR(MOT_ERROR_OOM,

1
src/gausskernel/storage/mot/core/src/system/transaction/txn_access.h
View File

@ -338,7 +338,6 @@ private:
InitDummyInd,
CreateRowZero,
CreateRowSet,
CreateInsertSet,
Done
} m_initPhase;

4
src/gausskernel/storage/mot/core/src/system/transaction/txn_ddl_access.cpp
View File

@ -74,8 +74,10 @@ uint32_t TxnDDLAccess::Size()
void TxnDDLAccess::Reset()
{
for (int i = 0; i < m_size; i++)
for (int i = 0; i < m_size; i++) {
delete m_accessList[i];
m_accessList[i] = nullptr;
}
m_size = 0;
}

8
src/gausskernel/storage/mot/fdw_adapter/src/mot_fdw.cpp
View File

@ -519,7 +519,7 @@ static bool IsOrderingApplicable(PathKey* pathKey, RelOptInfo* rel, MOT::Index*
do {
const int16_t* cols = ix->GetColumnKeyFields();
int16_t numKeyCols = ix->GetNumFields();
ListCell* lcEm;
ListCell* lcEm = nullptr;
foreach (lcEm, pathKey->pk_eclass->ec_members) {
EquivalenceMember* em = (EquivalenceMember*)lfirst(lcEm);
@ -975,7 +975,7 @@ static TupleTableSlot* MOTIterateForeignScan(ForeignScanState* node)
return nullptr;
}
MOT::Row* currRow;
MOT::Row* currRow = nullptr;
MOTFdwStateSt* festate = (MOTFdwStateSt*)node->fdw_state;
TupleTableSlot* slot = node->ss.ss_ScanTupleSlot;
bool found = false;
@ -1430,7 +1430,7 @@ static TupleTableSlot* MOTExecForeignUpdate(
{
MOTFdwStateSt* fdwState = (MOTFdwStateSt*)resultRelInfo->ri_FdwState;
MOT::RC rc = MOT::RC_OK;
MOT::Row* currRow;
MOT::Row* currRow = nullptr;
AttrNumber num = fdwState->m_ctidNum - 1;
MOTRecConvertSt cv;
@ -1488,7 +1488,7 @@ static TupleTableSlot* MOTExecForeignDelete(
{
MOTFdwStateSt* fdwState = (MOTFdwStateSt*)resultRelInfo->ri_FdwState;
MOT::RC rc = MOT::RC_OK;
MOT::Row* currRow;
MOT::Row* currRow = nullptr;
AttrNumber num = fdwState->m_ctidNum - 1;
MOTRecConvertSt cv;

24
src/gausskernel/storage/mot/fdw_adapter/src/mot_internal.cpp
View File

@ -71,18 +71,6 @@
oid == FLOAT8OID || oid == INT1OID || oid == DATEOID || oid == TIMEOID || oid == TIMESTAMPOID || \
oid == TIMESTAMPTZOID)
#define FILL_KEY_NULL(toid, buf, len) \
{ \
errno_t erc = memset_s(buf, len, 0x00, len); \
securec_check(erc, "\0", "\0"); \
}
#define FILL_KEY_MAX(toid, buf, len) \
{ \
errno_t erc = memset_s(buf, len, 0xff, len); \
securec_check(erc, "\0", "\0"); \
}
MOT::MOTEngine* MOTAdaptor::m_engine = nullptr;
static XLOGLogger xlogger;
@ -838,7 +826,8 @@ void MOTAdaptor::OpenCursor(Relation rel, MOTFdwStateSt* festate)
festate->m_stateKey[bIx].InitKey(keyLength);
buf = festate->m_stateKey[bIx].GetKeyBuf();
FILL_KEY_MAX(INT8OID, buf, keyLength);
errno_t erc = memset_s(buf, keyLength, 0xff, keyLength);
securec_check(erc, "\0", "\0");
festate->m_cursor[bIx] =
ix->Search(&festate->m_stateKey[bIx], false, false, festate->m_currTxn->GetThdId(), found);
break;
@ -853,7 +842,8 @@ void MOTAdaptor::OpenCursor(Relation rel, MOTFdwStateSt* festate)
festate->m_stateKey[1].InitKey(keyLength);
buf = festate->m_stateKey[1].GetKeyBuf();
FILL_KEY_MAX(INT8OID, buf, keyLength);
errno_t erc = memset_s(buf, keyLength, 0xff, keyLength);
securec_check(erc, "\0", "\0");
festate->m_cursor[1] =
ix->Search(&festate->m_stateKey[1], false, false, festate->m_currTxn->GetThdId(), found);
} else {
@ -1257,8 +1247,7 @@ MOT::RC MOTAdaptor::CreateTable(CreateForeignTableStmt* table, ::TransactionId t
break;
}
ListCell* cell;
ListCell* cell = nullptr;
foreach (cell, table->base.tableElts) {
int16 typeLen = 0;
bool isBlob = false;
@ -1692,7 +1681,8 @@ void MOTAdaptor::CreateKeyBuffer(Relation rel, MOTFdwStateSt* festate, int start
Datum val = ExecEvalExpr((ExprState*)(expr), festate->m_econtext, &is_null, nullptr);
if (is_null) {
MOT_ASSERT((offset + fieldLengths[i]) <= keyLength);
FILL_KEY_NULL(desc->attrs[orgCols[i] - 1]->atttypid, buf + offset, fieldLengths[i]);
errno_t erc = memset_s(buf + offset, fieldLengths[i], 0x00, fieldLengths[i]);
securec_check(erc, "\0", "\0");
} else {
MOT::Column* col = festate->m_table->GetField(orgCols[i]);
uint8_t fill = 0x00;

13
src/gausskernel/storage/mot/jit_exec/src/jit_llvm_blocks.cpp
View File

@ -2022,7 +2022,8 @@ static llvm::Value* buildAggregateCount(
static bool buildAggregateMaxMin(JitLlvmCodeGenContext* ctx, JitAggregate* aggregate, llvm::Value* var_expr)
{
bool result = true;
bool result = false;
llvm::Value* aggregateExpr = nullptr;
// we first check if the min/max value is null and if so just store the column value
JIT_IF_BEGIN(test_max_min_value_null)
@ -2035,21 +2036,25 @@ static bool buildAggregateMaxMin(JitLlvmCodeGenContext* ctx, JitAggregate* aggre
llvm::Value* current_aggregate = AddGetAggValue(ctx);
switch (aggregate->_aggreaget_op) {
case JIT_AGGREGATE_MAX:
result = buildAggregateMax(ctx, aggregate, current_aggregate, var_expr);
aggregateExpr = buildAggregateMax(ctx, aggregate, current_aggregate, var_expr);
break;
case JIT_AGGREGATE_MIN:
result = buildAggregateMin(ctx, aggregate, current_aggregate, var_expr);
aggregateExpr = buildAggregateMin(ctx, aggregate, current_aggregate, var_expr);
break;
default:
MOT_REPORT_ERROR(
MOT_ERROR_INTERNAL, "JIT Compile", "Invalid aggregate operator %d", (int)aggregate->_aggreaget_op);
result = false;
break;
}
JIT_IF_END()
if (aggregateExpr != nullptr) {
AddSetAggValue(ctx, aggregateExpr);
result = true;
}
return result;
}

4
src/gausskernel/storage/mot/jit_exec/src/jit_tvm.cpp
View File

@ -1054,7 +1054,9 @@ void Builder::CreateRet(Instruction* instruction)
Instruction* Builder::CreateConst(uint64_t value)
{
return new (std::nothrow) ConstInstruction(value);
Instruction* result = new (std::nothrow) ConstInstruction(value);
_current_function->addInstruction(result);
return result;
}
BasicBlock* Builder::GetInsertBlock()

13
src/gausskernel/storage/mot/jit_exec/src/jit_tvm_blocks.cpp
View File

@ -1981,7 +1981,8 @@ static Expression* buildAggregateCount(
static bool buildAggregateMaxMin(JitTvmCodeGenContext* ctx, JitAggregate* aggregate, Expression* var_expr)
{
bool result = true;
bool result = false;
Expression* aggregateExpr = nullptr;
// we first check if the min/max value is null and if so just store the column value
JIT_IF_BEGIN(test_max_min_value_null)
@ -1994,21 +1995,25 @@ static bool buildAggregateMaxMin(JitTvmCodeGenContext* ctx, JitAggregate* aggreg
Expression* current_aggregate = AddGetAggValue(ctx);
switch (aggregate->_aggreaget_op) {
case JIT_AGGREGATE_MAX:
result = buildAggregateMax(ctx, aggregate, current_aggregate, var_expr);
aggregateExpr = buildAggregateMax(ctx, aggregate, current_aggregate, var_expr);
break;
case JIT_AGGREGATE_MIN:
result = buildAggregateMin(ctx, aggregate, current_aggregate, var_expr);
aggregateExpr = buildAggregateMin(ctx, aggregate, current_aggregate, var_expr);
break;
default:
MOT_REPORT_ERROR(
MOT_ERROR_INTERNAL, "JIT Compile", "Invalid aggregate operator %d", (int)aggregate->_aggreaget_op);
result = false;
break;
}
JIT_IF_END()
if (aggregateExpr != nullptr) {
AddSetAggValue(ctx, aggregateExpr);
result = true;
}
return result;
}

8
src/gausskernel/storage/mot/jit_exec/src/jit_tvm_funcs.h
View File

@ -2362,7 +2362,9 @@ public:
~SaveAvgArrayInstruction() final
{
_avg_array = nullptr;
if (_avg_array != nullptr) {
delete _avg_array;
}
}
uint64_t Exec(tvm::ExecContext* exec_context) final
@ -2460,7 +2462,9 @@ public:
~SetAggValueInstruction() final
{
_value = nullptr;
if (_value != nullptr) {
delete _value;
}
}
Datum Exec(tvm::ExecContext* exec_context) final