... and now it compiles in godbolt: https://godbolt.org/z/xPMzP5 (I've also made some changes to the code, to better demonstrate the possible error-handling paths) Comments/corrections anyone?

Thanks Ronen On Thu, Jul 4, 2019 at 5:34 PM Ronen Friedman &lt;rfriedma(a)redhat.com&gt; wrote: > Hi Kefu, > > Like I said in the meeting: I really think this is the right direction. > I am not sure I fully understand the specifics you've proposed. Here is > my take (which may be exactly what you meant): > > Suppose this is the "happy path": > [image: outcome_1.jpg] > > > In the face of an error we may want to have this behavior (1'st option): > [image: outcome_2.jpg] > > > - let's assume that the values of type T are wrapped in > outcome::outcome<T, error-code, exception-code> (I'd rather using that over > outcome::result); > - Thus, f1() should be written to return the wrapper type > (outcome<T1>), > - f2() is now a function from outcome<T1> to outcome<T2>. > - as we may want to have error codes "pass thru" f2(), without > requiring f2() to be modified with the boiler-plat code for the pass-thru, > let us wrap f2() into pass_thru<T1,T2>(f2). I have attached code that > presents one way of doing just that. > > note that while f2() is just a "pass thru" for an incoming error code, > we still need to "re-code" the error value in the incoming outcome<T1> into > the same value in outcome<T2>. > > > Another desired behaviour, as you mention in the email, is to split the > logic into a separate "happy days" and "error path": > > [image: outcome_3.jpg] > > We can use a separate wrapper around the two paths (note that both paths > are expected to return optional<T2>). > > Here is some code that demonstrates what I mean (compiled with g++9, but > the link in Godbolt is only as a display mechanism): > > https://godbolt.org/z/i-d4TU <- use the new link above, not this one > > Note that the code is not polished yet. For example: I think I would be > able to replace the call > > return WpassR<Oint, Ofloat>(of2_ps, x) > > > with > > > return WpassR(of2_ps, x) > > > using some magic (deduction guides? that will require building some > structs around) - if we think this direction is worth our time. > > Ronen > > > > On Wed, Jul 3, 2019 at 7:13 AM kefu chai &lt;tchaikov(a)gmail.com&gt; wrote: > >> hi guys, >> >> i just came across boost::outcome[0]. it reminded me the discussion we >> had back in Barcelona regarding to the error handling in crimson. >> well, strictly speaking, it's not limited to errors. it covers the >> non-error handling as well. >> >> the question is: shall we start prototyping the crimson variant of >> outcome<> now? if yes, probably we can leverage boost::outcome<>? >> >> a little bit background: >> >> seastar uses exception for propagating the error. but it incurs >> runtime overhead. because, to throw an exception, the libstdc++ >> runtime needs to acquire a global lock. >> >> well, some of us might want to argue, why not just return a >> future<Result, Error>? let me use an example here, imagine we are >> handling a write request in OSD. we might need to go through following >> steps: >> >> 1. perform some sanity tests, for instance, to see if the OSD is ready >> for handling the write request >> 2. try to read the object info of the object from local storage to see >> if it already exists >> 3. write to the object to the local storage, and send write requests >> to replica OSDs (assuming it's in a replicated pool), wait for the >> completions of these write ops. >> 4. update the statistics >> 5. reply to the client >> >> and it's intuitive to structure these steps using chained continuation >> like >> >> do_with(std::move(request), [this](auto request) { >> return perform_tests(request->object_id).then([request, this] { >> return read_object_info(request->object_id); >> }).then([request, this](optional<object_info> object_info) { >> return when_all( >> write_local(request->object_id, request->offset, request->data), >> parallel_for_each(replica_osds, [request](auto replica_osd) { >> return replica_osd->write_remote(request->object_id, >> request->offset, request->data) >> })); >> }).then([write_size=request.data.size(), this] { >> update_statistics(write_size); >> return reply_to(reply_t::success, request); >> }); >> }).handle_exception([](auto exception) { >> return reply_to(reply_t::failure, exception.error_code, request); >> }); >> >> in which, if any test fails in step#1, we either need to wait until >> the OSD is ready, or just need to bail out, and skip the following >> steps. the "handle_exception()" clause is used to handle the "bail >> out" case, where we cannot do anything to serve the request. for >> instance, the request is invalid. >> >> we want to differentiate two types of errors. one of them are actually >> exceptions which does not happen often in real world, and we don't >> need/want to optimize for this case. but the other case could be >> normal. for instance, it's fairly normal that an object does not exist >> yet, when we are trying to write to it. and we do want to be >> performant when handling these "errors" in this category, and also, we >> want to do this in a convenient way just like handling exceptions. >> >> because, we need an efficient way to convey the message to caller that >> "please skip the following continuations, and i would go to this >> handling route instead". if my memory serves me correctly, we think >> that we need to create a wrapper around seastar::future<> to allow the >> caller to do something like >> >> // a helper to run func or skip it >> template<typename Func> >> auto ignore_on_error(Func&& f) { >> return [f=std::move(f)](auto&& t) { >> return t.is_value() ? f(t.value()) ? t; >> } >> } >> >> return read_object_info(oid).then( >> return ignore_on_error([](object_info& oi) { >> return handle_write_with_object_info(std::move(oi)); >> }).then([](auto t) { >> return handle_write_without_object_info(); >> }); >> ); >> >> in the example above, i assume we will do something very different >> depending on if the object's existence. >> >> >> cheers, >> >> --- >> [0] >> https://www.boost.org/doc/libs/1_70_0/libs/outcome/doc/html/index.html >> >> -- >> Regards >> Kefu Chai >> >