You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2018/02/06 23:56:22 UTC

[kudu-CR] Fix crash when log dir is not specified

Hello Jean-Daniel Cryans,

I'd like you to do a code review. Please visit

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

to review the following change.


Change subject: Fix crash when log_dir is not specified
......................................................................

Fix crash when log_dir is not specified

This fixes the tpch1 benchmark after the recent enabling of metrics
logging. The crash was due to FLAGS_log_dir being unset. In typical
servers, we rely on InitGoogleLoggingSafe to default log_dir to /tmp,
but our tpch1 benchmark embeds a client, and a constructor function in
the client calls InitGoogleLoggingSafeBasic which doesn't do this
defaulting. So, we ended up starting the minicluster with an empty log
dir.

This patch just makes it not start the metrics logger if no log dir is
set.

Change-Id: I725a7da038913ff05227c6d5555c34167138148b
---
M src/kudu/server/server_base.cc
1 file changed, 4 insertions(+), 0 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I725a7da038913ff05227c6d5555c34167138148b
Gerrit-Change-Number: 9233
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>

[kudu-CR] Fix crash when log dir is not specified

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9233 )

Change subject: Fix crash when log_dir is not specified
......................................................................

Fix crash when log_dir is not specified

This fixes the tpch1 benchmark after the recent enabling of metrics
logging. The crash was due to FLAGS_log_dir being unset. In typical
servers, we rely on InitGoogleLoggingSafe to default log_dir to /tmp,
but our tpch1 benchmark embeds a client, and a constructor function in
the client calls InitGoogleLoggingSafeBasic which doesn't do this
defaulting. So, we ended up starting the minicluster with an empty log
dir.

This patch just makes it not start the metrics logger if no log dir is
set.

Change-Id: I725a7da038913ff05227c6d5555c34167138148b
Reviewed-on: http://gerrit.cloudera.org:8080/9233
Reviewed-by: Jean-Daniel Cryans <jd...@apache.org>
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>
---
M src/kudu/server/server_base.cc
1 file changed, 4 insertions(+), 0 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Kudu Jenkins: Verified
  Mike Percy: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I725a7da038913ff05227c6d5555c34167138148b
Gerrit-Change-Number: 9233
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] Fix crash when log dir is not specified

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change. ( http://gerrit.cloudera.org:8080/9233 )

Change subject: Fix crash when log_dir is not specified
......................................................................


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I725a7da038913ff05227c6d5555c34167138148b
Gerrit-Change-Number: 9233
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 07 Feb 2018 00:01:55 +0000
Gerrit-HasComments: No

[kudu-CR] Fix crash when log dir is not specified

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9233 )

Change subject: Fix crash when log_dir is not specified
......................................................................


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I725a7da038913ff05227c6d5555c34167138148b
Gerrit-Change-Number: 9233
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Wed, 07 Feb 2018 03:36:14 +0000
Gerrit-HasComments: No