[ 
https://issues.apache.org/jira/browse/TS-306?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14682132#comment-14682132
 ] 

ASF GitHub Bot commented on TS-306:
-----------------------------------

Github user SolidWallOfCode commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/274#discussion_r36772493
  
    --- Diff: lib/ts/BaseLogFile.cc ---
    @@ -0,0 +1,571 @@
    +/** @file
    +
    +  Base class for log files implementation
    +
    +  @section license License
    +
    +  Licensed to the Apache Software Foundation (ASF) under one
    +  or more contributor license agreements.  See the NOTICE file
    +  distributed with this work for additional information
    +  regarding copyright ownership.  The ASF licenses this file
    +  to you under the Apache License, Version 2.0 (the
    +  "License"); you may not use this file except in compliance
    +  with the License.  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    + */
    +
    +#include "BaseLogFile.h"
    +
    +/*
    + * This consturctor creates a BaseLogFile based on a given name.
    + * This is the most common way BaseLogFiles are created.
    + */
    +BaseLogFile::BaseLogFile(const char *name)
    +  : m_name(ats_strdup(name)), m_hostname(NULL), m_is_regfile(false), 
m_has_signature(false), m_signature(0), m_is_init(false)
    +{
    +  m_fp = NULL;
    +  m_start_time = time(0);
    +  m_end_time = 0L;
    +  m_bytes_written = 0;
    +  m_meta_info = NULL;
    +
    +  log_log_trace("exiting BaseLogFile constructor, m_name=%s, this=%p\n", 
m_name, this);
    +}
    +
    +/*
    + * This consturctor creates a BaseLogFile based on a given name.
    + * Similar to above constructor, but is overloaded with the object 
signature
    + */
    +BaseLogFile::BaseLogFile(const char *name, uint64_t sig)
    +  : m_name(ats_strdup(name)), m_hostname(NULL), m_is_regfile(false), 
m_has_signature(true), m_signature(sig), m_is_init(false)
    +{
    +  m_fp = NULL;
    +  m_start_time = time(0);
    +  m_end_time = 0L;
    +  m_bytes_written = 0;
    +  m_meta_info = NULL;
    +
    +  log_log_trace("exiting BaseLogFile constructor, m_name=%s, this=%p\n", 
m_name, this);
    +}
    +
    +/*
    + * This copy constructor creates a BaseLogFile based on a given copy.
    + */
    +BaseLogFile::BaseLogFile(const BaseLogFile &copy)
    +  : m_fp(NULL), m_start_time(copy.m_start_time), m_end_time(0L), 
m_bytes_written(0), m_name(ats_strdup(copy.m_name)),
    +    m_hostname(ats_strdup(m_hostname)), m_is_regfile(false), 
m_has_signature(copy.m_has_signature), m_signature(copy.m_signature),
    +    m_is_init(copy.m_is_init), m_meta_info(NULL)
    +{
    +  log_log_trace("exiting BaseLogFile copy constructor, m_name=%s, 
this=%p\n", m_name, this);
    +}
    +
    +/*
    + * Destructor.
    + */
    +BaseLogFile::~BaseLogFile()
    +{
    +  log_log_trace("entering BaseLogFile destructor, m_name=%s, this=%p\n", 
m_name, this);
    +
    +  if (m_is_regfile)
    +    close_file();
    +  else
    +    log_log_trace("not a regular file, not closing, m_name=%s, this=%p\n", 
m_name, this);
    +  if (m_name)
    +    ats_free(m_name);
    +  if (m_hostname)
    +    ats_free(m_hostname);
    +
    +  log_log_trace("exiting BaseLogFile destructor, this=%p\n", this);
    +}
    +
    +/*
    + * This function is called by a client of BaseLogFile to roll the 
underlying
    + * file  The tricky part to this routine is in coming up with the new file 
name,
    + * which contains the bounding timestamp interval for the entries
    + * within the file.
    +
    + * Under normal operating conditions, this BaseLogFile object was in 
existence
    + * for all writes to the file.  In this case, the LogFile members 
m_start_time
    + * and m_end_time will have the starting and ending times for the actual
    + * entries written to the file.
    +
    + * On restart situations, it is possible to re-open an existing 
BaseLogFile,
    + * which means that the m_start_time variable will be later than the actual
    + * entries recorded in the file.  In this case, we'll use the creation time
    + * of the file, which should be recorded in the meta-information located on
    + * disk.
    +
    + * If we can't use the meta-file, either because it's not there or because
    + * it's not valid, then we'll use timestamp 0 (Jan 1, 1970) as the starting
    + * bound.
    +
    + * Return 1 if file rolled, 0 otherwise
    + */
    +int
    +BaseLogFile::roll(long interval_start, long interval_end)
    +{
    +  // First, let's see if a roll is even needed.
    +  if (m_name == NULL || !BaseLogFile::exists(m_name)) {
    +    log_log_trace("Roll not needed for %s; file doesn't exist\n", (m_name) 
? m_name : "no_name\n");
    +    return 0;
    +  }
    +
    +  // Then, check if this object is backing a regular file
    +  if (!m_is_regfile) {
    +    log_log_trace("Roll not needed for %s; not regular file\n", (m_name) ? 
m_name : "no_name\n");
    +    return 0;
    +  }
    +
    +  // Read meta info if needed (if file was not opened)
    +  if (!m_meta_info) {
    +    m_meta_info = new BaseMetaInfo(m_name);
    +  }
    +
    +  // Create the new file name, which consists of a timestamp and rolled
    +  // extension added to the previous file name.  The timestamp format is
    +  // ".%Y%m%d.%Hh%Mm%Ss-%Y%m%d.%Hh%Mm%Ss", where the two date/time values
    +  // represent the starting and ending times for entries in the rolled
    +  // log file.  In addition, we add the hostname.  So, the entire rolled
    +  // format is something like:
    +  //
    +  //    "squid.log.mymachine.19980712.12h00m00s-19980713.12h00m00s.old"
    +  char roll_name[LOGFILE_ROLL_MAXPATHLEN];
    +  char start_time_ext[64];
    +  char end_time_ext[64];
    +  time_t start, end;
    +
    +  // Make sure the file is closed so we don't leak any descriptors.
    +  close_file();
    +
    +  // Start with conservative values for the start and end bounds, then
    +  // try to refine.
    +  start = 0L;
    +  end = (interval_end >= m_end_time) ? interval_end : m_end_time;
    +
    +  if (m_meta_info->data_from_metafile()) {
    +    // If the metadata came from the metafile, this means that
    +    // the file was preexisting, so we can't use m_start_time for
    +    // our starting bounds.  Instead, we'll try to use the file
    +    // creation time stored in the metafile (if it's valid and we can
    +    // read it).  If all else fails, we'll use 0 for the start time.
    +    log_log_trace("in BaseLogFile::roll(..) used metadata starttime");
    +    m_meta_info->get_creation_time(&start);
    +  } else {
    +    // The logfile was not preexisting (normal case), so we'll use
    +    // earlier of the interval start time and the m_start_time.
    +    //
    +    // note that m_start_time is not the time of the first
    +    // transaction, but the time of the creation of the first log
    +    // buffer used by the file. These times may be different,
    +    // especially under light load, and using the m_start_time may
    +    // produce overlapping filenames (the problem is that we have
    +    // no easy way of keeping track of the timestamp of the first
    +    // transaction
    +    log_log_trace("in BaseLogFile::roll(..) used else math starttime\n");
    +    if (interval_start == 0)
    +      start = m_start_time;
    +    else
    +      start = (m_start_time < interval_start) ? m_start_time : 
interval_start;
    +  }
    +  log_log_trace("in BaseLogFile::roll(..), start = %ld, m_start_time = 
%ld, interval_start = %ld\n", start, m_start_time,
    +                interval_start);
    +
    +  // Now that we have our timestamp values, convert them to the proper
    +  // timestamp formats and create the rolled file name.
    +  timestamp_to_str((long)start, start_time_ext, 64);
    +  timestamp_to_str((long)end, end_time_ext, 64);
    +  snprintf(roll_name, LOGFILE_ROLL_MAXPATHLEN, "%s%s%s.%s-%s%s", m_name, 
(m_hostname ? LOGFILE_SEPARATOR_STRING : ""),
    +           (m_hostname ? m_hostname : ""), start_time_ext, end_time_ext, 
LOGFILE_ROLLED_EXTENSION);
    +
    +  // It may be possible that the file we want to roll into already
    +  // exists.  If so, then we need to add a version tag to the rolled
    +  // filename as well so that we don't clobber existing files.
    +  int version = 1;
    +  while (BaseLogFile::exists(roll_name)) {
    +    log_log_trace("The rolled file %s already exists; adding version "
    +                  "tag %d to avoid clobbering the existing file.\n",
    +                  roll_name, version);
    +    snprintf(roll_name, LOGFILE_ROLL_MAXPATHLEN, "%s%s%s.%s-%s.%d%s", 
m_name, (m_hostname ? LOGFILE_SEPARATOR_STRING : ""),
    +             (m_hostname ? m_hostname : ""), start_time_ext, end_time_ext, 
version, LOGFILE_ROLLED_EXTENSION);
    +    version++;
    --- End diff --
    
    `++version`. ALways use prefix unless you specifically need postfix.


> enable log rotation for diags.log
> ---------------------------------
>
>                 Key: TS-306
>                 URL: https://issues.apache.org/jira/browse/TS-306
>             Project: Traffic Server
>          Issue Type: Improvement
>          Components: Logging
>            Reporter: Miles Libbey
>            Assignee: Daniel Xu
>              Labels: newbie
>             Fix For: 6.1.0
>
>
> (from yahoo bug 913896)
> Original description
> by Leif Hedstrom 3 years ago at 2006-12-04 12:42
> There might be reasons why this file might get filled up, e.g. libraries used 
> by plugins producing output on STDOUT/STDERR. A few suggestions have been
> made, to somehow rotate traffic.out. One possible solution (suggested by 
> Ryan) is to use cronolog (http://cronolog.org/), which seems like a fine idea.
>               
>  
> Comment 1
>  by Joseph Rothrock  2 years ago at 2007-10-17 09:13:24
> Maybe consider rolling diags.log as well. -Feature enhancement.
>               
> Comment 2
>  by Kevin Dalley 13 months ago at 2009-03-04 15:32:18
> When traffic.out gets filled up, error.log stops filing up, even though 
> rotation is turned on. This is
> counter-intuitive.  Rotation does not control traffic.out, but a large 
> traffic.out will stop error.log from being
> written.
>               



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to