mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrei Sekretenko <asekrete...@mesosphere.io>
Subject Re: Review Request 72448: Fixed operator api event inconsistencies issue.
Date Fri, 17 Jul 2020 17:40:20 GMT

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


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