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