[
https://issues.apache.org/jira/browse/TS-306?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14693812#comment-14693812
]
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_r36883442
--- Diff: lib/ts/BaseLogFile.cc ---
@@ -0,0 +1,578 @@
+/** @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_signature(0),
m_has_signature(false)
+{
+ init(name);
+ log_log_trace("exiting BaseLogFile constructor, m_name=%s, this=%p\n",
(char *)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_signature(sig), m_has_signature(true)
+{
+ init(name);
+ log_log_trace("exiting BaseLogFile signature constructor, m_name=%s,
m_signature=%ld, this=%p\n", (char *)m_name, m_signature,
+ this);
+}
+
+/*
+ * This copy constructor creates a BaseLogFile based on a given copy.
+ */
+BaseLogFile::BaseLogFile(const BaseLogFile ©)
+ : m_fp(NULL), m_start_time(copy.m_start_time), m_end_time(0L),
m_bytes_written(0), m_signature(copy.m_signature),
+ m_has_signature(copy.m_has_signature),
m_name(ats_strdup(copy.m_name)), m_hostname(ats_strdup(copy.m_hostname)),
+ m_is_regfile(false), m_is_init(copy.m_is_init), m_meta_info(NULL)
+{
+ log_log_trace("exiting BaseLogFile copy constructor, m_name=%s,
this=%p\n", (char *)m_name, this);
+}
+
+/*
+ * Destructor.
+ */
+BaseLogFile::~BaseLogFile()
+{
+ log_log_trace("entering BaseLogFile destructor, m_name=%s, this=%p\n",
(char *)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",
(char *)m_name, this);
+
+ log_log_trace("exiting BaseLogFile destructor, this=%p\n", this);
+}
+
+/*
+ * Initializes the defaults of some of the common member values of this
class
+ */
+void
+BaseLogFile::init(const char *name)
+{
+ m_fp = NULL;
+ m_start_time = time(0);
+ m_end_time = 0L;
+ m_bytes_written = 0;
+ m_name = ats_strdup(name);
+ m_hostname = NULL;
+ m_is_regfile = false;
+ m_is_init = false;
+ m_meta_info = NULL;
+}
+
+/*
+ * 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((char *)m_name)) {
+ log_log_trace("Roll not needed for %s; file doesn't exist\n", ((char
*)m_name) ? (char *)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", ((char
*)m_name) ? (char *)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((char *)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, sizeof(start_time_ext));
+ timestamp_to_str((long)end, end_time_ext, sizeof(start_time_ext));
+ snprintf(roll_name, LOGFILE_ROLL_MAXPATHLEN, "%s%s%s.%s-%s%s", (char
*)m_name,
+ ((char *)m_hostname ? LOGFILE_SEPARATOR_STRING : ""), ((char
*)m_hostname ? (char *)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",
(char *)m_name,
+ ((char *)m_hostname ? LOGFILE_SEPARATOR_STRING : ""), ((char
*)m_hostname ? (char *)m_hostname : ""), start_time_ext,
+ end_time_ext, version, LOGFILE_ROLLED_EXTENSION);
+ ++version;
+ }
+
+ // It's now safe to rename the file.
+ if (::rename((char *)m_name, roll_name) < 0) {
+ log_log_error("Traffic Server could not rename logfile %s to %s, error
%d: "
+ "%s.\n",
+ (char *)m_name, roll_name, errno, strerror(errno));
+ return 0;
+ }
+
+ // reset m_start_time
+ m_start_time = 0;
+ m_bytes_written = 0;
+
+ log_log_trace("The logfile %s was rolled to %s.\n", (char *)m_name,
roll_name);
+
+ return 1;
+}
+
+/*
+ * The more convienent rolling function. Intended use is for less
+ * critical logs such as diags.log or traffic.out, since _exact_
+ * timestamps may be less important
+ *
+ * The function calls roll(long,long) with these parameters:
+ * Start time is either 0 or creation time stored in the metafile,
+ * whichever is greater
+ *
+ * End time is the current time
+ *
+ * Returns 1 on success, 0 otherwise
+ */
+int
+BaseLogFile::roll()
+{
+ long start;
+ time_t now = time(NULL);
+
+ if (!m_meta_info || !m_meta_info->get_creation_time(&start))
+ start = 0L;
+
+ return roll(start, now);
+}
+
+
+/*
+ * This function will return true if the given filename corresponds to a
+ * rolled logfile. We make this determination based on the file extension.
+ */
+bool
+BaseLogFile::rolled_logfile(char *path)
+{
+ const int target_len = (int)strlen(LOGFILE_ROLLED_EXTENSION);
+ int len = (int)strlen(path);
+ if (len > target_len) {
+ char *str = &path[len - target_len];
+ if (!strcmp(str, LOGFILE_ROLLED_EXTENSION)) {
+ return true;
+ }
+ }
+ return false;
+}
+
+/*
+ * Returns if the provided file in 'pathname' exists or not
+ */
+bool
+BaseLogFile::exists(const char *pathname)
+{
+ ink_assert(pathname != NULL);
+ return (pathname && ::access(pathname, F_OK) == 0);
+}
+
+/*
+ * Opens the BaseLogFile and associated BaseMetaInfo on disk if it exists
+ * Returns relevant exit status
+ */
+int
+BaseLogFile::open_file(int perm)
+{
+ log_log_trace("BaseLogFile: entered open_file()\n");
+ if (is_open()) {
+ return LOG_FILE_NO_ERROR;
+ }
+
+ if (!(char *)m_name) {
+ log_log_error("BaseLogFile: m_name is NULL, aborting open_file()\n");
+ return LOG_FILE_COULD_NOT_OPEN_FILE;
+ } else if (!strcmp((char *)m_name, "stdout")) {
+ log_log_trace("BaseLogFile: stdout opened\n");
+ m_fp = stdout;
+ m_is_init = true;
+ return LOG_FILE_NO_ERROR;
+ } else if (!strcmp((char *)m_name, "stderr")) {
+ log_log_trace("BaseLogFile: stderr opened\n");
+ m_fp = stderr;
+ m_is_init = true;
+ return LOG_FILE_NO_ERROR;
+ }
+
+ // means this object is representing a real file on disk
+ m_is_regfile = true;
+
+ // Check to see if the file exists BEFORE we try to open it, since
+ // opening it will also create it.
+ bool file_exists = BaseLogFile::exists((char *)m_name);
+
+ if (file_exists) {
+ if (!m_meta_info) {
+ // This object must be fresh since it has not built its MetaInfo
+ // so we create a new MetaInfo object that will read right away
+ // (in the constructor) the corresponding metafile
+ m_meta_info = new BaseMetaInfo((char *)m_name);
+ }
+ } else {
+ // The log file does not exist, so we create a new MetaInfo object
+ // which will save itself to disk right away (in the constructor)
+ if (m_has_signature)
+ m_meta_info = new BaseMetaInfo((char *)m_name, (long)time(0),
m_signature);
+ else
+ m_meta_info = new BaseMetaInfo((char *)m_name, (long)time(0));
+ }
+
+ // open actual log file (not metainfo)
+ log_log_trace("BaseLogFile: attempting to open %s\n", (char *)m_name);
+ m_fp = fopen((char *)m_name, "a+");
+
+ // error check
+ if (!m_fp) {
+ log_log_error("Error opening log file %s: %s\n", (char *)m_name,
strerror(errno));
+ log_log_error("Actual error: %s\n", (errno == EINVAL ? "einval" :
"something else"));
+ return LOG_FILE_COULD_NOT_OPEN_FILE;
+ }
+
+ // set permissions if necessary
+ if (perm != -1) {
+ // means LogFile passed in some permissions we need to set
+ if (chmod((char *)m_name, perm) != 0)
+ log_log_error("Error changing logfile=%s permissions: %s\n", (char
*)m_name, strerror(errno));
+ }
+
+ // set m_bytes_written to force the rolling based on filesize.
+ m_bytes_written = fseek(m_fp, 0, SEEK_CUR);
+
+ log_log_trace("BaseLogFile %s is now open (fd=%d)\n", (char *)m_name,
fileno(m_fp));
+ m_is_init = true;
+ return LOG_FILE_NO_ERROR;
+}
+
+/*
+ * Closes actual log file, not metainfo
+ */
+void
+BaseLogFile::close_file()
+{
+ if (is_open()) {
+ fclose(m_fp);
+ log_log_trace("BaseLogFile %s is closed\n", (char *)m_name);
+ m_fp = NULL;
+ m_is_init = false;
+ }
+}
+
+/*
+ * Changes names of the actual log file (not metainfo)
+ */
+void
+BaseLogFile::change_name(const char *new_name)
+{
+ ats_free(m_name);
--- End diff --
This will cause a double free.
> 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)
