From 0693c2389b1017dc0e9bc6c2a848bf5430f1ad8c Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 4 Oct 2006 12:05:24 +0200 Subject: [PATCH 1/4] ndb - bug#22892 Make sure checkKeepGci is also run on oldStoredReplicas to prevent keepgci to move backwards when crash node restarts ndb/src/kernel/blocks/dbdih/DbdihMain.cpp: Make sure checkKeepGci is also run on oldStoredReplicas to prevent keepgci to move backwards when crash node restarts --- ndb/src/kernel/blocks/dbdih/DbdihMain.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/ndb/src/kernel/blocks/dbdih/DbdihMain.cpp b/ndb/src/kernel/blocks/dbdih/DbdihMain.cpp index 0b0b070899c..d8454271a4d 100644 --- a/ndb/src/kernel/blocks/dbdih/DbdihMain.cpp +++ b/ndb/src/kernel/blocks/dbdih/DbdihMain.cpp @@ -9399,6 +9399,7 @@ void Dbdih::calculateKeepGciLab(Signal* signal, Uint32 tableId, Uint32 fragId) FragmentstorePtr fragPtr; getFragstore(tabPtr.p, fragId, fragPtr); checkKeepGci(tabPtr, fragId, fragPtr.p, fragPtr.p->storedReplicas); + checkKeepGci(tabPtr, fragId, fragPtr.p, fragPtr.p->oldStoredReplicas); fragId++; if (fragId >= tabPtr.p->totalfragments) { jam(); From 311abf108bcc9fff9aff1a9e176ac667014019e9 Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 6 Oct 2006 16:05:46 +0200 Subject: [PATCH 2/4] ndb - bug#22893 Add checking of REDO to earlier during SR so take-over of node can be performed if it can't be restarted using logs (which btw is really weird...as it _should_ be able to use logs of other node in node group) Otherwise cluster could be started and 1 fragment on one node could not have been restored Making the cluster inconsisten, VERY BAD ndb/src/kernel/blocks/dbdih/Dbdih.hpp: Break-out methods which searches for REDO for a fragment, so it can be used earlier during SR ndb/src/kernel/blocks/dbdih/DbdihMain.cpp: Add checking of REDO to earlier during SR so take-over of node can be performed if it can't be restarted using logs (which btw is really weird...as it _should_ be able to use logs of other node in node group) --- ndb/src/kernel/blocks/dbdih/Dbdih.hpp | 2 + ndb/src/kernel/blocks/dbdih/DbdihMain.cpp | 161 ++++++++++++---------- 2 files changed, 92 insertions(+), 71 deletions(-) diff --git a/ndb/src/kernel/blocks/dbdih/Dbdih.hpp b/ndb/src/kernel/blocks/dbdih/Dbdih.hpp index 78acf1ffd19..559d13f6e4b 100644 --- a/ndb/src/kernel/blocks/dbdih/Dbdih.hpp +++ b/ndb/src/kernel/blocks/dbdih/Dbdih.hpp @@ -1044,6 +1044,8 @@ private: void removeStoredReplica(FragmentstorePtr regFragptr, ReplicaRecordPtr replicaPtr); void searchStoredReplicas(FragmentstorePtr regFragptr); + bool setup_create_replica(FragmentstorePtr, CreateReplicaRecord*, + ConstPtr); void updateNodeInfo(FragmentstorePtr regFragptr); //------------------------------------ diff --git a/ndb/src/kernel/blocks/dbdih/DbdihMain.cpp b/ndb/src/kernel/blocks/dbdih/DbdihMain.cpp index d8454271a4d..7ae7db967b2 100644 --- a/ndb/src/kernel/blocks/dbdih/DbdihMain.cpp +++ b/ndb/src/kernel/blocks/dbdih/DbdihMain.cpp @@ -8344,14 +8344,30 @@ Dbdih::resetReplicaSr(TabRecordPtr tabPtr){ resetReplicaLcp(replicaPtr.p, newestRestorableGCI); - /* ----------------------------------------------------------------- - * LINK THE REPLICA INTO THE STORED REPLICA LIST. WE WILL USE THIS - * NODE AS A STORED REPLICA. - * WE MUST FIRST LINK IT OUT OF THE LIST OF OLD STORED REPLICAS. - * --------------------------------------------------------------- */ - removeOldStoredReplica(fragPtr, replicaPtr); - linkStoredReplica(fragPtr, replicaPtr); - + /** + * Make sure we can also find REDO for restoring replica... + */ + { + CreateReplicaRecord createReplica; + ConstPtr constReplicaPtr; + constReplicaPtr.i = replicaPtr.i; + constReplicaPtr.p = replicaPtr.p; + if (setup_create_replica(fragPtr, + &createReplica, constReplicaPtr)) + { + removeOldStoredReplica(fragPtr, replicaPtr); + linkStoredReplica(fragPtr, replicaPtr); + } + else + { + infoEvent("Forcing take-over of node %d due to unsufficient REDO" + " for table %d fragment: %d", + nodePtr.i, tabPtr.i, i); + + setNodeActiveStatus(nodePtr.i, + Sysfile::NS_NotActive_NotTakenOver); + } + } } default: jam(); @@ -12282,16 +12298,75 @@ void Dbdih::removeTooNewCrashedReplicas(ReplicaRecordPtr rtnReplicaPtr) /* CHECKPOINT WITHOUT NEEDING ANY EXTRA LOGGING FACILITIES.*/ /* A MAXIMUM OF FOUR NODES IS RETRIEVED. */ /*************************************************************************/ +bool +Dbdih::setup_create_replica(FragmentstorePtr fragPtr, + CreateReplicaRecord* createReplicaPtrP, + ConstPtr replicaPtr) +{ + createReplicaPtrP->dataNodeId = replicaPtr.p->procNode; + createReplicaPtrP->replicaRec = replicaPtr.i; + + /* ----------------------------------------------------------------- */ + /* WE NEED TO SEARCH FOR A PROPER LOCAL CHECKPOINT TO USE FOR THE */ + /* SYSTEM RESTART. */ + /* ----------------------------------------------------------------- */ + Uint32 startGci; + Uint32 startLcpNo; + Uint32 stopGci = SYSFILE->newestRestorableGCI; + bool result = findStartGci(replicaPtr, + stopGci, + startGci, + startLcpNo); + if (!result) + { + jam(); + /* --------------------------------------------------------------- */ + /* WE COULD NOT FIND ANY LOCAL CHECKPOINT. THE FRAGMENT THUS DO NOT*/ + /* CONTAIN ANY VALID LOCAL CHECKPOINT. IT DOES HOWEVER CONTAIN A */ + /* VALID FRAGMENT LOG. THUS BY FIRST CREATING THE FRAGMENT AND THEN*/ + /* EXECUTING THE FRAGMENT LOG WE CAN CREATE THE FRAGMENT AS */ + /* DESIRED. THIS SHOULD ONLY OCCUR AFTER CREATING A FRAGMENT. */ + /* */ + /* TO INDICATE THAT NO LOCAL CHECKPOINT IS TO BE USED WE SET THE */ + /* LOCAL CHECKPOINT TO ZNIL. */ + /* --------------------------------------------------------------- */ + createReplicaPtrP->lcpNo = ZNIL; + } + else + { + jam(); + /* --------------------------------------------------------------- */ + /* WE FOUND A PROPER LOCAL CHECKPOINT TO RESTART FROM. */ + /* SET LOCAL CHECKPOINT ID AND LOCAL CHECKPOINT NUMBER. */ + /* --------------------------------------------------------------- */ + createReplicaPtrP->lcpNo = startLcpNo; + arrGuard(startLcpNo, MAX_LCP_STORED); + createReplicaPtrP->createLcpId = replicaPtr.p->lcpId[startLcpNo]; + }//if + + + /* ----------------------------------------------------------------- */ + /* WE HAVE EITHER FOUND A LOCAL CHECKPOINT OR WE ARE PLANNING TO */ + /* EXECUTE THE LOG FROM THE INITIAL CREATION OF THE TABLE. IN BOTH */ + /* CASES WE NEED TO FIND A SET OF LOGS THAT CAN EXECUTE SUCH THAT */ + /* WE RECOVER TO THE SYSTEM RESTART GLOBAL CHECKPOINT. */ + /* -_--------------------------------------------------------------- */ + return findLogNodes(createReplicaPtrP, fragPtr, startGci, stopGci); +} + void Dbdih::searchStoredReplicas(FragmentstorePtr fragPtr) { Uint32 nextReplicaPtrI; - ConstPtr replicaPtr; + Ptr replicaPtr; replicaPtr.i = fragPtr.p->storedReplicas; while (replicaPtr.i != RNIL) { jam(); ptrCheckGuard(replicaPtr, creplicaFileSize, replicaRecord); nextReplicaPtrI = replicaPtr.p->nextReplica; + ConstPtr constReplicaPtr; + constReplicaPtr.i = replicaPtr.i; + constReplicaPtr.p = replicaPtr.p; NodeRecordPtr nodePtr; nodePtr.i = replicaPtr.p->procNode; ptrCheckGuard(nodePtr, MAX_NDB_NODES, nodeRecord); @@ -12311,69 +12386,13 @@ void Dbdih::searchStoredReplicas(FragmentstorePtr fragPtr) createReplicaPtr.i = cnoOfCreateReplicas; ptrCheckGuard(createReplicaPtr, 4, createReplicaRecord); cnoOfCreateReplicas++; - createReplicaPtr.p->dataNodeId = replicaPtr.p->procNode; - createReplicaPtr.p->replicaRec = replicaPtr.i; - /* ----------------------------------------------------------------- */ - /* WE NEED TO SEARCH FOR A PROPER LOCAL CHECKPOINT TO USE FOR THE */ - /* SYSTEM RESTART. */ - /* ----------------------------------------------------------------- */ - Uint32 startGci; - Uint32 startLcpNo; - Uint32 stopGci = SYSFILE->newestRestorableGCI; - bool result = findStartGci(replicaPtr, - stopGci, - startGci, - startLcpNo); - if (!result) { - jam(); - /* --------------------------------------------------------------- */ - /* WE COULD NOT FIND ANY LOCAL CHECKPOINT. THE FRAGMENT THUS DO NOT*/ - /* CONTAIN ANY VALID LOCAL CHECKPOINT. IT DOES HOWEVER CONTAIN A */ - /* VALID FRAGMENT LOG. THUS BY FIRST CREATING THE FRAGMENT AND THEN*/ - /* EXECUTING THE FRAGMENT LOG WE CAN CREATE THE FRAGMENT AS */ - /* DESIRED. THIS SHOULD ONLY OCCUR AFTER CREATING A FRAGMENT. */ - /* */ - /* TO INDICATE THAT NO LOCAL CHECKPOINT IS TO BE USED WE SET THE */ - /* LOCAL CHECKPOINT TO ZNIL. */ - /* --------------------------------------------------------------- */ - createReplicaPtr.p->lcpNo = ZNIL; - } else { - jam(); - /* --------------------------------------------------------------- */ - /* WE FOUND A PROPER LOCAL CHECKPOINT TO RESTART FROM. */ - /* SET LOCAL CHECKPOINT ID AND LOCAL CHECKPOINT NUMBER. */ - /* --------------------------------------------------------------- */ - createReplicaPtr.p->lcpNo = startLcpNo; - arrGuard(startLcpNo, MAX_LCP_STORED); - createReplicaPtr.p->createLcpId = replicaPtr.p->lcpId[startLcpNo]; - }//if - - if(ERROR_INSERTED(7073) || ERROR_INSERTED(7074)){ - jam(); - nodePtr.p->nodeStatus = NodeRecord::DEAD; - } - - /* ----------------------------------------------------------------- */ - /* WE HAVE EITHER FOUND A LOCAL CHECKPOINT OR WE ARE PLANNING TO */ - /* EXECUTE THE LOG FROM THE INITIAL CREATION OF THE TABLE. IN BOTH */ - /* CASES WE NEED TO FIND A SET OF LOGS THAT CAN EXECUTE SUCH THAT */ - /* WE RECOVER TO THE SYSTEM RESTART GLOBAL CHECKPOINT. */ - /* -_--------------------------------------------------------------- */ - if (!findLogNodes(createReplicaPtr.p, fragPtr, startGci, stopGci)) { - jam(); - /* --------------------------------------------------------------- */ - /* WE WERE NOT ABLE TO FIND ANY WAY OF RESTORING THIS REPLICA. */ - /* THIS IS A POTENTIAL SYSTEM ERROR. */ - /* --------------------------------------------------------------- */ - cnoOfCreateReplicas--; - return; - }//if - - if(ERROR_INSERTED(7073) || ERROR_INSERTED(7074)){ - jam(); - nodePtr.p->nodeStatus = NodeRecord::ALIVE; - } + /** + * Should have been checked in resetReplicaSr + */ + ndbrequire(setup_create_replica(fragPtr, + createReplicaPtr.p, + constReplicaPtr)); break; } default: From 855c4e063d8c81ad3136f58df7a75bd37eca22dc Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 12 Oct 2006 14:02:48 +0200 Subject: [PATCH 3/4] ndb - bug#23210 Fix race-condition between COPY_GCIREQ (GCP) and lcpSetActiveStatusEnd Solution is _not_ to copy sysfileData from COPY_GCIREQ from "self" ndb/src/kernel/blocks/ERROR_codes.txt: Add error insert for dealying of copy sysfileData ndb/src/kernel/blocks/dbdih/DbdihMain.cpp: 1) Add error insert for delaying of sysfileData 2) Change to that master is _not_ copying sysfileData from COPY_GCIREQ as it might be updating it while COPY_GCIREQ is "in the fly" --- ndb/src/kernel/blocks/ERROR_codes.txt | 4 ++- ndb/src/kernel/blocks/dbdih/DbdihMain.cpp | 38 +++++++++++++++++++---- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/ndb/src/kernel/blocks/ERROR_codes.txt b/ndb/src/kernel/blocks/ERROR_codes.txt index d6dfcfe8587..874c128a56d 100644 --- a/ndb/src/kernel/blocks/ERROR_codes.txt +++ b/ndb/src/kernel/blocks/ERROR_codes.txt @@ -5,7 +5,7 @@ Next DBACC 3002 Next DBTUP 4014 Next DBLQH 5043 Next DBDICT 6006 -Next DBDIH 7174 +Next DBDIH 7178 Next DBTC 8038 Next CMVMI 9000 Next BACKUP 10022 @@ -66,6 +66,8 @@ Delay GCP_SAVEREQ by 10 secs 7030: Delay in GCP_PREPARE until node has completed a node failure 7031: Delay in GCP_PREPARE and die 3s later +7177: Delay copying of sysfileData in execCOPY_GCIREQ + ERROR CODES FOR TESTING NODE FAILURE, LOCAL CHECKPOINT HANDLING: ----------------------------------------------------------------- diff --git a/ndb/src/kernel/blocks/dbdih/DbdihMain.cpp b/ndb/src/kernel/blocks/dbdih/DbdihMain.cpp index 7ae7db967b2..fcb9b0fbb60 100644 --- a/ndb/src/kernel/blocks/dbdih/DbdihMain.cpp +++ b/ndb/src/kernel/blocks/dbdih/DbdihMain.cpp @@ -626,22 +626,48 @@ void Dbdih::execCOPY_GCIREQ(Signal* signal) ndbrequire(c_copyGCISlave.m_copyReason == CopyGCIReq::IDLE); ndbrequire(c_copyGCISlave.m_expectedNextWord == tstart); ndbrequire(reason != CopyGCIReq::IDLE); - + bool isdone = (tstart + CopyGCIReq::DATA_SIZE) >= Sysfile::SYSFILE_SIZE32; + + if (ERROR_INSERTED(7177)) + { + jam(); + + if (signal->getLength() == 3) + { + jam(); + goto done; + } + } + arrGuard(tstart + CopyGCIReq::DATA_SIZE, sizeof(sysfileData)/4); for(Uint32 i = 0; idata[i]; - if ((tstart + CopyGCIReq::DATA_SIZE) >= Sysfile::SYSFILE_SIZE32) { + if (ERROR_INSERTED(7177) && isMaster() && isdone) + { + sendSignalWithDelay(reference(), GSN_COPY_GCIREQ, signal, 1000, 3); + return; + } + +done: + if (isdone) + { jam(); c_copyGCISlave.m_expectedNextWord = 0; - } else { + } + else + { jam(); c_copyGCISlave.m_expectedNextWord += CopyGCIReq::DATA_SIZE; return; - }//if - - memcpy(sysfileData, cdata, sizeof(sysfileData)); + } + if (cmasterdihref != reference()) + { + jam(); + memcpy(sysfileData, cdata, sizeof(sysfileData)); + } + c_copyGCISlave.m_copyReason = reason; c_copyGCISlave.m_senderRef = signal->senderBlockRef(); c_copyGCISlave.m_senderData = copyGCI->anyData; From 2acf07aaef197fbc9d1fed490e7098a8572fc4c3 Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 12 Oct 2006 18:55:22 +0200 Subject: [PATCH 4/4] ndb - missing if for bug#21941 note this does not happen in 5.0, but i'm committing it here to keep code same between 5.0 and 5.1 ndb/src/ndbapi/NdbScanOperation.cpp: missing if for bug#21941 --- ndb/src/ndbapi/NdbScanOperation.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ndb/src/ndbapi/NdbScanOperation.cpp b/ndb/src/ndbapi/NdbScanOperation.cpp index a644efe9406..628d7551a52 100644 --- a/ndb/src/ndbapi/NdbScanOperation.cpp +++ b/ndb/src/ndbapi/NdbScanOperation.cpp @@ -678,7 +678,7 @@ void NdbScanOperation::close(bool forceSend, bool releaseOp) theNdbCon = NULL; m_transConnection = NULL; - if (tTransCon) + if (tTransCon && releaseOp) { NdbIndexScanOperation* tOp = (NdbIndexScanOperation*)this; @@ -693,7 +693,7 @@ void NdbScanOperation::close(bool forceSend, bool releaseOp) &tTransCon->m_theLastScanOperation, tOp); } - else if (releaseOp) + else { ret = tTransCon->releaseScanOperation(&tTransCon->m_firstExecutedScanOp, 0, tOp);