... and now it compiles in godbolt:
(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 <rfriedma@xxxxxxxxxx> 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":In the face of an error we may want to have this behavior (1'st option):
- 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":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 oneNote that the code is not polished yet. For example: I think I would be able to replace the callreturn 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.RonenOn Wed, Jul 3, 2019 at 7:13 AM kefu chai <tchaikov@xxxxxxxxx> 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
_______________________________________________ Dev mailing list -- dev@xxxxxxx To unsubscribe send an email to dev-leave@xxxxxxx
