----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72448/#review221253 -----------------------------------------------------------
src/common/http.hpp Lines 436-441 (patched) <https://reviews.apache.org/r/72448/#comment310108> Thanks for taking care about logging! We definitely don't want to write a log line per every authorization error, as the errors might (and likely will) come in large bunches. This should happen at most once per disconnecting the subscriber (one more reason to move this into `Subscribers::send()`). Also, it probably would be beneficial to log some other details allowing the persons reading the log to identify the subscriber (at the very least, IP + principal, and probably something else - honestly, I don't remember what else Mesos master knows about a subscriber). src/common/http.hpp Lines 437 (patched) <https://reviews.apache.org/r/72448/#comment310109> Is it necessary to use an intermediate `ostringstream` instead of directly writing into the glog's stream returned by `LOG(WARNING)`? - Andrei Sekretenko On July 14, 2020, 9:43 a.m., Dong Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72448/ > ----------------------------------------------------------- > > (Updated July 14, 2020, 9:43 a.m.) > > > Review request for mesos, Andrei Sekretenko and Benjamin Mahler. > > > Repository: mesos > > > Description > ------- > > This patch intends to fix issue MESOS-10085. > > When the authorization failed happens master return nothing to the > subscriber, subscriber isn't aware of what is happening, this issue > can lead to inconsistencies in Event stream. > > > Diffs > ----- > > include/mesos/master/master.proto 021dadcea026da41347b3aaee5ddd12f4f14fa29 > include/mesos/v1/master/master.proto > 488fe294e8bfe8e0c6fc23c88f06c0d41169b96d > src/common/http.hpp 02633e175c0848ee622cb5108a2e18db5e030641 > src/master/master.cpp a8cca622ff0bd172300b9a2717b4860ed06b620c > src/tests/master/mock_master_api_subscriber.cpp > 893d3e366164ccebd2847ed4c2874ab00e0e5b7b > > > Diff: https://reviews.apache.org/r/72448/diff/2/ > > > Testing > ------- > > - Manually tested > - make check > > > Thanks, > > Dong Zhu > >
