You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2018/01/10 22:26:04 UTC
[kudu-CR] data dirs: fix logging message
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8999
Change subject: data_dirs: fix logging message
......................................................................
data_dirs: fix logging message
If fs_target_data_dirs_per_tablet is set to be greater than the number
of available directories, it will log a message that is dependent on
there being a configured metrics entity, which is not always available.
This patch avoids the potential nullptr access.
Change-Id: I8135e0ad2411524fcb74b3e349bea7a5828a4237
---
M src/kudu/fs/data_dirs.cc
1 file changed, 8 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/8999/1
--
To view, visit http://gerrit.cloudera.org:8080/8999
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8135e0ad2411524fcb74b3e349bea7a5828a4237
Gerrit-Change-Number: 8999
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
[kudu-CR] data dirs: fix logging message
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8999 )
Change subject: data_dirs: fix logging message
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/8999/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/8999/1//COMMIT_MSG@10
PS1, Line 10:
Nit: got an extra space here
http://gerrit.cloudera.org:8080/#/c/8999/1/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:
http://gerrit.cloudera.org:8080/#/c/8999/1/src/kudu/fs/data_dirs.cc@834
PS1, Line 834: LOG(INFO) << Substitute(msg);
Why INFO and not WARNING?
--
To view, visit http://gerrit.cloudera.org:8080/8999
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8135e0ad2411524fcb74b3e349bea7a5828a4237
Gerrit-Change-Number: 8999
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 10 Jan 2018 22:29:30 +0000
Gerrit-HasComments: Yes
[kudu-CR] data dirs: fix logging message
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8999
to look at the new patch set (#2).
Change subject: data_dirs: fix logging message
......................................................................
data_dirs: fix logging message
If fs_target_data_dirs_per_tablet is set to be greater than the number
of available directories, it will log a message that is dependent on
there being a configured metrics entity, which is not always available.
This patch avoids the potential nullptr access.
This patch also changes the logging to INFO-level instead of
WARNING-level, as the message doesn't necessarily indicate a problem.
Change-Id: I8135e0ad2411524fcb74b3e349bea7a5828a4237
---
M src/kudu/fs/data_dirs.cc
1 file changed, 8 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/8999/2
--
To view, visit http://gerrit.cloudera.org:8080/8999
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8135e0ad2411524fcb74b3e349bea7a5828a4237
Gerrit-Change-Number: 8999
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] data dirs: fix logging message
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8999 )
Change subject: data_dirs: fix logging message
......................................................................
data_dirs: fix logging message
If fs_target_data_dirs_per_tablet is set to be greater than the number
of available directories, it will log a message that is dependent on
there being a configured metrics entity, which is not always available.
This patch avoids the potential nullptr access.
This patch also changes the logging to INFO-level instead of
WARNING-level, as the message doesn't necessarily indicate a problem.
Change-Id: I8135e0ad2411524fcb74b3e349bea7a5828a4237
Reviewed-on: http://gerrit.cloudera.org:8080/8999
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/fs/data_dirs.cc
1 file changed, 8 insertions(+), 4 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/8999
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8135e0ad2411524fcb74b3e349bea7a5828a4237
Gerrit-Change-Number: 8999
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] data dirs: fix logging message
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8999 )
Change subject: data_dirs: fix logging message
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/8999/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/8999/1//COMMIT_MSG@10
PS1, Line 10:
> Nit: got an extra space here
Done
http://gerrit.cloudera.org:8080/#/c/8999/1/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:
http://gerrit.cloudera.org:8080/#/c/8999/1/src/kudu/fs/data_dirs.cc@834
PS1, Line 834: LOG(INFO) << Substitute(msg);
> Why INFO and not WARNING?
At first thought, WARNING didn't seem appropriate because this doesn't necessarily point to anything "wrong". E.g. if the default were 3, it would spew warnings by default in clusters configured with 1 data dir.
I suppose it could indicate a improper configuration, but given the upcoming new default behavior, I thought INFO would be more appropriate.
--
To view, visit http://gerrit.cloudera.org:8080/8999
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8135e0ad2411524fcb74b3e349bea7a5828a4237
Gerrit-Change-Number: 8999
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 10 Jan 2018 22:45:22 +0000
Gerrit-HasComments: Yes
[kudu-CR] data dirs: fix logging message
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8999 )
Change subject: data_dirs: fix logging message
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/8999
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8135e0ad2411524fcb74b3e349bea7a5828a4237
Gerrit-Change-Number: 8999
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 10 Jan 2018 22:50:20 +0000
Gerrit-HasComments: No