The reason for the performance difference between the OrderedExecutor and
the OrderedScheduler is
the underlying queue that is used to pass tasks to the executor thread.

In the case of OrderedExecutor, it needs a BlockingQueue and the current
default is to use JDK LinkedBlockingQueue which relies on CAS for
enqueue/dequeue. Additional room for improvement here is to use a more
specialized MP-SC queue with different wait strategies.

For OrderedScheduler, it needs a priority queue, since tasks can be either
immediate or scheduler for future execution. With priority queue, a mutex
is typically necessary and will introduce contention when there are
multiple threads passing tasks to the scheduler.

Overall, I think we should still keep separated the critical data path from
the rest of operations that are not necessarily time critical.

> What I want to achieve here is that, after a operation is submitted to
a ledger, it only ever operates on a single thread.

I think a viable option would be to keep the scheduler separate and then
always jump in the ledger thread. eg:

scheduler.schedule(() -> {
    executor.executeOrdered(ledgerId, SafeRunnable.safeRun(() -> {
        // do something
    });
}, 100, TimeUnit.MILLISECONDS);

If the scheduler is not always used, the thread could also be lazily
created.

Matteo

On Thu, Aug 23, 2018 at 8:48 AM Ivan Kelly <iv...@apache.org> wrote:

> > I don't think it is accidently. OrderedExecutor has performance
> advantages
> > than OrderedScheduler.
> >
> > A bit background on this:
> >
> > - OrderedScheduler was introduced by me. And I changed existing
> > OrderedSafeExecutor to be extending from OrderedScheduler.
> >   Trying to standarize to one `OrderedScheduler`.
> > - I think Matteo noticed performance difference between `executor` and
> > `scheduler`, so he made the change
> >
> >
> https://github.com/apache/bookkeeper/commit/46171e67e526702487641438144f28b7eb1aa07b
> > .
> > So the `executor` is used as the main callback executor, since it just
> > requires ordering but doesn't need scheduling capability.
> > the `scheduler` is used for scheduling tasks but doesn't require
> ordering.
>
> The scheduler does need ordering in one case for explicit Lac.
>
> I think we could modify the scheduler, so that we could use the same
> scheduler object for the executor and for scheduling.
>
> Instead of having multiple executors in the scheduler, just create one
> scheduled executor, which then submits to the executor service after
> the delay.
>
> What I want to achieve here is that, after a operation is submitted to
> a ledger, it only ever operates on a single thread.
> If you look at LedgerHandle now, you have to jump around 4 files to
> deduce which thread methods like handleBookieFailure or
> sendAddSuccessCallbacks are called on, and even then you can't even be
> sure, so we wrap everything in synchronized when we don't really need
> to.
>
> -Ivan
>
-- 
Matteo Merli
<mme...@apache.org>

Reply via email to