Thomas Tauber-Marshall has posted comments on this change. Change subject: IMPALA-1822: Cleanup query profile log ......................................................................
Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/2260/3/be/src/service/impala-server.cc File be/src/service/impala-server.cc: Line 395: " > why break this line? Sailesh suggested it since its too long. Probably better to leave it out of this patch, though, since its not really relevant to the issue being fixed. http://gerrit.cloudera.org:8080/#/c/2260/3/be/src/util/logging-support.cc File be/src/util/logging-support.cc: Line 100: typedef > nit: space between this and below Done Line 102: if (max_log_files <= 0) return; > nit: reorder this, makes sense to read this line first and have the typedef Done Line 106: glob > We should probably check the return value of this. If it's not zero, we sho Done http://gerrit.cloudera.org:8080/#/c/2260/3/be/src/util/logging-support.h File be/src/util/logging-support.h: Line 36: string > in header files we typically use namespaces, so std::string here (no need t Done http://gerrit.cloudera.org:8080/#/c/2260/3/be/src/util/simple-logger.cc File be/src/util/simple-logger.cc: Line 117: log_file_name > A bit more concise (and probably more efficient) to use: Done -- To view, visit http://gerrit.cloudera.org:8080/2260 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I52430d192d27cdf9bc0d81f26a63048eaeec5d29 Gerrit-PatchSet: 3 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-HasComments: Yes
