----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19007/#review37576 -----------------------------------------------------------
src/state/log.cpp <https://reviews.apache.org/r/19007/#comment69224> Could we add a check here: CHECK_SOME(truncated); Because I had a hard time follow line 196 and line 267. src/state/log.cpp <https://reviews.apache.org/r/19007/#comment69173> Why passing a 'Nothing' here? Comments would be nice! src/state/log.cpp <https://reviews.apache.org/r/19007/#comment69177> Maybe not for this review. We enqueue a truncate operation for each set/expunge. As a result, multiple truncate operations can get queued. IIUC, consecutive truncate operations seems to be wasteful as only the first one has an effect, right? I understand the current code won't cause any log layer operation as you remember the 'truncated' field. But acquiring/releasing mutex is also expensive as it prevents set/expunge from being executed. Maybe we should consider doing the following: if there is a truncate operation pending, do not enqueue another truncate? src/tests/state_tests.cpp <https://reviews.apache.org/r/19007/#comment69209> Regarding timeout, I understand that we want to push that to the top level like the following: State* state = new ...; state->store(...) .after(Seconds(10), defer(self(), &Self::timeout, lambda::_1)); Future<Option<Variable> > timeout( const Future<Option<Variable> >& future) { // This step is important, otherwise subsequent // stores will be blocked as mutex may not be released. future.discard(); // XXX: Should we wait for 'future' to be in non-pending status? return Failure("..."); } This looks not trivial, can you add some tests for testing the timeout situation? - Jie Yu On March 11, 2014, 2:53 a.m., Benjamin Hindman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19007/ > ----------------------------------------------------------- > > (Updated March 11, 2014, 2:53 a.m.) > > > Review request for mesos, Ben Mahler and Jie Yu. > > > Repository: mesos-git > > > Description > ------- > > This is a combination of both https://reviews.apache.org/r/17423 and > https://reviews.apache.org/r/17424 which have been merged to make it easier > to make changes. > > > Diffs > ----- > > src/Makefile.am 384b3122b61294401ba4a894c06e985d9fc2fb1e > src/messages/state.proto 7f7a8a505d6f24b01fec0c3ad47b0e15b2b17ffa > src/state/log.hpp PRE-CREATION > src/state/log.cpp PRE-CREATION > src/tests/log_tests.cpp b368cdec6a81a8ddeb6fc14d4723a09797a6b0a1 > src/tests/state_tests.cpp bc6c91439a50e7b93077f5b1e66b0419860cb35b > > Diff: https://reviews.apache.org/r/19007/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Hindman > >
