Re: bk commit into 5.1 tree (lzhou:1.2548) BUG#26498

[leonard zhou]

Hi, stewart

        I have modified some code and add comments inline.  please check.
        Thanks.

BR.
Leonard.


Stewart Smith wrote:

>Hi!
>
>Comments inline...
>
>On Fri, 2007-09-14 at 22:12 +0800, lzhou@stripped wrote:
>  
>
>>diff -Nrup a/storage/ndb/include/kernel/ndb_limits.h
> b/storage/ndb/include/kernel/ndb_limits.h
>>--- a/storage/ndb/include/kernel/ndb_limits.h	2006-12-24 03:20:03 +08:00
>>+++ b/storage/ndb/include/kernel/ndb_limits.h	2007-09-14 22:12:13 +08:00
>>@@ -27,6 +27,7 @@
>> #define MAX_NDB_NODES 49
>> #define MAX_NODES     64
>> #define UNDEF_NODEGROUP 0xFFFF
>>+#define MAX_BACKUPS   0xFFFFFFFF
>> 
>> /**
>>  * MAX_API_NODES = MAX_NODES - No of NDB Nodes in use
>>    
>>
>
>ok.
>
>  
>
>>diff -Nrup a/storage/ndb/include/kernel/signaldata/BackupImpl.hpp
> b/storage/ndb/include/kernel/signaldata/BackupImpl.hpp
>>--- a/storage/ndb/include/kernel/signaldata/BackupImpl.hpp	2006-12-24 03:20:03
> +08:00
>>+++ b/storage/ndb/include/kernel/signaldata/BackupImpl.hpp	2007-09-14 22:12:13
> +08:00
>>@@ -94,7 +94,8 @@ public:
>>     FailedInsertTableList = 1346,
>>     FailedAllocateTableMem = 1347,
>>     FailedToAllocateFileRecord = 1348,
>>-    FailedToAllocateAttributeRecord = 1349
>>+    FailedToAllocateAttributeRecord = 1349,
>>+    FailedForBackupFilesAleadyExist  = 1350
>>   };
>> private:
>>   Uint32 backupId;
>>    
>>
>
>Okay.
>
>I gather in second pass implementation (as discussed in bug report)
>we'll end up having this unused as we'll pick the highest available
>backup_id to run the backup with.
>
>  
>
>>diff -Nrup a/storage/ndb/include/kernel/signaldata/BackupSignalData.hpp
> b/storage/ndb/include/kernel/signaldata/BackupSignalData.hpp
>>--- a/storage/ndb/include/kernel/signaldata/BackupSignalData.hpp	2006-12-24
> 03:20:03 +08:00
>>+++ b/storage/ndb/include/kernel/signaldata/BackupSignalData.hpp	2007-09-14
> 22:12:13 +08:00
>>@@ -35,7 +35,7 @@ class BackupReq {
>> 
>>   friend bool printBACKUP_REQ(FILE *, const Uint32 *, Uint32, Uint16);
>> public:
>>-  STATIC_CONST( SignalLength = 3 );
>>+  STATIC_CONST( SignalLength = 4 );
>> 
>> private:
>>   Uint32 senderData;
>>@@ -43,6 +43,7 @@ private:
>>   /* & 0x3 - waitCompleted
>>    */
>>   Uint32 flags;
>>+  Uint32 inputBackupId;
>> };
>>    
>>
>
>ok.
>
>  
>
>> class BackupData {
>>diff -Nrup a/storage/ndb/include/kernel/signaldata/UtilSequence.hpp
> b/storage/ndb/include/kernel/signaldata/UtilSequence.hpp
>>--- a/storage/ndb/include/kernel/signaldata/UtilSequence.hpp	2006-12-24 03:20:07
> +08:00
>>+++ b/storage/ndb/include/kernel/signaldata/UtilSequence.hpp	2007-09-14 22:12:13
> +08:00
>>@@ -33,17 +33,19 @@ class UtilSequenceReq {
>> 
>>   friend bool printUTIL_SEQUENCE_REQ(FILE *, const Uint32 *, Uint32, Uint16);
>> public:
>>-  STATIC_CONST( SignalLength = 3 );
>>+  STATIC_CONST( SignalLength = 4 );
>>   
>>   enum RequestType {
>>     NextVal = 1, // Return uniq value
>>     CurrVal = 2, // Read
>>-    Create  = 3  // Create a sequence
>>+    Create  = 3,  // Create a sequence
>>+    SetVal  = 4  // Set a new sequence
>>   };
>> private:
>>   Uint32 senderData;  
>>   Uint32 sequenceId;  // Number of sequence variable
>>   Uint32 requestType;
>>+  Uint32 backupId;
>> };
>>    
>>
>
>Don't name it backupId - name it "value" or something as it's not
>specific to BACKUP.
>  
>
Have changed backupId to "value".
############
  Uint32 requestType;
  Uint32 value;

>  
>
>> class UtilSequenceConf {
>>diff -Nrup a/storage/ndb/include/mgmapi/mgmapi.h
> b/storage/ndb/include/mgmapi/mgmapi.h
>>--- a/storage/ndb/include/mgmapi/mgmapi.h	2007-06-14 18:35:33 +08:00
>>+++ b/storage/ndb/include/mgmapi/mgmapi.h	2007-09-14 22:12:13 +08:00
>>@@ -980,13 +980,15 @@ extern "C" {
>>    *                          2:  Wait for backup to be completed
>>    * @param   backup_id       Backup ID is returned from function.
>>    * @param   reply           Reply message.
>>+   * @param   input_backupId  user input backupId.
>>    
>>
>
>s/user input backupid/run as backupId and set next backup id to
>input_backupId+1/
>  
>
have changed the comment.
############
   * @param   input_backupId  run as backupId and set next backupid to 
input_backupId+1.

>  
>
>>    * @return                  -1 on error.
>>    * @note                    backup_id will not be returned if
>>    *                          wait_completed == 0
>>    */
>>   int ndb_mgm_start_backup(NdbMgmHandle handle, int wait_completed,
>> 			   unsigned int* backup_id,
>>-			   struct ndb_mgm_reply* reply);
>>+			   struct ndb_mgm_reply* reply,
>>+			   unsigned int input_backupId = 0);
>> 
>>   /**
>>    * Abort backup
>>    
>>
>
>This should also be ndb_mgm_start_backup2 with the old calling
>convention kept for ndb_mgm_start_backup.
>
>should not have default value.
>  
>
changed to the following:
############
  int ndb_mgm_start_backup2(NdbMgmHandle handle, int wait_completed,
                           unsigned int* backup_id,
                           struct ndb_mgm_reply* reply,
                           unsigned int input_backupId);
############

>  
>
>>diff -Nrup a/storage/ndb/src/common/debugger/signaldata/UtilSequence.cpp
> b/storage/ndb/src/common/debugger/signaldata/UtilSequence.cpp
>>--- a/storage/ndb/src/common/debugger/signaldata/UtilSequence.cpp	2006-12-24
> 03:20:11 +08:00
>>+++ b/storage/ndb/src/common/debugger/signaldata/UtilSequence.cpp	2007-09-14
> 22:12:13 +08:00
>>@@ -25,6 +25,8 @@ type2string(UtilSequenceReq::RequestType
>>     return "CurrVal";
>>   case UtilSequenceReq::Create:
>>     return "Create";
>>+  case UtilSequenceReq::SetVal:
>>+    return "SetVal";
>>   default:
>>     return "Unknown";
>>   }
>>    
>>
>
>ok.
>
>  
>
>>diff -Nrup a/storage/ndb/src/kernel/blocks/backup/Backup.cpp
> b/storage/ndb/src/kernel/blocks/backup/Backup.cpp
>>--- a/storage/ndb/src/kernel/blocks/backup/Backup.cpp	2007-06-12 16:38:05 +08:00
>>+++ b/storage/ndb/src/kernel/blocks/backup/Backup.cpp	2007-09-14 22:12:13 +08:00
>>@@ -1046,6 +1046,7 @@ Backup::execBACKUP_REQ(Signal* signal)
>>   const BlockReference senderRef = signal->senderBlockRef();
>>   const Uint32 dataLen32 = req->backupDataLen; // In 32 bit words
>>   const Uint32 flags = signal->getLength() > 2 ? req->flags : 2;
>>+  const Uint32 input_backupId = signal->getLength() > 3 ?
> req->inputBackupId : 0;
>> 
>>   if(getOwnNodeId() != getMasterNodeId()) {
>>     jam();
>>@@ -1090,7 +1091,10 @@ Backup::execBACKUP_REQ(Signal* signal)
>>   ptr.p->flags = flags;
>>   ptr.p->masterRef = reference();
>>   ptr.p->nodes = c_aliveNodes;
>>-  ptr.p->backupId = 0;
>>+  if(input_backupId > 0)
>>+    ptr.p->backupId = input_backupId;
>>+  else
>>+    ptr.p->backupId = 0;
>>    
>>
>
>unneeeded with ternary operator in input_backupId assignment.
>  
>
I think we should have this ternary operator because we couldn't know if 
the api send backupid.
If user start a normal backup, then we can't get value from signal.

>  
>
>>   ptr.p->backupKey[0] = 0;
>>   ptr.p->backupKey[1] = 0;
>>   ptr.p->backupDataLen = 0;
>>@@ -1101,7 +1105,13 @@ Backup::execBACKUP_REQ(Signal* signal)
>>   ptr.p->masterData.gsn = GSN_UTIL_SEQUENCE_REQ;
>>   utilReq->senderData  = ptr.i;
>>   utilReq->sequenceId  = BACKUP_SEQUENCE;
>>-  utilReq->requestType = UtilSequenceReq::NextVal;
>>+  if(input_backupId > 0) {
>>    
>>
>
>Just if(input_backupId) is sufficient.
>  
>
changed to
############
    if(input_backupId) {

>  
>
>>+    utilReq->requestType = UtilSequenceReq::SetVal;
>>+    utilReq->backupId = input_backupId;
>>+  }
>>+  else { 
>>+    utilReq->requestType = UtilSequenceReq::NextVal;
>>+  }
>>   sendSignal(DBUTIL_REF, GSN_UTIL_SEQUENCE_REQ, 
>> 	     signal, UtilSequenceReq::SignalLength, JBB);
>> }
>>@@ -1183,11 +1193,13 @@ Backup::execUTIL_SEQUENCE_CONF(Signal* s
>>   }//if
>> 
>>
>>+  if(ptr.p->backupId <= 0 && conf->requestType !=
> UtilSequenceReq::SetVal)
>>    
>>
>
>if !ptr.p->backupId as backupIds are unsigned (right?) so never < 0.
>  
>
Changed to
###########
if(!ptr.p->backupId && conf->requestType != UtilSequenceReq::SetVal)

>  
>
>>   {
>>     Uint64 backupId;
>>     memcpy(&backupId,conf->sequenceValue,8);
>>     ptr.p->backupId= (Uint32)backupId;
>>   }
>>+
>>   ptr.p->backupKey[0] = (getOwnNodeId() << 16) | (ptr.p->backupId
> & 0xFFFF);
>>   ptr.p->backupKey[1] = NdbTick_CurrentMillisecond();
>> 
>>@@ -2768,8 +2780,7 @@ Backup::openFiles(Signal* signal, Backup
>>   req->userReference = reference();
>>   req->fileFlags = 
>>     FsOpenReq::OM_WRITEONLY | 
>>-    FsOpenReq::OM_TRUNCATE |
>>-    FsOpenReq::OM_CREATE | 
>>+    FsOpenReq::OM_CREATE_IF_NONE |
>>     FsOpenReq::OM_APPEND |
>>     FsOpenReq::OM_AUTOSYNC;
>>   FsOpenReq::v2_setCount(req->fileNumber, 0xFFFFFFFF);
>>@@ -2881,11 +2892,20 @@ Backup::openFilesReply(Signal* signal, 
>>     }//if
>>   }//for
>> 
>>+  if (ERROR_INSERTED(10037)) {
>>+    jam();
>>+    ptr.p->errorCode = DefineBackupRef::FailedForBackupFilesAleadyExist;
>>+    defineBackupRef(signal, ptr);
>>+    return;
>>+  }
>>   /**
>>    * Did open succeed for all files
>>    */
>>   if(ptr.p->checkError()) {
>>     jam();
>>+    if(ptr.p->errorCode == FsRef::fsErrFileExists)
>>+      ptr.p->errorCode = DefineBackupRef::FailedForBackupFilesAleadyExist;
>>+
>>     defineBackupRef(signal, ptr);
>>     return;
>>   }//if
>>diff -Nrup a/storage/ndb/src/kernel/blocks/dbutil/DbUtil.cpp
> b/storage/ndb/src/kernel/blocks/dbutil/DbUtil.cpp
>>--- a/storage/ndb/src/kernel/blocks/dbutil/DbUtil.cpp	2006-12-24 03:20:17 +08:00
>>+++ b/storage/ndb/src/kernel/blocks/dbutil/DbUtil.cpp	2007-09-14 22:12:13 +08:00
>>@@ -171,7 +171,7 @@ DbUtil::execREAD_CONFIG_REQ(Signal* sign
>> 
>>   c_pagePool.setSize(10);
>>   c_preparePool.setSize(1);            // one parallel prepare at a time
>>-  c_preparedOperationPool.setSize(5);  // three hardcoded, two for test
>>+  c_preparedOperationPool.setSize(6);  // three hardcoded, one for setval, two
> for test
>>   c_operationPool.setSize(64);         // 64 parallel operations
>>   c_transactionPool.setSize(32);       // 16 parallel transactions
>>   c_attrMappingPool.setSize(100);
>>@@ -1496,6 +1496,35 @@ DbUtil::hardcodedPrepare() {
>>     ptr.p->tckey.requestInfo = requestInfo;
>>     ptr.p->tckey.tableSchemaVersion = 1;
>>   }
>>+
>>+  /**
>>+   * Prepare SetSequence (UPDATE)
>>+   */
>>+  {
>>+    PreparedOperationPtr ptr;
>>+    ndbrequire(c_preparedOperationPool.seizeId(ptr, 3));
>>+    ptr.p->keyLen = 1;
>>+    ptr.p->rsLen = 0;
>>+    ptr.p->tckeyLenInBytes = (TcKeyReq::StaticLength + ptr.p->keyLen + 5) *
> 4;
>>+    ptr.p->keyDataPos = TcKeyReq::StaticLength;
>>+    ptr.p->tckey.attrLen = 9;
>>+    ptr.p->tckey.tableId = 0;
>>+    Uint32 requestInfo = 0;
>>+    TcKeyReq::setAbortOption(requestInfo, TcKeyReq::CommitIfFailFree);
>>+    TcKeyReq::setOperationType(requestInfo, ZUPDATE);
>>+    TcKeyReq::setKeyLength(requestInfo, 1);
>>+    TcKeyReq::setAIInTcKeyReq(requestInfo, 5);
>>+    TcKeyReq::setInterpretedFlag(requestInfo, 1);
>>+    ptr.p->tckey.requestInfo = requestInfo;
>>+    ptr.p->tckey.tableSchemaVersion = 1;
>>+
>>+    Uint32 * attrInfo = &ptr.p->tckey.distrGroupHashValue;
>>+    attrInfo[0] = 0; // IntialReadSize
>>+    attrInfo[1] = 3; // InterpretedSize
>>+    attrInfo[2] = 0; // FinalUpdateSize
>>+    attrInfo[3] = 1; // FinalReadSize
>>+    attrInfo[4] = 0; // SubroutineSize
>>+  }
>> }
>> 
>> void
>>@@ -1516,6 +1545,10 @@ DbUtil::execUTIL_SEQUENCE_REQ(Signal* si
>>   case UtilSequenceReq::Create:
>>     prepOp = c_preparedOperationPool.getPtr(2); //c_CreateSequence
>>     break;
>>+  case UtilSequenceReq::SetVal:{
>>+    prepOp = c_preparedOperationPool.getPtr(3);
>>+    break;
>>+  }
>>   default:
>>     ndbrequire(false);
>>     prepOp = 0; // remove warning
>>@@ -1566,6 +1599,21 @@ DbUtil::execUTIL_SEQUENCE_REQ(Signal* si
>>     * it.data = 0;
>>   }
>>   
>>+  if(req->requestType == UtilSequenceReq::SetVal)
>>+  { // AttrInfo
>>+    ndbrequire(opPtr.p->attrInfo.seize(4));
>>+    AttrInfoBuffer::DataBufferIterator it;
>>+    opPtr.p->attrInfo.first(it);
>>+    * it.data = Interpreter::LoadConst16(7, req->backupId);
>>+    ndbrequire(opPtr.p->attrInfo.next(it));
>>+    * it.data = Interpreter::Write(1, 7);
>>+    ndbrequire(opPtr.p->attrInfo.next(it))
>>+    * it.data = Interpreter::ExitOK();
>>+
>>+    ndbrequire(opPtr.p->attrInfo.next(it));
>>+    AttributeHeader::init(it.data, 1, 0);
>>+  }
>>+ 
>>   runTransaction(signal, transPtr);
>> }
>>    
>>
>
>We probably shouldn't really be using the Interpreter here (instead
>using real store)... possibly bug Jonas for the right code.
>
>  
>
Keep the Interpreter, maybe should fix together with other code.

>>@@ -1643,6 +1691,9 @@ DbUtil::reportSequence(Signal* signal, c
>>       ret->sequenceValue[1] = rsit.data[2];
>>       break;
>>     }
>>+    case UtilSequenceReq::SetVal:
>>+      ok = true;
>>+      break;
>>     case UtilSequenceReq::Create:
>>       ok = true;
>>       ret->sequenceValue[0] = 0;
>>@@ -1659,6 +1710,7 @@ DbUtil::reportSequence(Signal* signal, c
>> 
>>   switch(transP->sequence.requestType)
>>     {
>>+    case UtilSequenceReq::SetVal:
>>     case UtilSequenceReq::CurrVal:
>>     case UtilSequenceReq::NextVal:{
>>       if (transP->errorCode == 626)
>>diff -Nrup a/storage/ndb/src/mgmapi/mgmapi.cpp
> b/storage/ndb/src/mgmapi/mgmapi.cpp
>>--- a/storage/ndb/src/mgmapi/mgmapi.cpp	2007-08-29 08:51:05 +08:00
>>+++ b/storage/ndb/src/mgmapi/mgmapi.cpp	2007-09-14 22:12:13 +08:00
>>@@ -2031,7 +2031,8 @@ extern "C"
>> int 
>> ndb_mgm_start_backup(NdbMgmHandle handle, int wait_completed,
>> 		     unsigned int* _backup_id,
>>-		     struct ndb_mgm_reply* /*reply*/) 
>>+		     struct ndb_mgm_reply*, /*reply*/
>>+		     unsigned int input_backupId) 
>>    
>>
>
>As previously discussed,
>
>this should be ndb_mgm_start_backup2 and ndb_mgm_start_backup should be
>implemented by calling start_backup2.
>  
>
Changed to :
##############

extern "C"
int
ndb_mgm_start_backup2(NdbMgmHandle handle, int wait_completed,
                     unsigned int* _backup_id,
                     struct ndb_mgm_reply*, /*reply*/
                     unsigned int input_backupId)
..................
..................

extern "C"
int
ndb_mgm_start_backup(NdbMgmHandle handle, int wait_completed,
                     unsigned int* _backup_id,
                     struct ndb_mgm_reply* /*reply*/)
{
  struct ndb_mgm_reply reply;
  return ndb_mgm_start_backup2(handle, wait_completed, _backup_id, 
&reply, 0);
}

>  
>
>> {
>>   SET_ERROR(handle, NDB_MGM_NO_ERROR, "Executing: ndb_mgm_start_backup");
>>   const ParserRow<ParserDummy> start_backup_reply[] = {
>>@@ -2045,6 +2046,8 @@ ndb_mgm_start_backup(NdbMgmHandle handle
>> 
>>   Properties args;
>>   args.put("completed", wait_completed);
>>+  if(input_backupId > 0)
>>+    args.put("backupid", input_backupId);
>>   const Properties *reply;
>>   { // start backup can take some time, set timeout high
>>     Uint64 old_timeout= handle->timeout;
>>diff -Nrup a/storage/ndb/src/mgmclient/CommandInterpreter.cpp
> b/storage/ndb/src/mgmclient/CommandInterpreter.cpp
>>--- a/storage/ndb/src/mgmclient/CommandInterpreter.cpp	2007-07-30 17:28:28 +08:00
>>+++ b/storage/ndb/src/mgmclient/CommandInterpreter.cpp	2007-09-14 22:12:13 +08:00
>>@@ -260,7 +260,7 @@ static const char* helpText =
>> "SHOW CONFIG                            Print configuration\n"
>> "SHOW PARAMETERS                        Print configuration parameters\n"
>> #endif
>>-"START BACKUP [NOWAIT | WAIT STARTED | WAIT COMPLETED]\n"
>>+"START BACKUP [<backup id>] [NOWAIT | WAIT STARTED | WAIT COMPLETED]\n"
>> "                                       Start backup (default WAIT COMPLETED)\n"
>> "ABORT BACKUP <backup id>               Abort backup\n"
>> "SHUTDOWN                               Shutdown all processes in cluster\n"
>>@@ -324,12 +324,15 @@ static const char* helpTextStartBackup =
>> " NDB Cluster -- Management Client -- Help for START BACKUP command\n"
>> "---------------------------------------------------------------------------\n"
>> "START BACKUP  Start a cluster backup\n\n"
>>-"START BACKUP [NOWAIT | WAIT STARTED | WAIT COMPLETED]\n"
>>+"START BACKUP [<backup id>] [NOWAIT | WAIT STARTED | WAIT COMPLETED]\n"
>> "                   Start a backup for the cluster.\n"
>> "                   Each backup gets an ID number that is reported to the\n"
>> "                   user. This ID number can help you find the backup on the\n"
>> "                   file system, or ABORT BACKUP if you wish to cancel a \n"
>>-"                   running backup.\n\n"
>>+"                   running backup.\n"
>>+"                   You can also start specified backup using START BACKUP
> <backup id> \n\n"
>>+"                   <backup id> \n"
>>+"                     Start a specified backup using <backup id> as bakcup
> ID number.\n" 
>> "                   NOWAIT \n"
>> "                     Start a cluster backup and return immediately.\n"
>> "                     The management client will return control directly\n"
>>@@ -2604,6 +2607,7 @@ CommandInterpreter::executeStartBackup(c
>> {
>>   struct ndb_mgm_reply reply;
>>   unsigned int backupId;
>>+  unsigned int input_backupId = 0;
>> 
>>   Vector<BaseString> args;
>>   {
>>@@ -2632,6 +2636,31 @@ CommandInterpreter::executeStartBackup(c
>>     ndbout_c("Waiting for started, this may take several minutes");
>>     flags = 1;
>>   }
>>+  else if (sscanf(args[1].c_str(), "%u", &input_backupId) == 1 &&
> input_backupId > 0 && input_backupId < MAX_BACKUPS)
>>+  {
>>+    // start backup n nowait
>>+    if (sz == 3 && args[2] == "NOWAIT")
>>+    {
>>+      flags = 0;
>>+    }
>>+    // start backup n; start backup n wait complete
>>+    else if ( sz == 2 || (sz == 4 && args[2] == "WAIT" && args[3]
> =="COMPLETED"))
>>+    {
>>+      flags = 2;
>>+      ndbout_c("Waiting for completed, this may take several minutes");
>>+    }
>>+    //start backup n wait started
>>+    else if (sz == 4 && args[2] == "WAIT" && args[3] ==
> "STARTED")
>>+    {
>>+      ndbout_c("Waiting for started, this may take several minutes");
>>+      flags = 1;
>>+    }
>>+    else
>>+    {
>>+      invalid_command(parameters);
>>+      return -1;
>>+    }
>>+  }
>>    
>>
>
>shouldn't this be common code?
>  
>
Can you explain this?  I  don't know how to do it better...

>  
>
>>   else
>>   {
>>     invalid_command(parameters);
>>@@ -2651,7 +2680,10 @@ CommandInterpreter::executeStartBackup(c
>>       return -1;
>>     }
>>   }
>>-  result = ndb_mgm_start_backup(m_mgmsrv, flags, &backupId, &reply);
>>+  if (input_backupId > 0)
>>+    result = ndb_mgm_start_backup(m_mgmsrv, flags, &backupId, &reply,
> input_backupId);
>>+  else
>>+    result = ndb_mgm_start_backup(m_mgmsrv, flags, &backupId, &reply);
>> 
>>   if (result != 0) {
>>     ndbout << "Backup failed" << endl;
>>diff -Nrup a/storage/ndb/src/mgmsrv/MgmtSrvr.cpp
> b/storage/ndb/src/mgmsrv/MgmtSrvr.cpp
>>--- a/storage/ndb/src/mgmsrv/MgmtSrvr.cpp	2007-06-14 17:26:52 +08:00
>>+++ b/storage/ndb/src/mgmsrv/MgmtSrvr.cpp	2007-09-14 22:12:13 +08:00
>>@@ -2505,7 +2505,7 @@ MgmtSrvr::eventReport(const Uint32 * the
>>  ***************************************************************************/
>> 
>> int
>>-MgmtSrvr::startBackup(Uint32& backupId, int waitCompleted)
>>+MgmtSrvr::startBackup(Uint32& backupId, int waitCompleted, Uint32
> input_backupId)
>> {
>>   SignalSender ss(theFacade);
>>   ss.lock(); // lock will be released on exit
>>@@ -2530,6 +2530,8 @@ MgmtSrvr::startBackup(Uint32& backupId, 
>>   req->backupDataLen = 0;
>>   assert(waitCompleted < 3);
>>   req->flags = waitCompleted & 0x3;
>>+  if(input_backupId > 0)
>>+    req->inputBackupId = input_backupId;
>> 
>>   BackupEvent event;
>>   int do_send = 1;
>>    
>>
>
>Should only send longer signal if backup id is specified. This means
>we'll most certainly be sending the correct signal for older nodes.
>
>  
>
Changed to :
#################
  if(input_backupId > 0)
  {
    ssig.set(ss, TestOrd::TraceAPI, BACKUP, GSN_BACKUP_REQ,
             BackupReq::SignalLength);
    req->inputBackupId = input_backupId;
  }
  else
    ssig.set(ss, TestOrd::TraceAPI, BACKUP, GSN_BACKUP_REQ,
             BackupReq::SignalLength - 1);


>>diff -Nrup a/storage/ndb/src/mgmsrv/MgmtSrvr.hpp
> b/storage/ndb/src/mgmsrv/MgmtSrvr.hpp
>>--- a/storage/ndb/src/mgmsrv/MgmtSrvr.hpp	2007-06-14 17:26:52 +08:00
>>+++ b/storage/ndb/src/mgmsrv/MgmtSrvr.hpp	2007-09-14 22:12:13 +08:00
>>@@ -307,7 +307,7 @@ public:
>>   /**
>>    * Backup functionallity
>>    */
>>-  int startBackup(Uint32& backupId, int waitCompleted= 2);
>>+  int startBackup(Uint32& backupId, int waitCompleted= 2, Uint32
> input_backupId= 0);
>>   int abortBackup(Uint32 backupId);
>>   int performBackup(Uint32* backupId);
>> 
>>diff -Nrup a/storage/ndb/src/mgmsrv/Services.cpp
> b/storage/ndb/src/mgmsrv/Services.cpp
>>--- a/storage/ndb/src/mgmsrv/Services.cpp	2007-07-11 20:36:40 +08:00
>>+++ b/storage/ndb/src/mgmsrv/Services.cpp	2007-09-14 22:12:13 +08:00
>>@@ -189,6 +189,7 @@ ParserRow<MgmApiSession> commands[] = {
>> 
>>   MGM_CMD("start backup", &MgmApiSession::startBackup, ""),
>>     MGM_ARG("completed", Int, Optional ,"Wait until completed"),
>>+    MGM_ARG("backupid", Int, Optional ,"User input backup id"),
>> 
>>   MGM_CMD("abort backup", &MgmApiSession::abortBackup, ""),
>>     MGM_ARG("id", Int, Mandatory, "Backup id"),
>>@@ -690,12 +691,19 @@ MgmApiSession::startBackup(Parser<MgmApi
>> 			   Properties const &args) {
>>   DBUG_ENTER("MgmApiSession::startBackup");
>>   unsigned backupId;
>>+  unsigned input_backupId;
>>   Uint32 completed= 2;
>>   int result;
>> 
>>   args.get("completed", &completed);
>> 
>>-  result = m_mgmsrv.startBackup(backupId, completed);
>>+  if(args.contains("backupid"))
>>+  {
>>+    args.get("backupid", &input_backupId);
>>+    result = m_mgmsrv.startBackup(backupId, completed, input_backupId);
>>+  }
>>+  else
>>+    result = m_mgmsrv.startBackup(backupId, completed);
>> 
>>   m_output->println("start backup reply");
>>   if(result != 0)
>>diff -Nrup a/storage/ndb/src/ndbapi/ndberror.c
> b/storage/ndb/src/ndbapi/ndberror.c
>>--- a/storage/ndb/src/ndbapi/ndberror.c	2007-06-14 17:26:52 +08:00
>>+++ b/storage/ndb/src/ndbapi/ndberror.c	2007-09-14 22:12:13 +08:00
>>@@ -465,6 +465,7 @@ ErrorBundle ErrorCodes[] = {
>>   { 1347, DMEC, AE, "Backup failed to allocate table memory (check
> configuration)" },
>>   { 1348, DMEC, AE, "Backup failed to allocate file record (check configuration)"
> },
>>   { 1349, DMEC, AE, "Backup failed to allocate attribute record (check
> configuration)" },
>>+  { 1350, DMEC, TR, "Backup failed: file already exists (use 'START BACKUP
> <backup id>')" },
>>   { 1329, DMEC, AE, "Backup during software upgrade not supported" },
>> 
>>   /**
>>diff -Nrup a/storage/ndb/test/include/NDBT_Test.hpp
> b/storage/ndb/test/include/NDBT_Test.hpp
>>--- a/storage/ndb/test/include/NDBT_Test.hpp	2007-03-01 17:39:45 +08:00
>>+++ b/storage/ndb/test/include/NDBT_Test.hpp	2007-09-14 22:12:14 +08:00
>>@@ -17,6 +17,7 @@
>> #define NDBT_TEST_HPP
>> 
>> #include <ndb_global.h>
>>+#include <kernel/ndb_limits.h>
>> 
>> #include "NDBT_ReturnCodes.h"
>> #include <Properties.hpp>
>>diff -Nrup a/storage/ndb/test/ndbapi/testBackup.cpp
> b/storage/ndb/test/ndbapi/testBackup.cpp
>>--- a/storage/ndb/test/ndbapi/testBackup.cpp	2006-12-24 03:20:26 +08:00
>>+++ b/storage/ndb/test/ndbapi/testBackup.cpp	2007-09-14 22:12:14 +08:00
>>@@ -137,6 +137,19 @@ int runBackupOne(NDBT_Context* ctx, NDBT
>>   return NDBT_OK;
>> }
>> 
>>+int runBackupRandom(NDBT_Context* ctx, NDBT_Step* step){
>>+  NdbBackup backup(GETNDB(step)->getNodeId()+1);
>>+  unsigned backupId = rand() % (MAX_BACKUPS);
>>+
>>+  if (backup.start(backupId) == -1){
>>+    return NDBT_FAILED;
>>+  }
>>+  ndbout << "Started backup " << backupId << endl;
>>+  ctx->setProperty("BackupId", backupId);
>>+
>>+  return NDBT_OK;
>>+}
>>+
>> int
>> runBackupLoop(NDBT_Context* ctx, NDBT_Step* step){
>>   NdbBackup backup(GETNDB(step)->getNodeId()+1);
>>@@ -470,6 +483,20 @@ TESTCASE("BackupOne", 
>> 	 "5. Verify count and content of table\n"){
>>   INITIALIZER(runLoadTable);
>>   INITIALIZER(runBackupOne);
>>+  INITIALIZER(runDropTablesRestart);
>>+  INITIALIZER(runRestoreOne);
>>+  VERIFIER(runVerifyOne);
>>+  FINALIZER(runClearTable);
>>+}
>>+TESTCASE("BackupRandom", 
>>+	 "Test that backup n and restore works on one table \n"
>>+	 "1. Load table\n"
>>+	 "2. Backup\n"
>>+	 "3. Drop tables and restart \n"
>>+	 "4. Restore\n"
>>+	 "5. Verify count and content of table\n"){
>>+  INITIALIZER(runLoadTable);
>>+  INITIALIZER(runBackupRandom);
>>   INITIALIZER(runDropTablesRestart);
>>   INITIALIZER(runRestoreOne);
>>   VERIFIER(runVerifyOne);
>>diff -Nrup a/storage/ndb/test/src/NdbBackup.cpp
> b/storage/ndb/test/src/NdbBackup.cpp
>>--- a/storage/ndb/test/src/NdbBackup.cpp	2006-12-24 03:20:31 +08:00
>>+++ b/storage/ndb/test/src/NdbBackup.cpp	2007-09-14 22:12:14 +08:00
>>@@ -350,7 +350,8 @@ FailS_codes[] = {
>>   10025,
>>   10027,
>>   10033,
>>-  10035
>>+  10035,
>>+  10037
>> };
>> 
>> int
>>    
>>
>
>  
>


-- 
Leonard (Li) Zhou, Software Engineer, Beijing Team
MySQL AB, www.mysql.com
Office: 86-10-65054020 Ext: 299