From 9bfed5190a93327194ed323c6d80a23956ef3c07 Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 18 May 2007 09:48:52 +0200 Subject: [PATCH 1/3] ndb - bug#28443 Make sure that data can not e left lingering in receive buffer ndb/src/common/transporter/Packer.cpp: test, make MAX_RECEIVED_SIGNALS an variable ndb/src/common/transporter/TCP_Transporter.hpp: add new method for checking of data in receive buffer ndb/src/common/transporter/TransporterRegistry.cpp: Make sure that 1) poll_TCP return true if any transporter has data in receiveBuffer (previously only if socket had data) 2) performReceive executes signals if TCP transporter has data in receiveBuffer (previously only if data was recevied) ndb/src/kernel/blocks/cmvmi/Cmvmi.cpp: test, make MAX_RECEIVED_SIGNALS an variable ndb/test/ndbapi/testNdbApi.cpp: testcase ndb/test/run-test/daily-basic-tests.txt: testcase --- ndb/src/common/transporter/Packer.cpp | 5 +++ .../common/transporter/TCP_Transporter.hpp | 4 ++ .../transporter/TransporterRegistry.cpp | 39 ++++++++++--------- ndb/src/kernel/blocks/cmvmi/Cmvmi.cpp | 17 ++++++++ ndb/test/ndbapi/testNdbApi.cpp | 34 ++++++++++++++++ ndb/test/run-test/daily-basic-tests.txt | 4 ++ 6 files changed, 85 insertions(+), 18 deletions(-) diff --git a/ndb/src/common/transporter/Packer.cpp b/ndb/src/common/transporter/Packer.cpp index 9eba335330d..503ff453e7e 100644 --- a/ndb/src/common/transporter/Packer.cpp +++ b/ndb/src/common/transporter/Packer.cpp @@ -21,7 +21,12 @@ #include #include +#ifdef ERROR_INSERT +Uint32 MAX_RECEIVED_SIGNALS = 1024; +#else #define MAX_RECEIVED_SIGNALS 1024 +#endif + Uint32 TransporterRegistry::unpack(Uint32 * readPtr, Uint32 sizeOfData, diff --git a/ndb/src/common/transporter/TCP_Transporter.hpp b/ndb/src/common/transporter/TCP_Transporter.hpp index 151ec261506..d081c6175a0 100644 --- a/ndb/src/common/transporter/TCP_Transporter.hpp +++ b/ndb/src/common/transporter/TCP_Transporter.hpp @@ -100,6 +100,10 @@ private: virtual void updateReceiveDataPtr(Uint32 bytesRead); virtual Uint32 get_free_buffer() const; + + inline bool hasReceiveData () const { + return receiveBuffer.sizeOfData > 0; + } protected: /** * Setup client/server and perform connect/accept diff --git a/ndb/src/common/transporter/TransporterRegistry.cpp b/ndb/src/common/transporter/TransporterRegistry.cpp index 3f190d16264..c0ee93a8489 100644 --- a/ndb/src/common/transporter/TransporterRegistry.cpp +++ b/ndb/src/common/transporter/TransporterRegistry.cpp @@ -807,6 +807,7 @@ TransporterRegistry::poll_OSE(Uint32 timeOutMillis) Uint32 TransporterRegistry::poll_TCP(Uint32 timeOutMillis) { + bool hasdata = false; if (false && nTCPTransporters == 0) { tcpReadSelectReply = 0; @@ -851,6 +852,7 @@ TransporterRegistry::poll_TCP(Uint32 timeOutMillis) // Put the connected transporters in the socket read-set FD_SET(socket, &tcpReadset); } + hasdata |= t->hasReceiveData(); } // The highest socket value plus one @@ -867,7 +869,7 @@ TransporterRegistry::poll_TCP(Uint32 timeOutMillis) } #endif - return tcpReadSelectReply; + return tcpReadSelectReply || hasdata; } #endif @@ -902,25 +904,26 @@ TransporterRegistry::performReceive() #endif #ifdef NDB_TCP_TRANSPORTER - if(tcpReadSelectReply > 0) + for (int i=0; igetRemoteNodeId(); - const NDB_SOCKET_TYPE socket = t->getSocket(); - if(is_connected(nodeId)){ - if(t->isConnected() && FD_ISSET(socket, &tcpReadset)) + checkJobBuffer(); + TCP_Transporter *t = theTCPTransporters[i]; + const NodeId nodeId = t->getRemoteNodeId(); + const NDB_SOCKET_TYPE socket = t->getSocket(); + if(is_connected(nodeId)){ + if(t->isConnected()) + { + if (FD_ISSET(socket, &tcpReadset)) { - const int receiveSize = t->doReceive(); - if(receiveSize > 0) - { - Uint32 * ptr; - Uint32 sz = t->getReceiveData(&ptr); - Uint32 szUsed = unpack(ptr, sz, nodeId, ioStates[nodeId]); - t->updateReceiveDataPtr(szUsed); - } + t->doReceive(); + } + + if (t->hasReceiveData()) + { + Uint32 * ptr; + Uint32 sz = t->getReceiveData(&ptr); + Uint32 szUsed = unpack(ptr, sz, nodeId, ioStates[nodeId]); + t->updateReceiveDataPtr(szUsed); } } } diff --git a/ndb/src/kernel/blocks/cmvmi/Cmvmi.cpp b/ndb/src/kernel/blocks/cmvmi/Cmvmi.cpp index 6519444c364..c5f1ba2575a 100644 --- a/ndb/src/kernel/blocks/cmvmi/Cmvmi.cpp +++ b/ndb/src/kernel/blocks/cmvmi/Cmvmi.cpp @@ -135,6 +135,7 @@ Cmvmi::~Cmvmi() #ifdef ERROR_INSERT NodeBitmask c_error_9000_nodes_mask; +extern Uint32 MAX_RECEIVED_SIGNALS; #endif void Cmvmi::execNDB_TAMPER(Signal* signal) @@ -164,6 +165,22 @@ void Cmvmi::execNDB_TAMPER(Signal* signal) kill(getpid(), SIGABRT); } #endif + +#ifdef ERROR_INSERT + if (signal->theData[0] == 9003) + { + if (MAX_RECEIVED_SIGNALS < 1024) + { + MAX_RECEIVED_SIGNALS = 1024; + } + else + { + MAX_RECEIVED_SIGNALS = rand() % 128; + } + ndbout_c("MAX_RECEIVED_SIGNALS: %d", MAX_RECEIVED_SIGNALS); + CLEAR_ERROR_INSERT_VALUE; + } +#endif }//execNDB_TAMPER() void Cmvmi::execSET_LOGLEVELORD(Signal* signal) diff --git a/ndb/test/ndbapi/testNdbApi.cpp b/ndb/test/ndbapi/testNdbApi.cpp index 3a06269f8dc..1ef8c628dd4 100644 --- a/ndb/test/ndbapi/testNdbApi.cpp +++ b/ndb/test/ndbapi/testNdbApi.cpp @@ -1131,7 +1131,36 @@ int runBug_11133(NDBT_Context* ctx, NDBT_Step* step){ return result; } +int +runBug28443(NDBT_Context* ctx, NDBT_Step* step) +{ + int result = NDBT_OK; + int records = ctx->getNumRecords(); + + NdbRestarter restarter; + restarter.insertErrorInAllNodes(9003); + + for (Uint32 i = 0; igetNumLoops(); i++) + { + HugoTransactions hugoTrans(*ctx->getTab()); + if (hugoTrans.loadTable(GETNDB(step), records, 2048) != 0) + { + result = NDBT_FAILED; + goto done; + } + if (runClearTable(ctx, step) != 0) + { + result = NDBT_FAILED; + goto done; + } + } + +done: + restarter.insertErrorInAllNodes(9003); + + return result; +} NDBT_TESTSUITE(testNdbApi); TESTCASE("MaxNdb", @@ -1212,6 +1241,11 @@ TESTCASE("Bug_11133", INITIALIZER(runBug_11133); FINALIZER(runClearTable); } +TESTCASE("Bug28443", + ""){ + INITIALIZER(runBug28443); + FINALIZER(runClearTable); +} NDBT_TESTSUITE_END(testNdbApi); int main(int argc, const char** argv){ diff --git a/ndb/test/run-test/daily-basic-tests.txt b/ndb/test/run-test/daily-basic-tests.txt index 51ee6d14f00..fffe1ac9046 100644 --- a/ndb/test/run-test/daily-basic-tests.txt +++ b/ndb/test/run-test/daily-basic-tests.txt @@ -606,6 +606,10 @@ max-time: 500 cmd: testNdbApi args: -n Bug_11133 T1 +max-time: 1000 +cmd: testNdbApi +args: -n BugBug28443 + #max-time: 500 #cmd: testInterpreter #args: T1 From a5731933a1877dafad0783a226460067bd91bb70 Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 18 May 2007 11:06:03 +0200 Subject: [PATCH 2/3] ndb - bug#28443 review comment if some tcp-transporter has data, then do select with timeout 0 ndb/src/common/transporter/TransporterRegistry.cpp: review comment if some tcp-transporter has data, then do select with timeout 0 --- .../transporter/TransporterRegistry.cpp | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/ndb/src/common/transporter/TransporterRegistry.cpp b/ndb/src/common/transporter/TransporterRegistry.cpp index c0ee93a8489..c459b5640dc 100644 --- a/ndb/src/common/transporter/TransporterRegistry.cpp +++ b/ndb/src/common/transporter/TransporterRegistry.cpp @@ -814,22 +814,6 @@ TransporterRegistry::poll_TCP(Uint32 timeOutMillis) return 0; } - struct timeval timeout; -#ifdef NDB_OSE - // Return directly if there are no TCP transporters configured - - if(timeOutMillis <= 1){ - timeout.tv_sec = 0; - timeout.tv_usec = 1025; - } else { - timeout.tv_sec = timeOutMillis / 1000; - timeout.tv_usec = (timeOutMillis % 1000) * 1000; - } -#else - timeout.tv_sec = timeOutMillis / 1000; - timeout.tv_usec = (timeOutMillis % 1000) * 1000; -#endif - NDB_SOCKET_TYPE maxSocketValue = -1; // Needed for TCP/IP connections @@ -855,6 +839,24 @@ TransporterRegistry::poll_TCP(Uint32 timeOutMillis) hasdata |= t->hasReceiveData(); } + timeOutMillis = hasdata ? 0 : timeOutMillis; + + struct timeval timeout; +#ifdef NDB_OSE + // Return directly if there are no TCP transporters configured + + if(timeOutMillis <= 1){ + timeout.tv_sec = 0; + timeout.tv_usec = 1025; + } else { + timeout.tv_sec = timeOutMillis / 1000; + timeout.tv_usec = (timeOutMillis % 1000) * 1000; + } +#else + timeout.tv_sec = timeOutMillis / 1000; + timeout.tv_usec = (timeOutMillis % 1000) * 1000; +#endif + // The highest socket value plus one maxSocketValue++; From deba58ee928000a03f51afe29b67652104fdc444 Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 18 May 2007 11:34:57 +0200 Subject: [PATCH 3/3] ndb - bug#28443 review comment 2, atleast 1 signal need for test prg ndb/src/kernel/blocks/cmvmi/Cmvmi.cpp: review comment, atleast 1 signal need --- ndb/src/kernel/blocks/cmvmi/Cmvmi.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ndb/src/kernel/blocks/cmvmi/Cmvmi.cpp b/ndb/src/kernel/blocks/cmvmi/Cmvmi.cpp index c5f1ba2575a..75a6117ce08 100644 --- a/ndb/src/kernel/blocks/cmvmi/Cmvmi.cpp +++ b/ndb/src/kernel/blocks/cmvmi/Cmvmi.cpp @@ -175,7 +175,7 @@ void Cmvmi::execNDB_TAMPER(Signal* signal) } else { - MAX_RECEIVED_SIGNALS = rand() % 128; + MAX_RECEIVED_SIGNALS = 1 + (rand() % 128); } ndbout_c("MAX_RECEIVED_SIGNALS: %d", MAX_RECEIVED_SIGNALS); CLEAR_ERROR_INSERT_VALUE;