[trafficserver] branch 7.1.x updated: Fix logging log file roll issue (#2544).

[zwoop]

This is an automated email from the ASF dual-hosted git repository.

zwoop pushed a commit to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/7.1.x by this push:
     new c123afd  Fix logging log file roll issue (#2544).
c123afd is described below

commit c123afdb0b6ced2f065c52eb4121803b405056c7
Author: Peter Chou <pbc...@labs.att.com>
AuthorDate: Thu Oct 4 22:08:31 2018 -0700

    Fix logging log file roll issue (#2544).
    
    Perform the BaseLogFile::close_file() operation when rolling log files 
generated by the logging system.
    Do not perform this operation for diagnostic log files (this was removed by 
commit 346b419).
    
    (cherry picked from commit 67148468717b7116019a991d9c0f41a892bdbe05)
---
 proxy/logging/LogFile.cc | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/proxy/logging/LogFile.cc b/proxy/logging/LogFile.cc
index e6a5bb6..45450f8 100644
--- a/proxy/logging/LogFile.cc
+++ b/proxy/logging/LogFile.cc
@@ -255,7 +255,20 @@ int
 LogFile::roll(long interval_start, long interval_end)
 {
   if (m_log) {
-    return m_log->roll(interval_start, interval_end);
+    // Due to commit 346b419 the BaseLogFile::close_file() is no longer called 
within BaseLogFile::roll().
+    // For diagnostic log files, the rolling is implemented by renaming and 
destroying the BaseLogFile object
+    // and then creating a new BaseLogFile object with the original filename. 
Due to possible race conditions
+    // the old/new object swap happens within lock/unlock calls within 
Diags.cc.
+    // For logging log files, the rolling is implemented by renaming the 
original file and closing it.
+    // Afterwards, the LogFile object will re-open a new file with the 
original file name using the original object.
+    // There is no need to protect against contention since the 
open/close/writes are all executed under a
+    // single log flush thread.
+    // Since these two methods of using BaseLogFile are not compatible, we 
perform the logging log file specific
+    // close file operation here within the containing LogFile object.
+    if (m_log->roll(interval_start, interval_end)) {
+      m_log->close_file();
+      return 1;
+    }
   }
 
   return 0;