)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"aa9941c5cae618a1e03c6b728b6811ee9bcb66bb","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Tim Burke \u003ctim.burke@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2025-07-01 09:24:40 -0700"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"ssync: Raise ChunkReadError if we don\u0027t read a whole line"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If we\u0027re calling readline, we should always be getting a whole line;"},{"line_number":10,"context_line":"when we don\u0027t, it\u0027s likely because we\u0027ve only got a partial buffer from"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"6b663983_1e2611e6","line":7,"range":{"start_line":7,"start_character":13,"end_line":7,"end_character":27},"updated":"2025-07-02 17:56:08.000000000","message":"needs updating","commit_id":"98f444ef375e2d528371e8dc3205e0d54eb6e425"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"aa9941c5cae618a1e03c6b728b6811ee9bcb66bb","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If we\u0027re calling readline, we should always be getting a whole line;"},{"line_number":10,"context_line":"when we don\u0027t, it\u0027s likely because we\u0027ve only got a partial buffer from"},{"line_number":11,"context_line":"a sender that timed out and went away."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Signed-off-by: Tim Burke \u003ctim.burke@gmail.com\u003e"},{"line_number":14,"context_line":"Change-Id: Ia8febe6e9b84766d58522cf9227a947f232ae4d4"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"816ce443_b720c99d","line":11,"updated":"2025-07-02 17:56:08.000000000","message":"I\u0027m no longer convinced this is the root cause. I don\u0027t think ``wsgi.Input.readline`` won\u0027t return partial ssync protocol \u0027lines\u0027 because each ssync \u0027line\u0027 is a chunk of the chunked transfer body, which has its length declared, so wsgi would blow up if the chunk was shorter than expected.\n\nHowever, I do think the SSYNC subreq framing can be broken if the ssync receiver times out while reading, for example, a PUT subreq body while the underlying wsgi Input has read some but not all of the body. In that case the ssync receiver subreq iter cannot be drained of \u0027junk\u0027 because the iter has exited, but the ssync receiver tries to resume on the what it thinks is the next subreq! \n\nSee https://review.opendev.org/c/openstack/swift/+/953978 for a test to illustrated.","commit_id":"98f444ef375e2d528371e8dc3205e0d54eb6e425"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"826aba8bbdde4285b684122d5af04f502167d749","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"32ed8b9e_884c1cd6","updated":"2025-07-01 11:34:10.000000000","message":"LGTM\n\nmight be good to have test that covers the missing_check phase too\n\n```\ndiff --git a/test/unit/obj/test_ssync_receiver.py b/test/unit/obj/test_ssync_receiver.py\nindex 067ffacf4..4d5a7dd1d 100644\n--- a/test/unit/obj/test_ssync_receiver.py\n+++ b/test/unit/obj/test_ssync_receiver.py\n@@ -633,6 +633,20 @@ class TestReceiver(unittest.TestCase):\n             self.controller.logger.exception.assert_called_once_with(\n                 \u00273.4.5.6/sda1/1 EXCEPTION in ssync.Receiver\u0027)\n\n+    def test_MISSING_CHECK_partial_line(self):\n+        self.controller.logger \u003d mock.MagicMock()\n+        req \u003d swob.Request.blank(\n+            \u0027/sda1/1\u0027,\n+            environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027SSYNC\u0027},\n+            body\u003d\u0027:MISSING_CHECK: START\\r\\nhash no_newline\u0027\n+        )\n+        resp \u003d req.get_response(self.controller)\n+        self.assertFalse(self.body_lines(resp.body))\n+        self.assertEqual(resp.status_int, 200)\n+        self.controller.logger.error.assert_called_once_with(\n+            \"None/sda1/1 read failed in ssync.Receiver: missing_check \"\n+            \"line: only read partial line: b\u0027hash no_newline\u0027\")\n+\n     def test_MISSING_CHECK_empty_list(self):\n\n         self.controller.logger \u003d mock.MagicMock()\n         ```","commit_id":"c5c2bd9d40c75142966fc0eaa2e0366cb324481b"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"46eb574a354def99897318d3a4ac444e61b79e34","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"02560ed5_336001d5","updated":"2025-06-27 20:30:30.000000000","message":"recheck\n\ngrenade timeout","commit_id":"c5c2bd9d40c75142966fc0eaa2e0366cb324481b"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8ccefae50596b2ce203024fa0471ba467ead920e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c95d93fc_ba068b9c","updated":"2025-07-04 15:49:24.000000000","message":"@Tim WDYT about squashing this and https://review.opendev.org/c/openstack/swift/+/953978/4?usp\u003drelated-change and polishing that up?","commit_id":"98f444ef375e2d528371e8dc3205e0d54eb6e425"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"3c4279c4882686bd18dcad08b1d9fc98ae4367e0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9c436261_c1eeaffd","updated":"2025-08-04 12:12:23.000000000","message":"I have squashed this with my follow-on patch and proposed independently as \nhttps://review.opendev.org/c/openstack/swift/+/953978 ssync-receiver: terminate session if subreq read times out","commit_id":"98f444ef375e2d528371e8dc3205e0d54eb6e425"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"aa9941c5cae618a1e03c6b728b6811ee9bcb66bb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"0b0db796_9af11d13","updated":"2025-07-02 17:56:08.000000000","message":"I\u0027m on board with adding the check for newline but not convinced about the inferred cause i.e. rather than client disconnect I suspect ssync receiver timed out and then made a wrong assumption about the subreq framing being intact.","commit_id":"98f444ef375e2d528371e8dc3205e0d54eb6e425"}],"swift/obj/ssync_receiver.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ef8ab17421f44aba252b74bc01fdee286aa717f5","unresolved":true,"context_lines":[{"line_number":269,"context_line":"                self.app.client_timeout, context):"},{"line_number":270,"context_line":"            try:"},{"line_number":271,"context_line":"                line \u003d self.fp.readline(self.app.network_chunk_size)"},{"line_number":272,"context_line":"                if line and not line.endswith(b\u0027\\n\u0027):"},{"line_number":273,"context_line":"                    # We should always get a clean end-of-line; if we didn\u0027t,"},{"line_number":274,"context_line":"                    # it indicates a dropped connection and we only had a"},{"line_number":275,"context_line":"                    # partial buffer"}],"source_content_type":"text/x-python","patch_set":1,"id":"d1ee86d3_aa20bae1","line":272,"range":{"start_line":272,"start_character":16,"end_line":272,"end_character":24},"updated":"2025-07-01 12:01:09.000000000","message":"slightly off-topic: it\u0027s odd that empty line *is* ok, sometimes...\n\n* during missing_check it is an error to not receive the START but seems ok to not received the END (just terminate on an empty line)\n* similarly in the updates phase\n\nThe sender is not so forgiving when it reads a response: an empty readline always raises ``ReplicationException(\u0027Early disconnect\u0027)``","commit_id":"c5c2bd9d40c75142966fc0eaa2e0366cb324481b"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a2c26978fd91cc7369d0915c0c25f353ba28424a","unresolved":true,"context_lines":[{"line_number":269,"context_line":"                self.app.client_timeout, context):"},{"line_number":270,"context_line":"            try:"},{"line_number":271,"context_line":"                line \u003d self.fp.readline(self.app.network_chunk_size)"},{"line_number":272,"context_line":"                if line and not line.endswith(b\u0027\\n\u0027):"},{"line_number":273,"context_line":"                    # We should always get a clean end-of-line; if we didn\u0027t,"},{"line_number":274,"context_line":"                    # it indicates a dropped connection and we only had a"},{"line_number":275,"context_line":"                    # partial buffer"}],"source_content_type":"text/x-python","patch_set":1,"id":"f68e65f3_386b7de0","line":272,"range":{"start_line":272,"start_character":16,"end_line":272,"end_character":24},"in_reply_to":"d1ee86d3_aa20bae1","updated":"2025-07-01 15:51:55.000000000","message":"Yeah, I was mostly just trying to minimize test churn. Maybe those tests *should* churn...","commit_id":"c5c2bd9d40c75142966fc0eaa2e0366cb324481b"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"826aba8bbdde4285b684122d5af04f502167d749","unresolved":true,"context_lines":[{"line_number":270,"context_line":"            try:"},{"line_number":271,"context_line":"                line \u003d self.fp.readline(self.app.network_chunk_size)"},{"line_number":272,"context_line":"                if line and not line.endswith(b\u0027\\n\u0027):"},{"line_number":273,"context_line":"                    # We should always get a clean end-of-line; if we didn\u0027t,"},{"line_number":274,"context_line":"                    # it indicates a dropped connection and we only had a"},{"line_number":275,"context_line":"                    # partial buffer"},{"line_number":276,"context_line":"                    raise IOError(f\u0027only read partial line: {line!r}\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"2943420b_3ec551fd","line":273,"range":{"start_line":273,"start_character":22,"end_line":273,"end_character":62},"updated":"2025-07-01 11:34:10.000000000","message":"ok, this is a protocol feature - ssync missing_check phase always sends lines terminated with a newline, including the final line of that phase. Ditto the preamble to each subrequest in updates phase.","commit_id":"c5c2bd9d40c75142966fc0eaa2e0366cb324481b"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a2c26978fd91cc7369d0915c0c25f353ba28424a","unresolved":true,"context_lines":[{"line_number":270,"context_line":"            try:"},{"line_number":271,"context_line":"                line \u003d self.fp.readline(self.app.network_chunk_size)"},{"line_number":272,"context_line":"                if line and not line.endswith(b\u0027\\n\u0027):"},{"line_number":273,"context_line":"                    # We should always get a clean end-of-line; if we didn\u0027t,"},{"line_number":274,"context_line":"                    # it indicates a dropped connection and we only had a"},{"line_number":275,"context_line":"                    # partial buffer"},{"line_number":276,"context_line":"                    raise IOError(f\u0027only read partial line: {line!r}\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"83bfe846_af2ad064","line":273,"range":{"start_line":273,"start_character":22,"end_line":273,"end_character":62},"in_reply_to":"2943420b_3ec551fd","updated":"2025-07-01 15:51:55.000000000","message":"Yeah, I should maybe expand the context for the comment:\n\n\u003e *Everywhere we would call `readline`*, we should always get a clean end-of-line, *as we should be reading SSYNC-specific messages or HTTP request lines/headers*.","commit_id":"c5c2bd9d40c75142966fc0eaa2e0366cb324481b"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"aa9941c5cae618a1e03c6b728b6811ee9bcb66bb","unresolved":true,"context_lines":[{"line_number":271,"context_line":"                self.app.client_timeout, context):"},{"line_number":272,"context_line":"            try:"},{"line_number":273,"context_line":"                line \u003d self.fp.readline(self.app.network_chunk_size)"},{"line_number":274,"context_line":"                if line and not line.endswith(b\u0027\\n\u0027):"},{"line_number":275,"context_line":"                    # Everywhere we would call readline, we should always get"},{"line_number":276,"context_line":"                    # a clean end-of-line as we should be reading"},{"line_number":277,"context_line":"                    # SSYNC-specific messages or HTTP request lines/headers."}],"source_content_type":"text/x-python","patch_set":2,"id":"deb34814_d428d61b","line":274,"range":{"start_line":274,"start_character":28,"end_line":274,"end_character":52},"updated":"2025-07-02 17:56:08.000000000","message":"ok, so I do agree that this is not expected and should be treated as an error","commit_id":"98f444ef375e2d528371e8dc3205e0d54eb6e425"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"aa9941c5cae618a1e03c6b728b6811ee9bcb66bb","unresolved":true,"context_lines":[{"line_number":275,"context_line":"                    # Everywhere we would call readline, we should always get"},{"line_number":276,"context_line":"                    # a clean end-of-line as we should be reading"},{"line_number":277,"context_line":"                    # SSYNC-specific messages or HTTP request lines/headers."},{"line_number":278,"context_line":"                    # If we didn\u0027t, it indicates a dropped connection and we"},{"line_number":279,"context_line":"                    # only had a partial buffer"},{"line_number":280,"context_line":"                    raise SsyncClientDisconnected"},{"line_number":281,"context_line":"            except (eventlet.wsgi.ChunkReadError, IOError) as err:"},{"line_number":282,"context_line":"                raise exceptions.ChunkReadError(\u0027%s: %s\u0027 % (context, err))"}],"source_content_type":"text/x-python","patch_set":2,"id":"d07b2a55_c228fb64","line":279,"range":{"start_line":278,"start_character":20,"end_line":279,"end_character":47},"updated":"2025-07-02 17:56:08.000000000","message":"as commented on the commit message, I\u0027m not sure this is a safe inference.\nIf wsgi.Input.readline returns a value then it must at least be a complete chunk from the chunked body, so if it doesn\u0027t end with \u0027\\n\u0027 then that\u0027s either a sender bug or a framing error in the SSYNC request.\n\nGiven that, I wonder if we shouldn\u0027t raise a different exception and log it?","commit_id":"98f444ef375e2d528371e8dc3205e0d54eb6e425"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8ccefae50596b2ce203024fa0471ba467ead920e","unresolved":true,"context_lines":[{"line_number":277,"context_line":"                    # SSYNC-specific messages or HTTP request lines/headers."},{"line_number":278,"context_line":"                    # If we didn\u0027t, it indicates a dropped connection and we"},{"line_number":279,"context_line":"                    # only had a partial buffer"},{"line_number":280,"context_line":"                    raise SsyncClientDisconnected"},{"line_number":281,"context_line":"            except (eventlet.wsgi.ChunkReadError, IOError) as err:"},{"line_number":282,"context_line":"                raise exceptions.ChunkReadError(\u0027%s: %s\u0027 % (context, err))"},{"line_number":283,"context_line":"            return line"}],"source_content_type":"text/x-python","patch_set":2,"id":"30060b4d_6bd85f2b","line":280,"updated":"2025-07-04 15:49:24.000000000","message":"in my follow-on patch https://review.opendev.org/c/openstack/swift/+/953978/4?usp\u003drelated-change I change this to a ChunkReadError on the basis that I think it could only happen if the sender sent bad data.\n\nIf we only have a partial buffer for the *chunked* body, and don\u0027t receive a newline, then the eventlet.wsgi.Input won\u0027t return from readline (and eventually there will be a timeout) because it will be trying to read whatever length chunk it is expecting.\n\nThe eventlet.wsgi.Input readline would only return if it has read the expected bytes from the current chunk and reached the end of the chunked body, which implies the sender failed to send the newline in a valid chunk.","commit_id":"98f444ef375e2d528371e8dc3205e0d54eb6e425"}],"test/unit/obj/test_ssync_receiver.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"826aba8bbdde4285b684122d5af04f502167d749","unresolved":true,"context_lines":[{"line_number":1428,"context_line":"                self.body_lines(resp.body),"},{"line_number":1429,"context_line":"                [b\u0027:MISSING_CHECK: START\u0027, b\u0027:MISSING_CHECK: END\u0027])"},{"line_number":1430,"context_line":"            # Since the client (presumably) hung up, no point in sending"},{"line_number":1431,"context_line":"            # anything about the error"},{"line_number":1432,"context_line":"            self.assertEqual(resp.status_int, 200)"},{"line_number":1433,"context_line":"            self.controller.logger.error.assert_called_once_with("},{"line_number":1434,"context_line":"                \"None/device/partition read failed in ssync.Receiver: updates \""}],"source_content_type":"text/x-python","patch_set":1,"id":"f0cb2518_19b326b2","line":1431,"updated":"2025-07-01 11:34:10.000000000","message":"the comments would also be useful over in ``ssync_receiver`` line 208 where we ``except exceptions.ChunkReadError`` - that\u0027s the only exception for which the receiver doesn\u0027t bother sending some kind of error back to the sender","commit_id":"c5c2bd9d40c75142966fc0eaa2e0366cb324481b"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a2c26978fd91cc7369d0915c0c25f353ba28424a","unresolved":true,"context_lines":[{"line_number":1428,"context_line":"                self.body_lines(resp.body),"},{"line_number":1429,"context_line":"                [b\u0027:MISSING_CHECK: START\u0027, b\u0027:MISSING_CHECK: END\u0027])"},{"line_number":1430,"context_line":"            # Since the client (presumably) hung up, no point in sending"},{"line_number":1431,"context_line":"            # anything about the error"},{"line_number":1432,"context_line":"            self.assertEqual(resp.status_int, 200)"},{"line_number":1433,"context_line":"            self.controller.logger.error.assert_called_once_with("},{"line_number":1434,"context_line":"                \"None/device/partition read failed in ssync.Receiver: updates \""}],"source_content_type":"text/x-python","patch_set":1,"id":"d856be57_3fe3c1b4","line":1431,"in_reply_to":"f0cb2518_19b326b2","updated":"2025-07-01 15:51:55.000000000","message":"Hmm... maybe I should be raising `SsyncClientDisconnected` instead...","commit_id":"c5c2bd9d40c75142966fc0eaa2e0366cb324481b"}]}