[ 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)