From 2307886efebee4874c6606879623ad75f9f34d34 Mon Sep 17 00:00:00 2001
From: Charles Hsu <charles0126@gmail.com>
Date: Fri, 13 Apr 2018 15:08:39 +0800
Subject: [PATCH] Truncate error logs to prevent log handler runs out of the
 buffer.

Co-Authored-By: Matthew Oliver <matt@oliver.net.au>

Change-Id: If063a12cac74b67078b6db1c4f489160a2a69de1
Closes-Bug: 1763557
---
 swift/obj/replicator.py          | 27 +++++++++++++++++++++------
 test/unit/obj/test_replicator.py | 25 +++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/swift/obj/replicator.py b/swift/obj/replicator.py
index 01354f987a..5add94dc8e 100644
--- a/swift/obj/replicator.py
+++ b/swift/obj/replicator.py
@@ -191,6 +191,20 @@ class ObjectReplicator(Daemon):
         policy.load_ring(self.swift_dir)
         return policy.object_ring
 
+    def _limit_rsync_log(self, line):
+        """
+        If rsync_error_log_line_length is defined then
+        limit the error to that length
+
+        :param line: rsync log line
+        :return: If enabled the line limited to rsync_error_log_line_length
+                 otherwise the initial line.
+        """
+        if self.rsync_error_log_line_length:
+            return line[:self.rsync_error_log_line_length]
+
+        return line
+
     def _rsync(self, args):
         """
         Execute the rsync binary to replicate a partition.
@@ -208,7 +222,9 @@ class ObjectReplicator(Daemon):
                 results = proc.stdout.read()
                 ret_val = proc.wait()
         except Timeout:
-            self.logger.error(_("Killing long-running rsync: %s"), str(args))
+            self.logger.error(
+                self._limit_rsync_log(
+                    _("Killing long-running rsync: %s") % str(args)))
             if proc:
                 proc.kill()
                 try:
@@ -240,11 +256,10 @@ class ObjectReplicator(Daemon):
             else:
                 self.logger.error(result)
         if ret_val:
-            error_line = _('Bad rsync return code: %(ret)d <- %(args)s') % \
-                {'args': str(args), 'ret': ret_val}
-            if self.rsync_error_log_line_length:
-                error_line = error_line[:self.rsync_error_log_line_length]
-            self.logger.error(error_line)
+            self.logger.error(
+                self._limit_rsync_log(
+                    _('Bad rsync return code: %(ret)d <- %(args)s') %
+                    {'args': str(args), 'ret': ret_val}))
         else:
             log_method = self.logger.info if results else self.logger.debug
             log_method(
diff --git a/test/unit/obj/test_replicator.py b/test/unit/obj/test_replicator.py
index 0d061b4ec9..963931610a 100644
--- a/test/unit/obj/test_replicator.py
+++ b/test/unit/obj/test_replicator.py
@@ -2106,6 +2106,31 @@ class TestObjectReplicator(unittest.TestCase):
             ])
         self.assertEqual(len(mock_procs), 2)
 
+    def test_limit_rsync_log(self):
+        def do_test(length_limit, log_line, expected):
+            self.replicator.rsync_error_log_line_length = length_limit
+            result = self.replicator._limit_rsync_log(log_line)
+            self.assertEqual(result, expected)
+
+        tests = [{'length_limit': 20,
+                  'log_line': 'a' * 20,
+                  'expected': 'a' * 20},
+                 {'length_limit': 20,
+                  'log_line': 'a' * 19,
+                  'expected': 'a' * 19},
+                 {'length_limit': 20,
+                  'log_line': 'a' * 21,
+                  'expected': 'a' * 20},
+                 {'length_limit': None,
+                  'log_line': 'a' * 50,
+                  'expected': 'a' * 50},
+                 {'length_limit': 0,
+                  'log_line': 'a' * 50,
+                  'expected': 'a' * 50}]
+
+        for params in tests:
+            do_test(**params)
+
 
 if __name__ == '__main__':
     unittest.main()