PriorityQueue in AsynchronousFileImpl

[timfox]

Why is this necessary?

I thought you were applying context ordering on the thread level now? Isn't a thread level ordering sufficient?

[clebert.suconic]

This is for consistency also.

We are now reusing the same callbacks (Which is OperationContext).

Say, you added record1, record2, record3.

The callbacks could come record3, record1, record2.

If I don't force order on AIO, record1 would be delivered as soon as record3 is back.

[timfox]

Sure I understand the callbacks (e.g. responses to clients) need to be executed in the same order they were added, but this is on a per session level, and you are doing the ordering on a global level.

Why is it necessary to do the ordering at a global level (note we never had a global ordering previously)

[clebert.suconic]

What we were doing before didn't require ordering over the callbacks, because we were doing this:



The write method was something like this:

Callback callback = new Callback();
writeInternal(data, callback);
callback.wait();

So, we were blocking, and if you did write(1), write(2) and write(3)... each callback would get back correctly. I would know that when the callback for 1 is back, 1 is really on the disk.


What we are doing now is more like:


Callback callback = new Callback();

writeInternal(data, callback);
writeInternal(data, callback);
writeInternal(data, callback);
writeInternal(data, callback);



^^ We are reusing the same callback (OrderedContext).


if the response is back out of the order, the Callback will think the response is for the first element on the queue and it could deliver something that is not on the disk yet.

[timfox]

Yes, like I said before I understand why the responses need to be ordered for any particular thread.

However, you are ordering them *globally*. That was my question: why are you ordering them *globally* as opposed to on the thread level.

[clebert.suconic]

If you want to control this at thread level, The solution would be to have OrderedContext creating a new callback instance for each write done, and have the sequence controlled internally at the OrderedContext. The only overhead will be an extra callback instance created for each write operation done, and have one priority queue for each Context.

[timfox]

It's not about what I want, it's about what is the correct place to do the ordering.

My question was, why are you ordering at a global level as opposed to the thread level?

That question hasn't been answered yet.

I'm not looking for implementation details, I'm just curious in why it's necessary to order at the global level.

[clebert.suconic]

Ordering on the callback is an exclusive problem of AIO. To me the most obvious place to fix it was AIOSequentialFile

Controlling it outside of AIO would mean extra complexity and overhead on the OperationContext that will never happen on NIO.

AIO is mostly ordered, I didn't think the PriorityQueue would cause any performance issues from the tests I have made.

Doing it in a different way would mean more complexity and more overhead maybe.

[timfox]

I don't see how it is extra complexity doing the ordering on the context.

It's basically the same ordering algorithm you have already written on the AsynchronousFileImpl.

If you need to pass an id back in the completion, this is just an extra attribute.

Where is all this overhead, and complexity?

Having ordering at the global level also provides an overhead in increases latency where one session is waiting for completions from another session to complete - this is unnecessary as far as I can tell.

[clebert.suconic]

If you need to pass an id back in the completion, this is just an extra attribute.

The ids needs to come back in sequence.

I would need to hold the id somehwere until the completion is done. I would need to create a new IOCompletion that would hold the id, and each Context would have a PriorityQueue.

Having ordering at the global level also provides an overhead in increases latency where one session is waiting for completions from another session to complete - this is unnecessary as far as I can tell.

From the tests I have done, the out of order callback will come immediately. It's not like it would hold the callback out of order forever. That's very fast.


i.e. we would implement something on the OperationContext that is not really necessary IMO.