Adar Dembo has posted comments on this change. Change subject: KUDU-766: limit number of glog files ......................................................................
Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/5340/1/src/kudu/integration-tests/external_mini_cluster.cc File src/kudu/integration-tests/external_mini_cluster.cc: PS1, Line 293: peer_addrs[i], > Do you mind if I hold off on this? It's not immediately clear what the rig Hmm, didn't realize it was complicated. Okay. http://gerrit.cloudera.org:8080/#/c/5340/1/src/kudu/server/server_base.cc File src/kudu/server/server_base.cc: Line 362: if (metrics_logging_thread_) { > Reworked in what way? I looked at this when I first made the change to see 1) We're not joining on the new thread at all. 2) We should CountDown() regardless of whether metrics_logging_thread_ exists. http://gerrit.cloudera.org:8080/#/c/5340/1/src/kudu/util/logging.cc File src/kudu/util/logging.cc: Line 333: > Yes, I think it would. I think we should stick with mtimes for now, since Before committing to this approach I'd like to understand why Impala does it this way. Could you investigate? http://gerrit.cloudera.org:8080/#/c/5340/4/src/kudu/util/logging.cc File src/kudu/util/logging.cc: Line 18: Since you're changing the system includes, mind splitting out the real "system" ones from the Kudu-provided ones? That is, boost and glog should be in a separate group after the STL includes. -- To view, visit http://gerrit.cloudera.org:8080/5340 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia3f81638a6dfcf134665322f091ff26d29640fd9 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
