----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61262/#review182958 -----------------------------------------------------------
src/master/master.hpp Lines 309-311 (patched) <https://reviews.apache.org/r/61262/#comment258950> Now that we're sending heartbeats for multiple purposes, I'm concerned that the logging will be ambiguous. Instead of templating this class on the ID type, perhaps it makes more sense to initialize it with a string that it can use for logging purposes. For example: ``` class Heartbeater : public process::Process<Heartbeater> { public: Heartbeater(const string& _message, const HttpConnection& _http, const Duration& _interval, const Duration& _delay = Seconds(0)) : process::ProcessBase(process::ID::generate("heartbeater")), message(_message), http(_http), interval(_interval), delay(_delay) {} ... private: void heartbeat() { // Only send a heartbeat if the connection is not closed. if (http.closed().isPending()) { VLOG(1) << "Sending heartbeat to " << message; etc... ``` and then for the framework case we would do something like: ``` heartbeater = new Heartbeater<FrameworkID>( "framework " + stringify(info.id()), http.get(), DEFAULT_HEARTBEAT_INTERVAL); ``` and the subscriber case something like: ``` heartbeater = new Heartbeater<UUID>( "subscriber " + stringify(http.streamId), http, DEFAULT_HEARTBEAT_INTERVAL, DEFAULT_HEARTBEAT_INTERVAL); ``` what do you think? src/tests/api_tests.cpp Lines 2381 (patched) <https://reviews.apache.org/r/61262/#comment258960> How about: "Verifies that 'HEARTBEAT' events are sent at the correct times." src/tests/api_tests.cpp Lines 2444 (patched) <https://reviews.apache.org/r/61262/#comment258961> You should also verify that the type of the event is correct here. - Greg Mann On Aug. 14, 2017, 9:51 p.m., Quinn Leng wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61262/ > ----------------------------------------------------------- > > (Updated Aug. 14, 2017, 9:51 p.m.) > > > Review request for mesos, Anand Mazumdar and Greg Mann. > > > Bugs: MESOS-7695 > https://issues.apache.org/jira/browse/MESOS-7695 > > > Repository: mesos > > > Description > ------- > > Added the 'HEARTBEAT' event for the operator API, modified other > related test cases to accept heartbeats. > > > Diffs > ----- > > include/mesos/master/master.proto fc5bd894ce55fe8e946d4c5b4b33d3c0505f3c2b > include/mesos/v1/master/master.proto > c3fb31de2509adcdec8204f8bbe46b46a31540e8 > src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 > src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 > src/tests/api_tests.cpp 3ab4740bcac29ecb89585da6adb1f563d6fc1f5f > > > Diff: https://reviews.apache.org/r/61262/diff/5/ > > > Testing > ------- > > make check -j48 > > > Thanks, > > Quinn Leng > >
