LGTM, thanks
On Mon, Feb 24, 2014 at 11:31 AM, Hrvoje Ribicic <[email protected]> wrote: > Ack both comments. > > Interdiff: > > diff --git a/lib/cmdlib/test.py b/lib/cmdlib/test.py > index f73a40d..2b10740 100644 > --- a/lib/cmdlib/test.py > +++ b/lib/cmdlib/test.py > @@ -142,10 +142,11 @@ class LUTestDelay(NoHooksLU): > sock.settimeout(self.op.duration) > start = time.time() > (conn, _) = sock.accept() > - except socket.error, _: > + except socket.timeout, _: > # If we timed out, all is well > return False > finally: > + # Destroys the original socket, but the new connection is still > usable > socket_wrapper.Destroy() > > try: > @@ -160,7 +161,7 @@ class LUTestDelay(NoHooksLU): > conn.settimeout(time_to_go) > conn.recv(1) > # pylint: enable=E1101 > - except socket.error, _: > + except socket.timeout, _: > # A second timeout can occur if no data is sent > return False > finally: > > > > On Fri, Feb 21, 2014 at 3:51 PM, Petr Pudlák <[email protected]> wrote: > >> I'd suggest to specifically check for a timeout exception, and re-raise >> any others so that if something bad happens, the situation gets properly >> logged (instead of acting as if a timeout occurred). >> >> Also I'd add a comment documenting that the socket is closed just after >> the first connection (and the communication with the connected client is >> magically kept open). >> >> >> On Tue, Feb 18, 2014 at 3:39 PM, Hrvoje Ribicic <[email protected]> wrote: >> >>> The gnt-debug delay command could be useful as a means of acquiring >>> locks for testing purposes. In practice, to be useful it should be >>> interruptible, otherwise we risk race conditions or long delays. >>> >>> This patch follows the examples of the move-instance command and the >>> gnt-debug test-jobqueue commands, and introduces a mechanism for >>> communicating with the delay command, allowing it to be interrupted >>> early. >>> >>> Signed-off-by: Hrvoje Ribicic <[email protected]> >>> --- >>> lib/cmdlib/test.py | 68 >>> ++++++++++++++++++++++++++++++++++++++++++++++++++---- >>> 1 file changed, 64 insertions(+), 4 deletions(-) >>> >>> diff --git a/lib/cmdlib/test.py b/lib/cmdlib/test.py >>> index d623986..f73a40d 100644 >>> --- a/lib/cmdlib/test.py >>> +++ b/lib/cmdlib/test.py >>> @@ -25,6 +25,7 @@ import logging >>> import shutil >>> import socket >>> import tempfile >>> +import time >>> >>> from ganeti import compat >>> from ganeti import constants >>> @@ -127,8 +128,52 @@ class LUTestDelay(NoHooksLU): >>> self.needed_locks = {} >>> self.needed_locks[locking.LEVEL_NODE] = self.op.on_node_uuids >>> >>> - def _TestDelay(self): >>> - """Do the actual sleep. >>> + def _InterruptibleDelay(self): >>> + """Delays but provides the mechanisms necessary to interrupt the >>> delay as >>> + needed. >>> + >>> + """ >>> + socket_wrapper = TestSocketWrapper() >>> + sock, path = socket_wrapper.Create() >>> + >>> + self.Log(constants.ELOG_DELAY_TEST, (path,)) >>> + >>> + try: >>> + sock.settimeout(self.op.duration) >>> + start = time.time() >>> + (conn, _) = sock.accept() >>> + except socket.error, _: >>> + # If we timed out, all is well >>> + return False >>> + finally: >>> + socket_wrapper.Destroy() >>> + >>> + try: >>> + # Change to remaining time >>> + time_to_go = self.op.duration - (time.time() - start) >>> + self.Log(constants.ELOG_MESSAGE, >>> + "Received connection, time to go is %d" % time_to_go) >>> + if time_to_go < 0: >>> + time_to_go = 0 >>> + # pylint: disable=E1101 >>> + # Instance of '_socketobject' has no ... member >>> + conn.settimeout(time_to_go) >>> + conn.recv(1) >>> + # pylint: enable=E1101 >>> + except socket.error, _: >>> + # A second timeout can occur if no data is sent >>> + return False >>> + finally: >>> + conn.close() >>> + >>> + self.Log(constants.ELOG_MESSAGE, >>> + "Interrupted, time spent waiting: %d" % (time.time() - >>> start)) >>> + >>> + # Reaching this point means we were interrupted >>> + return True >>> + >>> + def _UninterruptibleDelay(self): >>> + """Delays without allowing interruptions. >>> >>> """ >>> if self.op.on_node_uuids: >>> @@ -140,17 +185,32 @@ class LUTestDelay(NoHooksLU): >>> if not utils.TestDelay(self.op.duration)[0]: >>> raise errors.OpExecError("Error during master delay test") >>> >>> + def _TestDelay(self): >>> + """Do the actual sleep. >>> + >>> + @rtype: bool >>> + @return: Whether the delay was interrupted >>> + >>> + """ >>> + if self.op.interruptible: >>> + return self._InterruptibleDelay() >>> + else: >>> + self._UninterruptibleDelay() >>> + return False >>> + >>> def Exec(self, feedback_fn): >>> """Execute the test delay opcode, with the wanted repetitions. >>> >>> """ >>> if self.op.repeat == 0: >>> - self._TestDelay() >>> + i = self._TestDelay() >>> else: >>> top_value = self.op.repeat - 1 >>> for i in range(self.op.repeat): >>> self.LogInfo("Test delay iteration %d/%d", i, top_value) >>> - self._TestDelay() >>> + # Break in case of interruption >>> + if self._TestDelay(): >>> + break >>> >>> >>> class LUTestJqueue(NoHooksLU): >>> -- >>> 1.9.0.rc1.175.g0b1dcb5 >>> >>> >> >
