Zhou Zheng Sheng has posted comments on this change.
Change subject: Improvement of the GuestAgent class memory usage
......................................................................
Patch Set 4: I would prefer that you didn't submit this
(1 inline comment)
When the buffer size exceeds the threshold, ideally it should discard all
succeeding incoming data until it sees '\n' which indicates the large message
comes to an end. This patch just receives the rest data and let json parser
raise error to indicate the format is wrong and discard the invalid message
leftover.
I think the log will be a little confusing for other people if they do not read
this piece of code. We will see a log message saying the message is too big
then another log message saying the content is invalid. If the message is
larger than 2MB, we will see two log messages saying content is too big and
another log message saying the content is invalid.
To implement what I suggest we have to introduce a new variable to store the
state ("discard/normal"). It's OK if you think the current implementation is
simple and easily understood.
-1 for socket.error handling.
....................................................
File vdsm/guestIF.py
Line 299: self._handle_data(data)
Line 300: except socket.error as err:
Line 301: if err.errno == 11:
Line 302: # Nothing more to receive (Resource temporarily
unavailable).
Line 303: pass
I'd like to suggest the following style to deal with EWOULDBLOCK
while not self._stopped:
try:
data = self._sock.recv(2 ** 16)
except socket.error as err:
if err.errno == errno.EWOULDBLOCK:
pass
else:
self._handle_data(data)
Quote from http://www.python.org/dev/peps/pep-0008/#programming-recommendations
"for all try/except clauses, limit the try clause to the absolute minimum
amount of code necessary."
And here we use errno.EWOULDBLOCK instead of the magical 11, so the comment can
be removed.
Line 304:
Line 305: def _parseLine(self, line):
Line 306: args = json.loads(line.decode('utf8'))
Line 307: name = args['__name__']
--
To view, visit http://gerrit.ovirt.org/9239
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf6274bb10c9e3b80962b69c5df316f03ee21214
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Gal Hammer <[email protected]>
Gerrit-Reviewer: Peter V. Saveliev <[email protected]>
Gerrit-Reviewer: ShaoHe Feng <[email protected]>
Gerrit-Reviewer: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: Zhou Zheng Sheng <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
