mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Guangya Liu" <gyliu...@gmail.com>
Subject Re: Review Request 38514: Update QUIESCE to SUPPRESS in Mesos Call
Date Mon, 21 Sep 2015 06:05:22 GMT


> On 九月 20, 2015, 6:46 p.m., Vinod Kone wrote:
> > src/master/http.cpp, line 486
> > <https://reviews.apache.org/r/38514/diff/2/?file=1077306#file1077306line486>
> >
> >     this method should be called suppress too.
> >     
> >     
> >     to avoid conflict with the suppress marco, can you try putting that macro inside
a "signals" namespace?
> 
> Guangya Liu wrote:
>     Vinod, I was planning to fix this in https://reviews.apache.org/r/38519/diff/1#1
, but the following code diff still does not work after apply to https://reviews.apache.org/r/38519
>     
>     diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/os/signals.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/os/signals.hpp
>     index 7a79024..91076c0 100644
>     --- a/3rdparty/libprocess/3rdparty/stout/include/stout/os/signals.hpp
>     +++ b/3rdparty/libprocess/3rdparty/stout/include/stout/os/signals.hpp
>     @@ -23,10 +23,12 @@
>      #include <stout/os/posix/signals.hpp>
>      #endif // __WINDOWS__
>      
>     +namespace signals {
>      
>      #define suppress(signal) \
>        if (os::signals::internal::Suppressor suppressor ## signal = \
>            os::signals::internal::Suppressor(signal))
>      
>     +} // namespace signals {
>      
>      #endif // __STOUT_OS_SIGNALS_HPP__
>     diff --git a/3rdparty/libprocess/3rdparty/stout/tests/os/signals_tests.cpp b/3rdparty/libprocess/3rdparty/stout/tests/os/signals_tests.cpp
>     index de86232..b1d11c3 100644
>     --- a/3rdparty/libprocess/3rdparty/stout/tests/os/signals_tests.cpp
>     +++ b/3rdparty/libprocess/3rdparty/stout/tests/os/signals_tests.cpp
>     @@ -40,7 +40,7 @@ TEST_F(OsSignalsTest, Suppress)
>        const string data = "hello";
>      
>        // Let's make sure we can suppress SIGPIPE!
>     -  suppress(SIGPIPE) {
>     +  signals::suppress(SIGPIPE) {
>          // Writing to a pipe that has been closed generates SIGPIPE.
>          ASSERT_EQ(-1, write(pipes[1], data.c_str(), data.length()));
>          
>     I was continually getting error of the following, any suggestions? Thanks.
>     
>     In file included from ../../3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp:28:0,
>                      from ../../3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp:23,
>                      from ../../3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp:84,
>                      from ../../3rdparty/libprocess/3rdparty/stout/include/stout/posix/net.hpp:39,
>                      from ../../3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp:59,
>                      from ../../3rdparty/libprocess/include/process/address.hpp:31,
>                      from ../../3rdparty/libprocess/include/process/pid.hpp:26,
>                      from ../../3rdparty/libprocess/include/process/latch.hpp:20,
>                      from ../../3rdparty/libprocess/include/process/future.hpp:33,
>                      from ../../include/mesos/authorizer/authorizer.hpp:28,
>                      from ../../src/local/local.cpp:26:
>     ../../3rdparty/libprocess/3rdparty/stout/include/stout/os/signals.hpp:29:3: error:
expected unqualified-id before 'if'
>        if (os::signals::internal::Suppressor suppressor ## signal = \
>        ^
>     ../../src/master/master.hpp:832:8: note: in expansion of macro 'suppress'
>        void suppress(Framework* framework);
>             ^
> 
> Vinod Kone wrote:
>     try putting "()" around the suppress function in master.cpp to avoid master expansion.
>     
>     alternatively, the 'suppress' macro can be renamed to 'block'. cc @bmahler

The issue was not addressed in https://reviews.apache.org/r/38544/


- Guangya


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38514/#review99710
-----------------------------------------------------------


On 九月 19, 2015, 1:27 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38514/
> -----------------------------------------------------------
> 
> (Updated 九月 19, 2015, 1:27 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3037
>     https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> A comment from Vinod: After some discussion with other committers,
> most of them felt SUPPRESS is better than QUIESCE.
> 
> 
> Diffs
> -----
> 
>   include/mesos/scheduler/scheduler.proto 89ddc10d5873cdf09bf5084c4869bc7c29933eab 
>   include/mesos/v1/scheduler/scheduler.proto bc19b8dfb2ded26a30a28e7370f06816e0fb1999

>   src/master/http.cpp 7cca8b1d3118fe99de79cfaf5360825012d830da 
>   src/master/master.cpp 64e5fb9e27b2e6797b2ce87c0e1a3ef8a2943e27 
>   src/master/validation.cpp 0f3fc1a20b065a88712cceabab0ceffaec598533 
>   src/sched/sched.cpp 84c2edb2c660f976fd57d1407dd2e9d5376500aa 
>   src/tests/scheduler_tests.cpp 0f892f9d423f0bc72cdab22452e0ad7965dda444 
> 
> Diff: https://reviews.apache.org/r/38514/diff/
> 
> 
> Testing
> -------
> 
> Platform: Ubuntu:14.04
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


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