Adar Dembo has posted comments on this change. Change subject: env_util: Factor out helper DeleteExcessFilesByPattern() ......................................................................
Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/5740/2/src/kudu/util/env_util-test.cc File src/kudu/util/env_util-test.cc: Line 29: #include "kudu/util/env_util.h" Shouldn't this come first though? Line 118: TEST_F(EnvUtilTest, TestDeleteExcessFilesByPattern) { Maybe you could use the utimes() syscall to artificially set the mtime of each file as you see fit? Then the test could behave the same way for slow and fast mode, and you won't need to sleep at all. http://gerrit.cloudera.org:8080/#/c/5740/2/src/kudu/util/env_util.cc File src/kudu/util/env_util.cc: PS2, Line 241: if (max_matches < 0) { : return Status::InvalidArgument("max_matches must be 0 or greater"); : } Simpler as a DCHECK? The logging.cc call is guaranteed to never provide <0. Line 265: for (const auto& matching_file: matching_file_mtimes) { Nit: "matching_file : matching_file_mtimes" -- To view, visit http://gerrit.cloudera.org:8080/5740 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6e76911b0a68f7c1397d93eb027b6a5c99e2fbed Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
