Hi Canh,
Ack with minor comments.
Regards, Vu
> -----Original Message-----
> From: Canh Van Truong <canh.v.tru...@dektech.com.au>
> Sent: Thursday, May 30, 2019 3:10 PM
> To: lennart.l...@ericsson.com; vu.m.ngu...@dektech.com.au
> Cc: opensaf-devel@lists.sourceforge.net; Canh Van Truong
> <canh.v.tru...@dektech.com.au>
> Subject: [PATCH 1/1] log: fix to remove the cfg file when log file
rotation
> [#3045]
>
> If the date(include year, month and day) in oldest cfg file name and the
> date of created oldest log file is different, logd cannot find out the
> oldest cfg file. The oldest cfg file is never removed when log file
rotation.
> So number of cfg file is huge after time while the log file has already
been
> rotated.
>
> Update to remove the cfg in this case.
> Also limit the number of cfg files should be removed if there are still
huge
> cfg files in disk to avoid hanging main thread
> ---
> src/log/logd/lgs_filehdl.cc | 8 ++--
> src/log/logd/lgs_stream.cc | 96
++++++++++++++++++++-----------------------
> --
> 2 files changed, 47 insertions(+), 57 deletions(-)
>
> diff --git a/src/log/logd/lgs_filehdl.cc b/src/log/logd/lgs_filehdl.cc
> index be2b95e1f..f31e45883 100644
> --- a/src/log/logd/lgs_filehdl.cc
> +++ b/src/log/logd/lgs_filehdl.cc
> @@ -794,10 +794,10 @@ int get_number_of_cfg_files_hdl(void *indata,
> void *outdata,
> }
> }
>
> - if ((old_ind != -1) && (cfg_old_date == log_old_date) &&
> - (cfg_old_time <= log_old_time)) {
> - TRACE_1(" (cfg_old_date:%d == log_old_date:%d) &&"
> - " (cfg_old_time:%d <= log_old_time:%d )",
> + if ((old_ind != -1) && (((cfg_old_date == log_old_date) &&
> + (cfg_old_time <= log_old_time)) || (cfg_old_date <
log_old_date))) {
[Vu] Can simplify these if by:
If (old_ind == -1) goto done_cfg_free;
bool is_cfg_outdate_1 = (cfg_old_date == log_old_date) && (cfg_old_time <=
log_old_time);
bool is_cfg_outdate_2 = (cfg_old_date < log_old_date);
if (is_cfg_outdate_1 || is_cfg_outdate_2) {
> + TRACE_1(" (cfg_old_date:%d - log_old_date:%d) -"
> + " (cfg_old_time:%d - log_old_time:%d )",
> cfg_old_date, log_old_date, cfg_old_time, log_old_time);
> TRACE_1("oldest: %s", cfg_namelist[old_ind]->d_name);
> n = snprintf(oldest_file, max_outsize, "%s/%s", path.c_str(),
> diff --git a/src/log/logd/lgs_stream.cc b/src/log/logd/lgs_stream.cc
> index d31ae170b..cb9a935e7 100644
> --- a/src/log/logd/lgs_stream.cc
> +++ b/src/log/logd/lgs_stream.cc
> @@ -270,69 +270,59 @@ static int delete_config_file(log_stream_t
> *stream) {
>
> /**
> * Remove oldest log file until there are 'maxFilesRotated' - 1 files
left
> + * The oldest cfg files are also removed
> *
> * @param stream
> - * @return -1 on error
> + * @return true/false
> */
> -static int rotate_if_needed(log_stream_t *stream) {
> +static bool rotate_if_needed(log_stream_t *stream) {
> char oldest_log_file[PATH_MAX];
> char oldest_cfg_file[PATH_MAX];
> - int rc = 0;
> - int log_file_cnt, cfg_file_cnt;
> - bool oldest_cfg = false;
> + const int max_files_rotated =
static_cast<int>(stream->maxFilesRotated);
>
> TRACE_ENTER();
>
> - /* Rotate out log files from previous lifes */
> - if ((log_file_cnt = get_number_of_log_files_h(stream, oldest_log_file))
==
> - -1) {
> - rc = -1;
> - goto done;
> - }
> -
> - /* Rotate out cfg files from previous lifes */
> - if (!((cfg_file_cnt = get_number_of_cfg_files_h(stream,
oldest_cfg_file)) ==
> - -1)) {
> - oldest_cfg = true;
> - }
> -
> - TRACE("delete oldest_cfg_file: %s oldest_log_file %s", oldest_cfg_file,
> - oldest_log_file);
> -
> - /*
> - ** Remove until we have one less than allowed, we are just about to
> - ** create a new one again.
> - */
> - while (log_file_cnt >= static_cast<int>(stream->maxFilesRotated)) {
> - if ((rc = file_unlink_h(oldest_log_file)) == -1) {
> - LOG_NO("Could not log delete: %s - %s", oldest_log_file,
> strerror(errno));
> - goto done;
> + // Get number of log files and the oldest log file
> + int log_file_cnt = get_number_of_log_files_h(stream, oldest_log_file);
> + while (log_file_cnt >= max_files_rotated) {
> + TRACE("Delete oldest_log_file %s", oldest_log_file);
> + if (file_unlink_h(oldest_log_file) == -1) {
> + LOG_NO("Delete log file fail: %s - %s", oldest_log_file,
strerror(errno));
> + return false;
> }
> -
> - if (oldest_cfg == true) {
> - oldest_cfg = false;
> - if ((rc = file_unlink_h(oldest_cfg_file)) == -1) {
> - LOG_NO("Could not cfg delete: %s - %s", oldest_cfg_file,
> - strerror(errno));
> - goto done;
> - }
> - }
> -
> - if ((log_file_cnt = get_number_of_log_files_h(stream,
oldest_log_file)) ==
> - -1) {
> - rc = -1;
> - goto done;
> + log_file_cnt = get_number_of_log_files_h(stream, oldest_log_file);
> + }
> + if (log_file_cnt == -1) return false;
> +
> + // Housekeeping for cfg files
> + int number_deleted_files = 0;
> + int cfg_file_cnt = get_number_of_cfg_files_h(stream, oldest_cfg_file);
> + while (cfg_file_cnt >= max_files_rotated) {
> + TRACE("Delete oldest_cfg_file %s", oldest_cfg_file);
> + if (file_unlink_h(oldest_cfg_file) == -1) {
> + LOG_NO("Delete cfg file fail: %s - %s", oldest_cfg_file,
strerror(errno));
> + return false;
> }
> -
> - if (!((cfg_file_cnt = get_number_of_cfg_files_h(stream,
oldest_cfg_file))
> ==
> - -1)) {
> - oldest_cfg = true;
> + ++number_deleted_files;
> + cfg_file_cnt = get_number_of_cfg_files_h(stream, oldest_cfg_file);
> +
> + // If there is too much cfg files that the rotation hasn't deleted
them
> + // in previous, lgs should limit the deleting to avoid main thread is
hung
> + // due to the deleting huge cfg files will take long times.
> + // The workaround here is that hard-code to delete max 100 cfg files
> + // in one time. Next rotation will continue to delete them
> + // It is useful when upgrading system and there are huge cfg files in
disk
> + if (number_deleted_files > 100) {
> + LOG_NO("There are huges cfg files. Limit num of deleting cfg
file(100)");
> + break;
> }
> }
>
> -done:
> - TRACE_LEAVE2("rc = %d", rc);
> - return rc;
> + TRACE_LEAVE();
> + if (cfg_file_cnt == -1)
> + return false;
> + else
> + return true;
[Vu] return (cfg_file_cnt != -1);
> }
>
> /**
> @@ -362,7 +352,7 @@ void log_initiate_stream_files(log_stream_t *stream)
> {
> (void)delete_config_file(stream);
>
> /* Remove files from a previous life if needed */
> - if (rotate_if_needed(stream) == -1) {
> + if (rotate_if_needed(stream) == false) {
> TRACE("%s - rotate_if_needed() FAIL", __FUNCTION__);
> goto done;
> }
> @@ -1111,7 +1101,7 @@ int log_rotation_stb(log_stream_t *stream) {
> }
>
> // Remove oldest file if needed
> - if (rotate_if_needed(stream) == -1) {
> + if (rotate_if_needed(stream) == false) {
> TRACE("Old file removal failed");
> }
> // Save new name for current log file and open it
> @@ -1180,7 +1170,7 @@ int log_rotation_act(log_stream_t *stream) {
> // Reset file size for current log file
> stream->curFileSize = 0;
> // Remove oldest file if needed
> - if (rotate_if_needed(stream) == -1)
> + if (rotate_if_needed(stream) == false)
> TRACE("Old file removal failed");
>
> // Create a new file name that includes "open time stamp" and open the
file
> --
> 2.15.1
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
