You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@kudu.apache.org by "yejiabao (Code Review)" <ge...@cloudera.org> on 2022/05/22 13:36:43 UTC

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

yejiabao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18554


Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This path support compress the rotated log files,
use 'FLAGS_max_compressed_log_files' to retain the max number
of compressed log files,
use 'FLAGS_max_all_compressed_files_size_mb' to retain the
max size of all compressed files

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
10 files changed, 275 insertions(+), 96 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/1
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 1
Gerrit-Owner: yejiabao <ye...@huawei.com>

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#6).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This patch adds support for compressing rotated log files.
Use --max_compressed_log_files to control the
maximum number of compressed log files to keep.
Use --max_all_compressed_files_size_mb to control the
total size of all compressed files under a Kudu server.

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/client/symbols.map
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
11 files changed, 294 insertions(+), 100 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/6
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 6
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#4).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This path support compress the rotated log files,
use 'FLAGS_max_compressed_log_files' to retain the max number
of compressed log files,
use 'FLAGS_max_all_compressed_files_size_mb' to retain the
max size of all compressed files

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/client/symbols.map
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
11 files changed, 279 insertions(+), 97 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/4
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 4
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#21).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This patch adds support for compressing rotated log files.
Use --max_compressed_log_files to control the
maximum number of compressed log files to keep.
Use --max_all_compressed_files_size_mb to control the
total size of all compressed files under a Kudu server.

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/client/symbols.map
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
11 files changed, 299 insertions(+), 100 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/21
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 21
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: yejiabao <ye...@huawei.com>

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.

Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18554 )

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................


Patch Set 19:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/env_util.h
File src/kudu/util/env_util.h:

http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/env_util.h@101
PS19, Line 101: max_all_compressed_files_size
What's "max_all_compressed_files_size" in this context?


http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/env_util.h@104
PS19, Line 104: int
Use size_t for the size (or ssize_t/int64_t if it's signed).  Also, what unit is used for the size?


http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/env_util.h@129
PS19, Line 129: Compress the given path
nit: it's not possible to compress a path, but rather compress a file at the specified path


http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/env_util.cc
File src/kudu/util/env_util.cc:

http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/env_util.cc@353
PS19, Line 353: matching_file_mtimes.insert({mtime, {matching_file_path, file_size}});
What if mtime is the same for multiple files?


http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/logging.cc
File src/kudu/util/logging.cc:

http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/logging.cc@80
PS19, Line 80: Maximum size of all compressed log files retained (in MB).
Is 0 a special value here?  If so, then please describe the behavior in such a case.

Is it possible to have negative value for this flag with some meaningful behavior?  If not, maybe change the type of the flag to uint32?


http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/logging.cc@81
PS19, Line 81: TAG_FLAG(max_all_compressed_log_files_size_mb, stable);
             : TAG_FLAG(max_all_compressed_log_files_size_mb, experimental);
A flag cannot be both 'stable' and 'experimental'.  I guess you meant it to be just 'experimental'?


http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/logging.cc@414
PS19, Line 414:   int32_t max_compressed_log_files = FLAGS_max_compressed_log_files;
nit: add 'const' since this variable isn't changing in the whole scope of this method


http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/logging.cc@427
PS19, Line 427: int
Use either int64 or size_t to avoid integer overflow here.  Also, add 'const' since this isn't going to change for this scope.


http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/logging.cc@431
PS19, Line 431:   RETURN_NOT_OK(env_util::DeleteExcessFilesByPattern(env, pattern, max_compressed_log_files));
              :   return Status::OK();
nit: shorten these lines to

  return env_util::DeleteExcessFilesByPattern(...);



-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 19
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: yejiabao <ye...@huawei.com>
Gerrit-Comment-Date: Tue, 11 Oct 2022 03:06:06 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#11).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This patch adds support for compressing rotated log files.
Use --max_compressed_log_files to control the
maximum number of compressed log files to keep.
Use --max_all_compressed_files_size_mb to control the
total size of all compressed files under a Kudu server.

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/client/symbols.map
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
11 files changed, 295 insertions(+), 100 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/11
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 11
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: yejiabao <ye...@huawei.com>

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#5).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This patch adds support for compressing rotated log files.
Use --max_compressed_log_files to control the
maximum number of compressed log files to keep.
Use --max_all_compressed_files_size_mb to control the
total size of all compressed files under a Kudu server.

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/client/symbols.map
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
11 files changed, 294 insertions(+), 100 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/5
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 5
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#17).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This patch adds support for compressing rotated log files.
Use --max_compressed_log_files to control the
maximum number of compressed log files to keep.
Use --max_all_compressed_files_size_mb to control the
total size of all compressed files under a Kudu server.

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/client/symbols.map
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
11 files changed, 296 insertions(+), 100 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/17
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 17
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: yejiabao <ye...@huawei.com>

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#22).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This patch adds support for compressing rotated log files.
Use --max_compressed_log_files to control the
maximum number of compressed log files to keep.
Use --max_all_compressed_files_size_mb to control the
total size of all compressed files under a Kudu server.

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/client/symbols.map
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
11 files changed, 300 insertions(+), 100 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/22
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 22
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: yejiabao <ye...@huawei.com>

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.

Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18554 )

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................


Patch Set 4:

(22 comments)

Thank you for working on this!

I did a quick review: overall this looks OK to me, but there are a few high-level questions and some nits.

http://gerrit.cloudera.org:8080/#/c/18554/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18554/4//COMMIT_MSG@9
PS4, Line 9: This path support compress the rotated log files
how about:

This patch adds support for compressing rotated log files.


http://gerrit.cloudera.org:8080/#/c/18554/4//COMMIT_MSG@10
PS4, Line 10: use 'FLAGS_max_compressed_log_files' to retain the max number
            : of compressed log files,
how about:

Use --max_compressed_log_files to control the maximum number of compressed log files to keep.


http://gerrit.cloudera.org:8080/#/c/18554/4//COMMIT_MSG@12
PS4, Line 12: use 'FLAGS_max_all_compressed_files_size_mb' to retain the
            : max size of all compressed files
how about:

Use --max_all_compressed_files_size_mb to control the total size of all compressed files under a Kudu server.


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:

http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/server/server_base.cc@843
PS4, Line 843: compressd
compressed


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/env_util.cc
File src/kudu/util/env_util.cc:

http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/env_util.cc@33
PS4, Line 33: #include <zlib.h>
move this into the section with gflags.h and logging.h


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/env_util.cc@57
PS4, Line 57: reserve
Why reserve?  Is there something reserved upfront?


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/env_util.cc@57
PS4, Line 57: files
log files


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/env_util.cc@58
PS4, Line 58: stable
I guess it's too early to declare this stable, isn't it?  Usually, the 'stable' status is given to flag that have been there for at least one release and it's already clear that it's not going to change in the future.


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/env_util.cc@322
PS4, Line 322: uint64_t all_matching_files_size = 0;
With computations below at line 335, there is a risk of underlow on an unsigned integer: consider using a signed type here instead.


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/env_util.cc@323
PS4, Line 323:  
nit: an extra space (not needed with contemporary C++ compilers)


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/env_util.cc@334
PS4, Line 334: FLAGS_max_all_compressed_files_size_mb
To follow the suite with other functions like this, maybe pass the threshold on the total files size as a parameter for this function?


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/env_util.cc@335
PS4, Line 335: matching_file_mtimes.begin()
Maybe, introduce a variable in this scope?


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/env_util.cc@396
PS4, Line 396: string
nit: add 'const' since this variable is not changing in this scope


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.h
File src/kudu/util/logging.h:

http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.h@316
PS4, Line 316: // Deletes excess compressed log files
nit: add a period (.) in the end of the sentence


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.h@318
PS4, Line 318: keeps
Keeps


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.cc
File src/kudu/util/logging.cc:

http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.cc@73
PS4, Line 73: max_compressed_log_files
BTW, max_log_files are per severity level.  Does it make sense to keep the similar semantics for compressed files as well, given there is also a threshold on the total size of all compressed files?

What do you think?


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.cc@74
PS4, Line 74: level
levels


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.cc@74
PS4, Line 74: all
among all


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.cc@75
PS4, Line 75: ,
The change the comma to a period (.)?


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.cc@75
PS4, Line 75: all log files
all compressed log files


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.cc@77
PS4, Line 77: stable
Usually, a flag gets the 'stable' tag after a while (maybe, at least one release).  I guess at this point it's not yet clear whether it's going to be changes in there in the nearest future.


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.cc@411
PS4, Line 411: Ignore bad input
You could make the flag to be uint32_t to avoid this.  0 can be still a special value.



-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 4
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 01 Jun 2022 18:44:59 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

yejiabao has posted comments on this change. ( http://gerrit.cloudera.org:8080/18554 )

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................


Patch Set 19:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/env_util.h
File src/kudu/util/env_util.h:

http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/env_util.h@101
PS19, Line 101: max_all_compressed_files_size
> What's "max_all_compressed_files_size" in this context?
Done


http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/env_util.h@104
PS19, Line 104: int
> Use size_t for the size (or ssize_t/int64_t if it's signed).  Also, what un
Done


http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/env_util.h@129
PS19, Line 129: Compress the given path
> nit: it's not possible to compress a path, but rather compress a file at th
Done


http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/env_util.cc
File src/kudu/util/env_util.cc:

http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/env_util.cc@353
PS19, Line 353: matching_file_mtimes.insert({mtime, {matching_file_path, file_size}});
> What if mtime is the same for multiple files?
Done


http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/logging.cc
File src/kudu/util/logging.cc:

http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/logging.cc@80
PS19, Line 80: Maximum size of all compressed log files retained (in MB).
> Is 0 a special value here?  If so, then please describe the behavior in suc
Done


http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/logging.cc@81
PS19, Line 81: TAG_FLAG(max_all_compressed_log_files_size_mb, stable);
             : TAG_FLAG(max_all_compressed_log_files_size_mb, experimental);
> A flag cannot be both 'stable' and 'experimental'.  I guess you meant it to
Done


http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/logging.cc@414
PS19, Line 414:   int32_t max_compressed_log_files = FLAGS_max_compressed_log_files;
> nit: add 'const' since this variable isn't changing in the whole scope of t
Done


http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/logging.cc@427
PS19, Line 427: int
> Use either int64 or size_t to avoid integer overflow here.  Also, add 'cons
Done


http://gerrit.cloudera.org:8080/#/c/18554/19/src/kudu/util/logging.cc@431
PS19, Line 431:   RETURN_NOT_OK(env_util::DeleteExcessFilesByPattern(env, pattern, max_compressed_log_files));
              :   return Status::OK();
> nit: shorten these lines to
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 19
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: yejiabao <ye...@huawei.com>
Gerrit-Comment-Date: Sun, 16 Oct 2022 07:45:18 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#18).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This patch adds support for compressing rotated log files.
Use --max_compressed_log_files to control the
maximum number of compressed log files to keep.
Use --max_all_compressed_files_size_mb to control the
total size of all compressed files under a Kudu server.

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/client/symbols.map
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
11 files changed, 296 insertions(+), 100 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/18
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 18
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: yejiabao <ye...@huawei.com>

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#10).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This patch adds support for compressing rotated log files.
Use --max_compressed_log_files to control the
maximum number of compressed log files to keep.
Use --max_all_compressed_files_size_mb to control the
total size of all compressed files under a Kudu server.

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/client/symbols.map
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
11 files changed, 295 insertions(+), 100 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/10
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 10
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: yejiabao <ye...@huawei.com>

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#19).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This patch adds support for compressing rotated log files.
Use --max_compressed_log_files to control the
maximum number of compressed log files to keep.
Use --max_all_compressed_files_size_mb to control the
total size of all compressed files under a Kudu server.

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/client/symbols.map
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
11 files changed, 296 insertions(+), 100 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/19
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 19
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: yejiabao <ye...@huawei.com>

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#3).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This path support compress the rotated log files,
use 'FLAGS_max_compressed_log_files' to retain the max number
of compressed log files,
use 'FLAGS_max_all_compressed_files_size_mb' to retain the
max size of all compressed files

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
10 files changed, 277 insertions(+), 96 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/3
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 3
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#2).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This path support compress the rotated log files,
use 'FLAGS_max_compressed_log_files' to retain the max number
of compressed log files,
use 'FLAGS_max_all_compressed_files_size_mb' to retain the
max size of all compressed files

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
10 files changed, 276 insertions(+), 96 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/2
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 2
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#16).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This patch adds support for compressing rotated log files.
Use --max_compressed_log_files to control the
maximum number of compressed log files to keep.
Use --max_all_compressed_files_size_mb to control the
total size of all compressed files under a Kudu server.

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/client/symbols.map
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
11 files changed, 296 insertions(+), 100 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/16
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 16
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: yejiabao <ye...@huawei.com>

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#23).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This patch adds support for compressing rotated log files.
Use --max_compressed_log_files to control the
maximum number of compressed log files to keep.
Use --max_all_compressed_files_size_mb to control the
total size of all compressed files under a Kudu server.

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/client/symbols.map
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
11 files changed, 300 insertions(+), 100 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/23
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 23
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: yejiabao <ye...@huawei.com>

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#20).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This patch adds support for compressing rotated log files.
Use --max_compressed_log_files to control the
maximum number of compressed log files to keep.
Use --max_all_compressed_files_size_mb to control the
total size of all compressed files under a Kudu server.

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/client/symbols.map
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
11 files changed, 299 insertions(+), 100 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/20
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 20
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: yejiabao <ye...@huawei.com>

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#8).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This patch adds support for compressing rotated log files.
Use --max_compressed_log_files to control the
maximum number of compressed log files to keep.
Use --max_all_compressed_files_size_mb to control the
total size of all compressed files under a Kudu server.

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/client/symbols.map
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
11 files changed, 294 insertions(+), 100 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/8
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 8
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

yejiabao has posted comments on this change. ( http://gerrit.cloudera.org:8080/18554 )

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................


Patch Set 4:

(17 comments)

http://gerrit.cloudera.org:8080/#/c/18554/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18554/4//COMMIT_MSG@9
PS4, Line 9: This path support compress the rotated log files
> how about:
Done


http://gerrit.cloudera.org:8080/#/c/18554/4//COMMIT_MSG@10
PS4, Line 10: use 'FLAGS_max_compressed_log_files' to retain the max number
            : of compressed log files,
> how about:
Done


http://gerrit.cloudera.org:8080/#/c/18554/4//COMMIT_MSG@12
PS4, Line 12: use 'FLAGS_max_all_compressed_files_size_mb' to retain the
            : max size of all compressed files
> how about:
Done


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:

http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/server/server_base.cc@843
PS4, Line 843: compressd
> compressed
Done


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/env_util.cc
File src/kudu/util/env_util.cc:

http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/env_util.cc@33
PS4, Line 33: #include <zlib.h>
> move this into the section with gflags.h and logging.h
Done


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/env_util.cc@57
PS4, Line 57: files
> log files
Done


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/env_util.cc@57
PS4, Line 57: reserve
> Why reserve?  Is there something reserved upfront?
Done


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/env_util.cc@58
PS4, Line 58: stable
> I guess it's too early to declare this stable, isn't it?  Usually, the 'sta
Done


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.h
File src/kudu/util/logging.h:

http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.h@316
PS4, Line 316: // Deletes excess compressed log files
> nit: add a period (.) in the end of the sentence
Done


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.h@318
PS4, Line 318: keeps
> Keeps
Done


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.cc
File src/kudu/util/logging.cc:

http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.cc@73
PS4, Line 73: max_compressed_log_files
> BTW, max_log_files are per severity level.  Does it make sense to keep the 
max_compressed_log_files controls the number of all compressed log files, max_log_files controls the number of log files by log level
for example?
kudu server sets the following parameters
max_log_files=1
max_all_compressed_files_size_mb=600
max_compressed_log_files=800
max_log_size=20
According to this setting, there will be only one log file for each level of logs. When the log size reaches 20MB, it will create a new log, and old log file will be compressed. When the number of all compressed files reaches 800 or the size of all compressed files reaches 600MB, delete the oldest compressed files

if the kudu server is restarted repeatedly, a large number of small compressed log files will be generated. In this scenario, the threshold of the total size will not be reached, but the threshold of the number of files in the system will be reached. so i think it make sense to keep the similar semantics for compressed files as well


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.cc@74
PS4, Line 74: all
> among all
Done


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.cc@74
PS4, Line 74: level
> levels
Done


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.cc@75
PS4, Line 75: ,
> The change the comma to a period (.)?
Done


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.cc@75
PS4, Line 75: all log files
> all compressed log files
Done


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.cc@77
PS4, Line 77: stable
> Usually, a flag gets the 'stable' tag after a while (maybe, at least one re
Done


http://gerrit.cloudera.org:8080/#/c/18554/4/src/kudu/util/logging.cc@411
PS4, Line 411: Ignore bad input
> You could make the flag to be uint32_t to avoid this.  0 can be still a spe
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 4
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: yejiabao <ye...@huawei.com>
Gerrit-Comment-Date: Sat, 04 Jun 2022 12:01:33 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#13).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This patch adds support for compressing rotated log files.
Use --max_compressed_log_files to control the
maximum number of compressed log files to keep.
Use --max_all_compressed_files_size_mb to control the
total size of all compressed files under a Kudu server.

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/client/symbols.map
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
11 files changed, 295 insertions(+), 100 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/13
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 13
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: yejiabao <ye...@huawei.com>

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.

Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18554 )

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................


Patch Set 24:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/18554/24/src/kudu/util/env.h
File src/kudu/util/env.h:

http://gerrit.cloudera.org:8080/#/c/18554/24/src/kudu/util/env.h@219
PS24, Line 219: farmat
format

Also, add a period in the end of the sentence.


http://gerrit.cloudera.org:8080/#/c/18554/24/src/kudu/util/env.h@222
PS24, Line 222: thime
time


http://gerrit.cloudera.org:8080/#/c/18554/24/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:

http://gerrit.cloudera.org:8080/#/c/18554/24/src/kudu/util/env_posix.cc@1868
PS24, Line 1868: because the access time doesn't matter.
If the access time doesn't matter, why not to set access time to be the same as the modified time?  Wouldn't it be more consistent?

Also, this needs to be documented in the header file as well as an additional side-effect of calling this utility function.


http://gerrit.cloudera.org:8080/#/c/18554/24/src/kudu/util/env_util.h
File src/kudu/util/env_util.h:

http://gerrit.cloudera.org:8080/#/c/18554/24/src/kudu/util/env_util.h@99
PS24, Line 99: bool compress_files = false
By adding the 'compress_files' parameter, the semantics of this method changes, and now the name no longer matches the behavior.

Maybe, keep the signature of the original method as is, but also add a new method CompressExcessFilesByPattern()?


http://gerrit.cloudera.org:8080/#/c/18554/24/src/kudu/util/env_util.cc
File src/kudu/util/env_util.cc:

http://gerrit.cloudera.org:8080/#/c/18554/24/src/kudu/util/env_util.cc@337
PS24, Line 337: DeleteExcessCompressedFilesByPattern
What's so special in the implementation of this function that is attributed to compressed files?  I couldn't find anything so far, but I might be missing something.

Maybe, this should be named DeleteFilesByPatternIfOverSize() or something?


http://gerrit.cloudera.org:8080/#/c/18554/24/src/kudu/util/env_util.cc@353
PS24, Line 353: emplace_back(std::make_pair(matching_file_path, file_size));
What if the map already contains an element for the 'mtime' key?  Isn't this going to be a no-op, so the information about another file with the same mtime is lost?


http://gerrit.cloudera.org:8080/#/c/18554/24/src/kudu/util/env_util.cc@357
PS24, Line 357: all_matching_files_size > max_files_size
Is this working as expected when 'all_matching_files_size' becomes negative given the fact that 'max_files_size' is of unsigned integer type?


http://gerrit.cloudera.org:8080/#/c/18554/24/src/kudu/util/logging.cc
File src/kudu/util/logging.cc:

http://gerrit.cloudera.org:8080/#/c/18554/24/src/kudu/util/logging.cc@80
PS24, Line 80:              
style nit: the indent is off, it should be either aligned to the opening parenthesis or be 4 spaces from the beginning of the line


http://gerrit.cloudera.org:8080/#/c/18554/24/src/kudu/util/logging.cc@428
PS24, Line 428:   
style nit: the indent is off, it should be 4 spaces from the beginning of the line above



-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 24
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: yejiabao <ye...@huawei.com>
Gerrit-Comment-Date: Wed, 28 Dec 2022 01:30:44 +0000
Gerrit-HasComments: Yes

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#14).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This patch adds support for compressing rotated log files.
Use --max_compressed_log_files to control the
maximum number of compressed log files to keep.
Use --max_all_compressed_files_size_mb to control the
total size of all compressed files under a Kudu server.

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/client/symbols.map
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
11 files changed, 296 insertions(+), 100 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/14
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 14
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: yejiabao <ye...@huawei.com>

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#12).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This patch adds support for compressing rotated log files.
Use --max_compressed_log_files to control the
maximum number of compressed log files to keep.
Use --max_all_compressed_files_size_mb to control the
total size of all compressed files under a Kudu server.

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/client/symbols.map
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
11 files changed, 295 insertions(+), 100 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/12
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 12
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: yejiabao <ye...@huawei.com>

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#15).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This patch adds support for compressing rotated log files.
Use --max_compressed_log_files to control the
maximum number of compressed log files to keep.
Use --max_all_compressed_files_size_mb to control the
total size of all compressed files under a Kudu server.

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/client/symbols.map
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
11 files changed, 296 insertions(+), 100 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/15
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 15
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: yejiabao <ye...@huawei.com>

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#9).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This patch adds support for compressing rotated log files.
Use --max_compressed_log_files to control the
maximum number of compressed log files to keep.
Use --max_all_compressed_files_size_mb to control the
total size of all compressed files under a Kudu server.

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/client/symbols.map
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
11 files changed, 295 insertions(+), 100 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/9
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 9
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

Posted by "yejiabao (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/18554

to look at the new patch set (#7).

Change subject: [Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage
......................................................................

[Log] KUDU-3369 Support compress Kudu log and use size and number of files to manage

This patch adds support for compressing rotated log files.
Use --max_compressed_log_files to control the
maximum number of compressed log files to keep.
Use --max_all_compressed_files_size_mb to control the
total size of all compressed files under a Kudu server.

Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
---
M src/kudu/client/symbols.map
M src/kudu/server/server_base.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
M src/kudu/util/env_util.cc
M src/kudu/util/env_util.h
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/rolling_log.cc
M src/kudu/util/rolling_log.h
11 files changed, 294 insertions(+), 100 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/18554/7
-- 
To view, visit http://gerrit.cloudera.org:8080/18554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I418743a0d521177bd3ca3343eb2774af2689d38a
Gerrit-Change-Number: 18554
Gerrit-PatchSet: 7
Gerrit-Owner: yejiabao <ye...@huawei.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)