Alexey Serbin has posted comments on this change.

Change subject: Add basic Hive MetaStore client
......................................................................


Patch Set 12:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7053/12/src/kudu/util/test_util.cc
File src/kudu/util/test_util.cc:

PS12, Line 327: Status WaitForBind(pid_t pid, uint16_t* port, const char* kind, 
MonoDelta timeout) {
              :   // In general, processes do not expose the port they bind to, 
and
              :   // reimplementing lsof involves parsing a lot of files in 
/proc/. So,
              :   // requiring lsof for tests and parsing its output seems more
              :   // straight-forward. We call lsof in a loop since it 
typically takes a long
              :   // time for it to initialize and bind a port.
              : 
              :   string lsof;
              :   RETURN_NOT_OK(GetExecutablePath("lsof", {"/sbin", 
"/usr/sbin"}, &lsof));
              : 
              :   const vector<string> cmd = {
              :     lsof, "-wbnP", "-Ffn",
              :     "-p", std::to_string(pid),
              :     "-a", "-i", kind
              :   };
              : 
              :   MonoTime deadline = MonoTime::Now() + timeout;
              :   string lsof_out;
              : 
              :   for (int64_t i = 1; ; i++) {
              :     lsof_out.clear();
              :     Status s = Subprocess::Call(cmd, "", &lsof_out);
              : 
              :     if (s.ok()) {
              :       StripTrailingNewline(&lsof_out);
              :       break;
              :     }
              :     if (deadline < MonoTime::Now()) {
              :       return s;
              :     }
              : 
              :     SleepFor(MonoDelta::FromMilliseconds(i * 10));
              :   }
              : 
              :   // The '-Ffn' flag gets lsof to output something like:
              :   //   p19730
              :   //   f123
              :   //   n*:41254
              :   // The first line is the pid. We ignore it.
              :   // The second line is the file descriptor number. We ignore 
it.
              :   // The third line has the bind address and port.
              :   vector<string> lines = strings::Split(lsof_out, "\n");
              :   int32_t p = -1;
              :   if (lines.size() != 3 ||
              :       lines[2].substr(0, 3) != "n*:" ||
              :       !safe_strto32(lines[2].substr(3), &p) ||
              :       p <= 0) {
              :     return Status::RuntimeError("unexpected lsof output", 
lsof_out);
              :   }
              :   CHECK(p > 0 && p < std::numeric_limits<uint16_t>::max()) << 
"parsed invalid port: " << p;
              :   VLOG(1) << "Determined bound port: " << p;
              :   *port = p;
              :   return Status::OK();
              : }
              : } // anonymous namespace
              : 
              : Status WaitForTcpBind(pid_t pid, uint16_t* port, MonoDelta 
timeout) {
              :   return WaitForBind(pid, port, "4TCP", timeout);
              : }
              : 
              : Status WaitForUdpBind(pid_t pid, uint16_t* port, MonoDelta 
timeout) {
              :   return WaitForBind(pid, port, "4UDP", timeout);
              : }
> I thought you were about to move this into a separate changelist.
woops, that's probably because this changelist hasn't been yet re-based.


http://gerrit.cloudera.org:8080/#/c/7053/12/src/kudu/util/test_util.h
File src/kudu/util/test_util.h:

PS12, Line 114: // as well as the $PATH environment variable.
              : Status GetExecutablePath(const std::string& binary,
              :                          const std::vector<std::string>& search,
              :                          std::string* path) WARN_UNUSED_RESULT;
              : 
              : // Waits for the subprocess to bind to any listening TCP port, 
and returns the port.
              : Status WaitForTcpBind(pid_t pid, uint16_t* port, MonoDelta 
timeout) WARN_UNUSED_RESULT;
              : 
              : // Waits for the subprocess to bind to any listening UDP port, 
and returns the port.
              : Status WaitForUdpBind(pid_t pid, uint16_t* port, MonoDelta 
timeout) WARN_UNUSED_RESULT;
> I thought you were about to move this into a separate changelist.
woops, that's probably because this changelist hasn't been yet re-based.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to