> On April 10, 2014, 8:06 p.m., Benjamin Hindman wrote: > > Hey Till, do we currently have any callers that require "undo" semantics? > > If we don't, we should consider killing the seek's all together. If we do, > > I'd prefer to make the "undoness" explicit by requiring one to pass a bool > > to protobuf::read requesting the undo (which will fail if they also use a > > file descriptor that doesn't let us seek!). We can then make the default to > > not undo.
That is a great question. My greps found three calls to protobuf::read that use the file descriptor specific overload whereas all others use the path-name overload, hence expect a complete open, read, close cycle. src/slave/state.cpp:582 src/tests/status_update_manager_tests.cpp:160 src/tests/protobuf_io_tests.cpp:66 The one in state.cpp does, from my understanding, in fact rely upon the undo functionality. Hence it appears to be unsafe to remove undoing altogether. Defaulting it to 'false' does force us to update that reference together with this patch. So I will update this patch and propose another one on src/slave/state.cpp. - Till ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20216/#review40076 ----------------------------------------------------------- On April 10, 2014, 3:51 p.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20216/ > ----------------------------------------------------------- > > (Updated April 10, 2014, 3:51 p.m.) > > > Review request for mesos and Benjamin Hindman. > > > Repository: mesos-git > > > Description > ------- > > Updates stout's protobuf::read to drop any seek requests when the given file > descriptor is detected to not support seeking. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 36b7f1a > > Diff: https://reviews.apache.org/r/20216/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Till Toenshoff > >
