mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Greg Mann" <g...@mesosphere.io>
Subject Re: Review Request 42165: Removed `hasPrincipal` parameter from unreserve validation.
Date Tue, 12 Jan 2016 18:03:45 GMT


> On Jan. 12, 2016, 2:33 a.m., Guangya Liu wrote:
> > Greg, I think that the document of reservation.md should also be updated. 
> > 
> > BTW: Did you test your patch? I did some test as this and mesos-master core dump.
> > 
> > curl -i -d slaveId="4c737d19-e3b2-41c0-b4d0-ffc60190b8eb-S0" -d resources='[ { "name":
"cpus",  "type": "SCALAR", "scalar": { "value": 3 }, "role": "ads", "reservation": { "principal":
"p1" } }, {  "name": "mem", "type": "SCALAR",  "scalar": { "value": 3000 }, "role": "ads",
"reservation": { "principal": "p1" } }  ]'  -X POST http://9.110.49.27:5050/master/reserve
> > curl: (52) Empty reply from server
> > 
> > --
> > I0112 10:26:35.901185 333451264 http.cpp:315] HTTP POST for /master/reserve from
9.110.49.27:50492 with User-Agent='curl/7.43.0'
> > Assertion failed: (isSome()), function get, file ../../3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp,
line 108.
> > *** Aborted at 1452565595 (unix time) try "date -d @1452565595" if you are using
GNU date ***
> > PC: @     0x7fff8ca15286 __pthread_kill
> > *** SIGABRT (@0x7fff8ca15286) received by PID 43187 (TID 0x113e01000) stack trace:
***
> >     @     0x7fff89a0cf1a _sigtramp
> >     @     0x7f917bd08fd0 (unknown)
> >     @     0x7fff867139ab abort
> >     @     0x7fff866dba91 __assert_rtn
> >     @        0x10c517947 Option<>::get()
> >     @        0x10cf6c0f9 mesos::internal::master::validation::operation::validate()
> >     @        0x10ca5c0c2 mesos::internal::master::Master::Http::reserve()
> >     @        0x10cba6634 mesos::internal::master::Master::initialize()::$_9::operator()()
> >     @        0x10cba65d4 _ZNSt3__128__invoke_void_return_wrapperIN7process6FutureINS1_4http8ResponseEEEE6__callIJRZN5mesos8internal6master6Master10initializeEvE3$_9RKNS3_7RequestERK6OptionINS_12basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEEEEEES5_DpOT_
> >     @        0x10cba64a3 std::__1::__function::__func<>::operator()()
> >     @        0x10eacfaa2 std::__1::function<>::operator()()
> >     @        0x10ea0b399 process::ProcessBase::visit()::$_3::operator()()
> >     @        0x10ea0e9f1 _ZZZNK7process9_DeferredIZNS_11ProcessBase5visitERKNS_9HttpEventEE3$_3EcvNSt3__18functionIFvT_EEEIRKNS_6FutureI6OptionINS_4http14authentication20AuthenticationResultEEEEEEvENKUlSL_E_clESL_ENKUlvE_clEv
> >     @        0x10ea0e9bd _ZNSt3__128__invoke_void_return_wrapperIvE6__callIJRZZNK7process9_DeferredIZNS3_11ProcessBase5visitERKNS3_9HttpEventEE3$_3EcvNS_8functionIFvT_EEEIRKNS3_6FutureI6OptionINS3_4http14authentication20AuthenticationResultEEEEEEvENKUlSO_E_clESO_EUlvE_EEEvDpOT_
> >     @        0x10ea0e6fc _ZNSt3__110__function6__funcIZZNK7process9_DeferredIZNS2_11ProcessBase5visitERKNS2_9HttpEventEE3$_3EcvNS_8functionIFvT_EEEIRKNS2_6FutureI6OptionINS2_4http14authentication20AuthenticationResultEEEEEEvENKUlSN_E_clESN_EUlvE_NS_9allocatorISP_EEFvvEEclEv
> >     @        0x10c510371 std::__1::function<>::operator()()
> >     @        0x10c56f999 _ZZN7process8dispatchERKNS_4UPIDERKNSt3__18functionIFvvEEEENKUlPNS_11ProcessBaseEE_clESA_
> >     @        0x10c56f970 _ZNSt3__128__invoke_void_return_wrapperIvE6__callIJRZN7process8dispatchERKNS3_4UPIDERKNS_8functionIFvvEEEEUlPNS3_11ProcessBaseEE_SD_EEEvDpOT_
> >     @        0x10c56f70c _ZNSt3__110__function6__funcIZN7process8dispatchERKNS2_4UPIDERKNS_8functionIFvvEEEEUlPNS2_11ProcessBaseEE_NS_9allocatorISD_EEFvSC_EEclEOSC_
> >     @        0x10ea2529f std::__1::function<>::operator()()
> >     @        0x10e9ff2ff process::ProcessBase::visit()
> >     @        0x10ea531be process::DispatchEvent::visit()
> >     @        0x10c4efd71 process::ProcessBase::serve()
> >     @        0x10e9fc091 process::ProcessManager::resume()
> >     @        0x10ea0784f process::ProcessManager::init_threads()::$_1::operator()()
> >     @        0x10ea074d2 _ZNSt3__114__thread_proxyINS_5tupleIJNS_6__bindIZN7process14ProcessManager12init_threadsEvE3$_1JNS_17reference_wrapperIKNS_6atomicIbEEEEEEEEEEEEPvSD_
> >     @     0x7fff8c6e105a _pthread_body
> >     @     0x7fff8c6e0fd7 _pthread_start
> >     @     0x7fff8c6de3ed thread_start

Yep, I tested as noted in the "Testing Done" section, but I didn't manually apply an operation
as you did - thanks a bunch for doing this Guangya, as you found a mistake that I made! In
the parent patch of this one (https://reviews.apache.org/r/42164/) you'll find the fix, as
well as a couple new test cases that apply reserve/unreserve operations without a principal.
The tests produced a segfault before the fix, and succeeded after the fix.

Thanks again!!!

I noticed in your `curl` command that you included a principal "p1" in the resources you were
attempting to reserve even though no authentication headers were included in the request.
Note that this will produce an error given the new changes I introduced, since if no principal
is provided with the operation, the `ReservationInfo` should contain an empty principal as
well.


- Greg


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


On Jan. 11, 2016, 10:40 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42165/
> -----------------------------------------------------------
> 
> (Updated Jan. 11, 2016, 10:40 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-3940
>     https://issues.apache.org/jira/browse/MESOS-3940
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Removed `hasPrincipal` parameter from unreserve validation.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp bcafc7aff89659a68352f3876ce6042f8b34bd5d 
>   src/master/master.cpp 2d9b7f9540574aa3ef9e5af3b2b8922dffeebac8 
>   src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
>   src/master/validation.cpp 6a43bce5b7df6a9d939245c4726d060fa19eb305 
>   src/tests/master_validation_tests.cpp fbf8fadbc04a7cbc60ee6091e0224339389b400f 
> 
> Diff: https://reviews.apache.org/r/42165/diff/
> 
> 
> Testing
> -------
> 
> Several instances of the relevant validation function were altered in the master validation
tests.
> 
> `make check` was used to test on both OSX and Ubuntu 14.04
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


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