On Tue, Sep 22, 2015 at 5:26 AM, Sage Weil <sage@xxxxxxxxxxxx> wrote:
> On Tue, 22 Sep 2015, Lakis, Jacek wrote:
>> Hi ceph-devel!
>> We're checking OSD::op_is_discardable condition two times: in dispatcher (handle_op) and in worker threads (do_request->can_discard_request->can_discard_op)
>>
>> if (!op->get_connection()->is_connected() &&
>> op->get_version().version == 0) {
>> return true;
>> }
>>
>> Is there any purpose to do this? If not, which of those should be
>> removed in your opinion? If condition in handle_op will remain, OP won't
>> pass to the worker threads through the prioritized queue. On the other
>> hand, leaving it in do_request will parallelize this check (I started
>> parallelizing in PR 5211).
>
> Is there a reason not to do it twice? It looks pretty cheap.
>
> But, if we have to choose, I'd leave it in the worker thread. It is
> unlikely to trigger on newly dispatched messages (that were *just* taken
> off the wire), and the worker thread check is critical to avoid wasting
> work on requeued/delayed messages that no longer have clients waiting on
> them.
While this is definitely the right choice if eliminating one, I think
there was a specific reason to leave it in both. Perhaps just because
it's cheap, but I think maybe the rest of the dispatch work in that
thread relies on it having a connection. (Unless that got changed? We
are a lot more generous about lack of connections than we used to be.)
-Greg
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
