You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by Igor Galić <i....@brainsware.org> on 2013/11/13 07:57:32 UTC

Re: git commit: TS-2340: fix TextLogObject log rolling


----- Original Message -----
> Updated Branches:
>   refs/heads/master e754a1d0c -> 2abdac235
> 
> 
> TS-2340: fix TextLogObject log rolling
> 
> 
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/2abdac23
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/2abdac23
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/2abdac23
> 
> Branch: refs/heads/master
> Commit: 2abdac235754fc7e60f78f3377f2d45e28ed4f34
> Parents: e754a1d
> Author: bettydramit <b1...@gmail.com>
> Authored: Tue Nov 12 20:58:25 2013 -0800
> Committer: James Peach <jp...@apache.org>
> Committed: Tue Nov 12 20:58:25 2013 -0800
> 
> ----------------------------------------------------------------------
>  CHANGES                   | 4 ++++
>  proxy/InkAPI.cc           | 1 +
>  proxy/logging/LogObject.h | 6 +++---
>  3 files changed, 8 insertions(+), 3 deletions(-)
> ----------------------------------------------------------------------
> 
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2abdac23/CHANGES
> ----------------------------------------------------------------------
> diff --git a/CHANGES b/CHANGES
> index dfeacc8..5e3c759 100644
> --- a/CHANGES
> +++ b/CHANGES
> @@ -1,6 +1,10 @@
>                                                           -*- coding: utf-8
>                                                           -*-
>  Changes with Apache Traffic Server 4.2.0
>  
> +
> +  *) [TS-2340] Fix TextLogObject log rolling.
> +   Author: bettydramit <b1...@gmail.com>
> +
>    *) [TS-2343] Remove the --schema option from Traffic Manager, and the code
>     around it.
>  
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2abdac23/proxy/InkAPI.cc
> ----------------------------------------------------------------------
> diff --git a/proxy/InkAPI.cc b/proxy/InkAPI.cc
> index f51f4c8..a4e82c2 100644
> --- a/proxy/InkAPI.cc
> +++ b/proxy/InkAPI.cc
> @@ -6758,6 +6758,7 @@ TSTextLogObjectCreate(const char *filename, int mode,
> TSTextLogObject *new_objec
>                                                (bool) mode &
>                                                TS_LOG_MODE_ADD_TIMESTAMP,
>                                                NULL,
>                                                Log::config->rolling_enabled,
> +
> Log::config->collation_preproc_threads,
>                                                Log::config->rolling_interval_sec,
>                                                Log::config->rolling_offset_hr,
>                                                Log::config->rolling_size_mb));

Why doesn't this constructor simply take the entire Log::config ?

> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2abdac23/proxy/logging/LogObject.h
> ----------------------------------------------------------------------
> diff --git a/proxy/logging/LogObject.h b/proxy/logging/LogObject.h
> index 04a1e36..06bf4c9 100644
> --- a/proxy/logging/LogObject.h
> +++ b/proxy/logging/LogObject.h
> @@ -258,9 +258,9 @@ public:
>    inkcoreapi TextLogObject(const char *name, const char *log_dir,
>                             bool timestamps, const char *header,
>                             int rolling_enabled, int flush_threads,
> -                           int rolling_interval_sec = 0,
> -                           int rolling_offset_hr = 0,
> -                           int rolling_size_mb = 0);
> +                           int rolling_interval_sec,
> +                           int rolling_offset_hr,
> +                           int rolling_size_mb);
>  
>    inkcoreapi int write(const char *format, ...) TS_PRINTFLIKE(2, 3);
>    inkcoreapi int va_write(const char *format, va_list ap);
> 
> 

-- 
Igor Galić

Tel: +43 (0) 664 886 22 883
Mail: i.galic@brainsware.org
URL: http://brainsware.org/
GPG: 8716 7A9F 989B ABD5 100F  4008 F266 55D6 2998 1641


Re: git commit: TS-2340: fix TextLogObject log rolling

Posted by James Peach <jp...@apache.org>.
On Nov 12, 2013, at 10:57 PM, Igor Galić <i....@brainsware.org> wrote:

> 
> 
> ----- Original Message -----
>> Updated Branches:
>>  refs/heads/master e754a1d0c -> 2abdac235
>> 
>> 
>> TS-2340: fix TextLogObject log rolling
>> 
[snip]
>> ----------------------------------------------------------------------
>> diff --git a/proxy/InkAPI.cc b/proxy/InkAPI.cc
>> index f51f4c8..a4e82c2 100644
>> --- a/proxy/InkAPI.cc
>> +++ b/proxy/InkAPI.cc
>> @@ -6758,6 +6758,7 @@ TSTextLogObjectCreate(const char *filename, int mode,
>> TSTextLogObject *new_objec
>>                                               (bool) mode &
>>                                               TS_LOG_MODE_ADD_TIMESTAMP,
>>                                               NULL,
>>                                               Log::config->rolling_enabled,
>> +
>> Log::config->collation_preproc_threads,
>>                                               Log::config->rolling_interval_sec,
>>                                               Log::config->rolling_offset_hr,
>>                                               Log::config->rolling_size_mb));
> 
> Why doesn't this constructor simply take the entire Log::config ?

Not sure why this was originally done like this, but IMHO this is generally a preferable pattern. By decoupling TSTextLogObject and LogConfig, you can do dependency injection for testing, and generally make it easier to use the objects in novel ways. 

J