Rev 4080: Small improvement to write buffering of HPSS v3 messages. (Andrew in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

[Canonical.com Patch Queue Manager]

At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 4080
revision-id: pqm at pqm.ubuntu.com-20090305070734-voo8r5npkyz1dicx
parent: pqm at pqm.ubuntu.com-20090305033030-065loicjpjmivxq1
parent: andrew.bennetts at canonical.com-20090305062402-p39a6z042tj4bgv2
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2009-03-05 07:07:34 +0000
message:
  Small improvement to write buffering of HPSS v3 messages. (Andrew
  	Bennetts)
modified:
  bzrlib/smart/protocol.py       protocol.py-20061108035435-ot0lstk2590yqhzr-1
  bzrlib/tests/test_smart_transport.py test_ssh_transport.py-20060608202016-c25gvf1ob7ypbus6-2
    ------------------------------------------------------------
    revno: 4078.1.2
    revision-id: andrew.bennetts at canonical.com-20090305062402-p39a6z042tj4bgv2
    parent: andrew.bennetts at canonical.com-20090305052009-ii3h1zileevhqhps
    committer: Andrew Bennetts <andrew.bennetts at canonical.com>
    branch nick: hpss-buffering
    timestamp: Thu 2009-03-05 17:24:02 +1100
    message:
      Adjust write buffering tests for improved buffering.
    modified:
      bzrlib/tests/test_smart_transport.py test_ssh_transport.py-20060608202016-c25gvf1ob7ypbus6-2
    ------------------------------------------------------------
    revno: 4078.1.1
    revision-id: andrew.bennetts at canonical.com-20090305052009-ii3h1zileevhqhps
    parent: pqm at pqm.ubuntu.com-20090304163710-r7hhqdi9f3jsbe5g
    committer: Andrew Bennetts <andrew.bennetts at canonical.com>
    branch nick: hpss-buffering
    timestamp: Thu 2009-03-05 16:20:09 +1100
    message:
      Slightly better buffering logic when generating protocol v3 messages, especially during streamed bodies.
    modified:
      bzrlib/smart/protocol.py       protocol.py-20061108035435-ot0lstk2590yqhzr-1
=== modified file 'bzrlib/smart/protocol.py'
--- a/bzrlib/smart/protocol.py	2009-03-02 04:22:53 +0000
+++ b/bzrlib/smart/protocol.py	2009-03-05 05:20:09 +0000
@@ -1060,16 +1060,18 @@
     response_marker = request_marker = MESSAGE_VERSION_THREE
 
     def __init__(self, write_func):
-        self._buf = ''
+        self._buf = []
         self._real_write_func = write_func
 
     def _write_func(self, bytes):
-        self._buf += bytes
+        self._buf.append(bytes)
+        if len(self._buf) > 100:
+            self.flush()
 
     def flush(self):
         if self._buf:
-            self._real_write_func(self._buf)
-            self._buf = ''
+            self._real_write_func(''.join(self._buf))
+            del self._buf[:]
 
     def _serialise_offsets(self, offsets):
         """Serialise a readv offset list."""
@@ -1165,8 +1167,11 @@
                     self._write_structure(error_struct)
                     break
                 else:
+                    if isinstance(chunk, request.FailedSmartServerResponse):
+                        self._write_error_status()
+                        self._write_structure(chunk.args)
+                        break
                     self._write_prefixed_body(chunk)
-                    self.flush()
         self._write_end()
 
 

=== modified file 'bzrlib/tests/test_smart_transport.py'
--- a/bzrlib/tests/test_smart_transport.py	2009-03-02 04:15:11 +0000
+++ b/bzrlib/tests/test_smart_transport.py	2009-03-05 06:24:02 +0000
@@ -2873,17 +2873,30 @@
         self.responder.send_response(response)
         self.assertWriteCount(1)
 
-    def test_send_response_with_body_stream_writes_once_per_chunk(self):
-        """A normal response with a stream body is written to the medium
-        writes to the medium once per chunk.
-        """
+    def test_send_response_with_body_stream_buffers_writes(self):
+        """A normal response with a stream body writes to the medium once."""
         # Construct a response with stream with 2 chunks in it.
         response = _mod_request.SuccessfulSmartServerResponse(
             ('arg', 'arg'), body_stream=['chunk1', 'chunk2'])
         self.responder.send_response(response)
-        # We will write 3 times: exactly once for each chunk, plus a final
-        # write to end the response.
-        self.assertWriteCount(3)
+        # We will write just once, despite the multiple chunks, due to
+        # buffering.
+        self.assertWriteCount(1)
+
+    def test_send_response_with_body_stream_flushes_buffers_sometimes(self):
+        """When there are many chunks (>100), multiple writes will occur rather
+        than buffering indefinitely.
+        """
+        # Construct a response with stream with 40 chunks in it.  Every chunk
+        # triggers 3 buffered writes, so we expect > 100 buffered writes, but <
+        # 200.
+        body_stream = ['chunk %d' % count for count in range(40)]
+        response = _mod_request.SuccessfulSmartServerResponse(
+            ('arg', 'arg'), body_stream=body_stream)
+        self.responder.send_response(response)
+        # The write buffer is flushed every 100 buffered writes, so we expect 2
+        # actual writes.
+        self.assertWriteCount(2)
 
 
 class TestSmartClientUnicode(tests.TestCase):