Date: Friday, August 25, 2006 @ 15:53:09
Author: gilles
Path: /cvsroot/carob/carob
Modified: include/ControllerPool.hpp (1.4 -> 1.5) src/ControllerPool.cpp
(1.4 -> 1.5) test/10-Connection/TestControllerPool.cpp (1.1 ->
1.2)
.refactored updateSuspectList function: simplified, commented and made it more
efficient (only 1 while loop instead of while+for)
.removed useless & non-performant function removeControllerFromSuspectList
-------------------------------------------+
include/ControllerPool.hpp | 7 --
src/ControllerPool.cpp | 84 ++++++++--------------------
test/10-Connection/TestControllerPool.cpp | 12 ----
3 files changed, 27 insertions(+), 76 deletions(-)
Index: carob/include/ControllerPool.hpp
diff -u carob/include/ControllerPool.hpp:1.4
carob/include/ControllerPool.hpp:1.5
--- carob/include/ControllerPool.hpp:1.4 Mon Jul 31 12:08:45 2006
+++ carob/include/ControllerPool.hpp Fri Aug 25 15:53:09 2006
@@ -109,13 +109,6 @@
void suspectControllerOfFailure(
ControllerInfo& controllerInfo);
/**
- * Removes the specified controller from the list of suspect controllers.
- * If the given controller is not in the list, logs a warning and returns
- * @param controller the controller to remove from the list
- */
- static void removeControllerFromSuspectList(
- const ControllerInfo& controller);
- /**
* Gives the number of controller failures since this process started
*/
static int getNumberOfFailures();
Index: carob/src/ControllerPool.cpp
diff -u carob/src/ControllerPool.cpp:1.4 carob/src/ControllerPool.cpp:1.5
--- carob/src/ControllerPool.cpp:1.4 Wed Aug 23 12:16:03 2006
+++ carob/src/ControllerPool.cpp Fri Aug 25 15:53:09 2006
@@ -114,13 +114,17 @@
return;
LockScope scLs(&suspected_controllers_CS);
- //create set of fds for select function
+ //create set of fds for polling
fd_set writableControllers;
FD_ZERO(&writableControllers);
struct timeval tv;
tv.tv_sec = 0;
tv.tv_usec = select_timeout;
- int fdMax = -1, retVal = 0;
+ // fdMax is *not* the number of fds, but the maximum value of created sockets
+ int fdMax = -1;
+ // return of polling function
+ int retVal = 0;
+ // Ping each controller and add the ping socket in the set of fds for polling
for (SuspectList::iterator iter = suspected_controllers.begin();
iter != suspected_controllers.end(); iter++)
{
@@ -152,36 +156,31 @@
// suspects
if (retVal > 0)
{
- // Anti-perfomant-but-very-sure way to remove up-again controllers from the
- // list of suspects
- bool mayHaveControllerLeftToRemove = true;
- while (mayHaveControllerLeftToRemove)
+ SuspectList::iterator iter = suspected_controllers.begin();
+ // iterates through controller list to find those that are up again
+ while (iter != suspected_controllers.end())
{
- mayHaveControllerLeftToRemove = false;
- for (SuspectList::iterator iter = suspected_controllers.begin();
- iter != suspected_controllers.end(); iter++)
+ if (FD_ISSET((*iter).pingSocketPtr->getFd(), &writableControllers))
{
- if (FD_ISSET((*iter).pingSocketPtr->getFd(), &writableControllers))
+ // send ping command, upon wich controller closes its connection with
+ // the client that issued this ping
+ (*iter).pingSocketPtr->writeJavaInt(Ping);
+ delete (*iter).pingSocketPtr; //free the ping socket
+ suspected_controllers.erase(iter);
+ if (isDebugEnabled())
{
- if (isDebugEnabled())
- {
- logDebug(fctName, L"Controller "
- + static_cast<wstring>((*iter).controllerInfo)
- + L" is up again. Removing it from the list of suspects (list
size="
- + toUserString(suspected_controllers.size())+L")");
- }
- //send ping and close socket
- (*iter).pingSocketPtr->writeJavaInt(Ping);
- // For performance, we could:
- //1.delete (*iter).pingSocketPtr;
- //2.suspected_controllers.erase(iter);
- // But we don't need perf in this part of code, so let's factorize
- // and use our dedicated function
- removeControllerFromSuspectList((*iter).controllerInfo);
- mayHaveControllerLeftToRemove = true;
- break;
+ logDebug(fctName, L"Controller " +
static_cast<wstring>(iter->controllerInfo)
+ + L" found up again. It has been removed from suspect list (new"
+ + L" list size=" + toUserString(suspected_controllers.size()) +
L")");
}
}
+ else
+ {
+ // increment iterator only if it has not been erase, because when the
+ // erase() function returns, it makes the given iterator point to the
+ // next element, immediatly following the one that has been removed
+ iter++;
+ }
}
}
}
@@ -239,37 +238,6 @@
}
/*static*/
-void AbstractControllerPool::removeControllerFromSuspectList(
- const ControllerInfo& controllerInfo)
-{
- wstring fctName(L"AbstractControllerPool::removeControllerFromSuspectList");
- LockScope scLs(&suspected_controllers_CS);
-
- for (SuspectList::iterator iter = suspected_controllers.begin();
- iter != suspected_controllers.end(); iter++)
- {
- if ((*iter).controllerInfo == controllerInfo)
- {
- // found it
- delete (*iter).pingSocketPtr; //free the ping socket
- suspected_controllers.erase(iter);
- if (isDebugEnabled())
- {
- logDebug(fctName, L"Controller " + static_cast<wstring>(controllerInfo)
- + L" has been removed from suspect list (list size="
- + toUserString(suspected_controllers.size())+L")");
- }
- return;
- }
- }
- if (isWarnEnabled())
- {
- logWarn(fctName, L"Controller " + static_cast<wstring>(controllerInfo)
- + L" is not (anymore?) in the suspect list");
- }
-}
-
-/*static*/
int AbstractControllerPool::getNumberOfFailures()
{
return controller_failure_number;
Index: carob/test/10-Connection/TestControllerPool.cpp
diff -u carob/test/10-Connection/TestControllerPool.cpp:1.1
carob/test/10-Connection/TestControllerPool.cpp:1.2
--- carob/test/10-Connection/TestControllerPool.cpp:1.1 Wed Jul 26 16:31:47 2006
+++ carob/test/10-Connection/TestControllerPool.cpp Fri Aug 25 15:53:09 2006
@@ -47,7 +47,7 @@
void TestControllerPool::testRoundRobinSequence()
{
- wstring fctName(L"TestControllerPool::testSuspectListUpdate");
+ wstring fctName(L"TestControllerPool::testRoundRobinSequence");
ControllerInfo c1(ConnectionSetup::DEFAULT_HOST, 4000);
ControllerInfo c2(ConnectionSetup::DEFAULT_HOST, 4001);
@@ -66,11 +66,6 @@
cp.suspectControllerOfFailure(c4);
CPPUNIT_ASSERT(cp.getController() == c3);
CPPUNIT_ASSERT(cp.getController() == c1);
- //clean up suspect list
- cp.removeControllerFromSuspectList(c2);
- cp.removeControllerFromSuspectList(c4);
- CPPUNIT_ASSERT(cp.getController() == c2);
- CPPUNIT_ASSERT(cp.getController() == c3);
}
void TestControllerPool::testNoMoreController()
@@ -107,11 +102,6 @@
logError(fctName, L"getController() failed (this is ok). Exception: "
+ nmce.description());
}
- //clean up suspect list
- cp.removeControllerFromSuspectList(c1);
- cp.removeControllerFromSuspectList(c2);
- cp.removeControllerFromSuspectList(c3);
- cp.removeControllerFromSuspectList(c4);
}
_______________________________________________
Carob-commits mailing list
[email protected]
https://forge.continuent.org/mailman/listinfo/carob-commits
