> On Feb. 16, 2015, 11:37 p.m., Adam B wrote: > > src/authentication/cram_md5/authenticator.hpp, line 533 > > <https://reviews.apache.org/r/27760/diff/14/?file=863170#file863170line533> > > > > Should this still `process::terminate(process, false)` if the short > > term fix is now in `~CRAMMD5AuthenticatorSession`? The 'false' injects the > > 'terminate' at the end of the process' queue, so other in-flight events get > > handled first (semi-graceful shutdown). > > Till Toenshoff wrote: > Correct, I think it should be a regular terminate at this point now > (getting pushed into the front of the queue).
>From the description of MESOS-1866, I understand the race to apply to >destruction of the session, not the entire master/authenticator. The >double-future approach might even fix this somehow. I'd have to dig deeper. Unfortunately, MESOS-1866 Race 1 never had a unit test written for it, so there's no way to test that we aren't regressing. Maybe @vinodkone can comment on the previous race and help us determine if we're maintaining/improving the proper behavior. > On Feb. 16, 2015, 11:37 p.m., Adam B wrote: > > src/authentication/cram_md5/authenticator.hpp, line 545 > > <https://reviews.apache.org/r/27760/diff/14/?file=863170#file863170line545> > > > > What's the reasoning for this being a `static*`? Why wouldn't a plain > > old `Option<Error>` work? > > Till Toenshoff wrote: > That way, if an error occured in the "once"-covered case, then any > additional calls to initialize (e.g. by another instance) would still have > access to that former, "once"-covered error and be able to return it. Okay, makes sense. Worth a comment? - Adam ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27760/#review72680 ----------------------------------------------------------- On Feb. 17, 2015, 7:57 p.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27760/ > ----------------------------------------------------------- > > (Updated Feb. 17, 2015, 7:57 p.m.) > > > Review request for mesos, Adam B, Kapil Arya, Niklas Nielsen, and Vinod Kone. > > > Bugs: MESOS-2050 > https://issues.apache.org/jira/browse/MESOS-2050 > > > Repository: mesos > > > Description > ------- > > The initial design and implementation of the authenticator module interface > caused issues and was not optimal for heavy lifting setup of external > dependencies. By introducing a two fold design, this has been decoupled from > the authentication message processing. The new design also gets us back on > track to the goal of makeing SASL a soft dependency of mesos. > > > Diffs > ----- > > include/mesos/authentication/authenticator.hpp f66217a > src/Makefile.am d372404 > src/authentication/cram_md5/authenticator.hpp 7578ea1 > src/authentication/cram_md5/authenticator.cpp PRE-CREATION > src/authentication/cram_md5/auxprop.hpp d036b11 > src/authentication/cram_md5/auxprop.cpp 5ff9755 > src/master/master.hpp 6a39df0 > src/master/master.cpp f10a3cf > src/tests/cram_md5_authentication_tests.cpp dd102dc > > Diff: https://reviews.apache.org/r/27760/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Till Toenshoff > >
