mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benno Evers <bev...@mesosphere.com>
Subject Re: Review Request 69472: Refactored master and agent streaming connections.
Date Mon, 10 Dec 2018 14:48:35 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message