> On Dec. 17, 2018, 11:53 a.m., Jan Schlicht wrote: > > src/master/master.cpp > > Line 4758 (original), 4758 (patched) > > <https://reviews.apache.org/r/69571/diff/1/?file=2114313#file2114313line4758> > > > > I don't see how this is an improvement over using `std::deque`. We > > still need to iterate over these elements and IMO using a queue for this is > > a good fit.
IMO using an iterator is a better fit to iterate through more than one iterables at the same time. Besides, this would make sure that `authorizations` is never modified. Unnecessary modifying a container when iterating through it sounds harder to maintain to me, and introducing an uncenessary container just feels redundant. Also, even with the current code, we still need to add `CHECK(!authorizations.empty())` everywhere. That said, since both of you and Benjamin have raised the same opinion, I'll keep the changes minimal and revert it back to use `std::deque`. - Chun-Hung ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69571/#review211353 ----------------------------------------------------------- On Dec. 17, 2018, 6:29 a.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69571/ > ----------------------------------------------------------- > > (Updated Dec. 17, 2018, 6:29 a.m.) > > > Review request for mesos, Benjamin Bannier, Greg Mann, and Till Toenshoff. > > > Bugs: MESOS-9480 > https://issues.apache.org/jira/browse/MESOS-9480 > > > Repository: mesos > > > Description > ------- > > This patch fixes a bug where the master does not complete the processing > of authorization results for `LAUNCH_GROUP`, causing a subsequent > operation to drop if one of the remaining authorization is denied. > > > Diffs > ----- > > src/master/master.cpp 3de0fd35cc815f4b5787ee2cb5e81f5059d7a47c > > > Diff: https://reviews.apache.org/r/69571/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Chun-Hung Hsiao > >
