Change in vdsm[master]: No need to call receive if send is timeout

Shu Ming has uploaded a new change for review.

Change subject: No need to call receive if send is timeout
......................................................................

No need to call receive if send is timeout

In callCrabRPCFunction(), the request packet is sent to the crab server first
and then wait for the response packet from the crab server.  When the
request packet is not sent correctly, there is no need to wait for the response
packet back.  So raising exception after the send operation is timeout.

Change-Id: Ib233b24d108a986d2f9e07a66f2a8f7d8cfb221a
Signed-off-by: Shu Ming <shuming(a)linux.vnet.ibm.com&gt;
---
M vdsm/storage/remoteFileHandler.py
1 file changed, 8 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/59/7759/1

diff --git a/vdsm/storage/remoteFileHandler.py b/vdsm/storage/remoteFileHandler.py
index 622ea5d..6995c39 100644
--- a/vdsm/storage/remoteFileHandler.py
+++ b/vdsm/storage/remoteFileHandler.py
@@ -173,8 +173,13 @@
 
     def callCrabRPCFunction(self, timeout, name, *args, **kwargs):
         request = pickle.dumps((name, args, kwargs))
-        self._sendAll(pack(LENGTH_STRUCT_FMT, len(request)), timeout)
-        self._sendAll(request, timeout)
+        try:
+            self._sendAll(pack(LENGTH_STRUCT_FMT, len(request)), timeout)
+            self._sendAll(request, timeout)
+        except:
+            self.log.warn("Problem with handler send, treating as timeout",
+                    exc_info=True)
+            raise Timeout()
 
         try:
             rawLength = self._recvAll(LENGTH_STRUCT_LENGTH, timeout)
@@ -184,7 +189,7 @@
         except:
             # If for some reason the connection drops\gets out of sync we treat
             # it as a timeout so we only have one error path
-            self.log.warn("Problem with handler, treating as timeout",
+            self.log.warn("Problem with handler receive, treating as timeout",
                     exc_info=True)
             raise Timeout()
 


--
To view, visit http://gerrit.ovirt.org/7759
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib233b24d108a986d2f9e07a66f2a8f7d8cfb221a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shu Ming <shuming(a)linux.vnet.ibm.com&gt;

Itamar Heim has posted comments on this change.

Change subject: No need to call receive if send is timeout
......................................................................


Patch Set 3:

ping?

-- 
To view, visit http://gerrit.ovirt.org/7759
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib233b24d108a986d2f9e07a66f2a8f7d8cfb221a
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shu Ming <shuming(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Better Saggi <bettersaggi(a)gmail.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;
Gerrit-HasComments: No

Zhou Zheng Sheng has posted comments on this change.

Change subject: No need to call receive if send is timeout
......................................................................


Patch Set 3: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/storage/remoteFileHandler.py
Line 178:             self._sendAll(request, timeout)
Line 179:         except:
Line 180:             self.log.warn("Problem with handler send, treating as
timeout",
Line 181:                     exc_info=True)
Line 182:             raise Timeout()
We could consolidate the send operations into the following "try" statement. I
do not find difference on timeout handling logic between the added "try" and the
following try.

Even without the added "try", the execution of the function will not continue
when "Timeout" or other exception is raised from "self._sendAll()".
Line 183: 
Line 184:         try:
Line 185:             rawLength = self._recvAll(LENGTH_STRUCT_LENGTH, timeout)
Line 186: 


--
To view, visit http://gerrit.ovirt.org/7759
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib233b24d108a986d2f9e07a66f2a8f7d8cfb221a
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shu Ming <shuming(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;

Itamar Heim has abandoned this change.

Change subject: No need to call receive if send is timeout
......................................................................


Abandoned

no comment for 30 days post last ping. abandoning. please re-open if/when relevant and/or
ping reviewers directly / promote discussion about the patch in the mailing list.

-- 
To view, visit http://gerrit.ovirt.org/7759
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ib233b24d108a986d2f9e07a66f2a8f7d8cfb221a
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shu Ming <shuming(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Better Saggi <bettersaggi(a)gmail.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt;