----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69472/#review210957 -----------------------------------------------------------
Fix it, then Ship it! src/common/heartbeater.hpp Lines 17 (patched) <https://reviews.apache.org/r/69472/#comment295781> Nit: I guess the include guard should be `__COMMON_HEARTBEATER_HPP__`. src/common/http.hpp Lines 161 (patched) <https://reviews.apache.org/r/69472/#comment296067> Nit: Add backticks around `Event`. src/master/framework.cpp Line 684 (original), 679 (patched) <https://reviews.apache.org/r/69472/#comment296066> Not related to this review, but I noticed while reading this that we're relying here on the thread-safety of `metrics`, but the fact concurrent modifications to `Metrics` are safe is only asserted in a comment in `master.hpp` but nowhere in libprocess itself. - Benno Evers On Dec. 6, 2018, 1:23 a.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69472/ > ----------------------------------------------------------- > > (Updated Dec. 6, 2018, 1:23 a.m.) > > > Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann. > > > Bugs: MESOS-7564 > https://issues.apache.org/jira/browse/MESOS-7564 > > > Repository: mesos > > > Description > ------- > > This moves the very similar `HttpConnection` classes inside the > master and agent into a common header. The refactored > `StreamingHttpConnection<Event>` is more explicitly named to avoid > potentially clashing with the libprocess HTTP helpers. > > This also moves the master's heartbeater helper into a new header > and transforms it into an RAII libprocess actor wrapper. The > heartbeater depends on this `StreamingHttpConnection` and is currently > used by the master for heartbeating the operator event stream > and HTTP framework connection. A later patch will use this heartbeater > for agent->executor heartbeats. > > > Diffs > ----- > > src/common/heartbeater.hpp PRE-CREATION > src/common/http.hpp ac9ed5e8b1c6e8e100eba401136eb95c9dd621d4 > src/master/framework.cpp 36eda9f287bef28608024e0036db1f955a1b393c > src/master/http.cpp 68ee2a6dcffbc772afec6e797b1af8da48f61937 > src/master/master.hpp c7becfa615964674dcf1ebd9424aa5818a0fdb85 > src/master/master.cpp ae5b24093156a6ba8ffa3e94a46e600eb4def5fa > src/slave/http.cpp ba389263ef7ab311682d542424eb56360841d24c > src/slave/slave.hpp edf7269d4057ec8c95bb54c855210ad00d002a50 > src/slave/slave.cpp e13b955ffd92a8872dd513c006e96bd72e298c85 > > > Diff: https://reviews.apache.org/r/69472/diff/4/ > > > Testing > ------- > > make > > The original plan was to use the same helper for the executor too, but of the > Master, Framework, and Agent heartbeats, only the executor lacks a streaming > connection. > > > Thanks, > > Joseph Wu > >
