Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A way to flush/deliver messages before disconnecting #805

Closed
jaypatel512 opened this issue Dec 3, 2018 · 6 comments
Closed

A way to flush/deliver messages before disconnecting #805

jaypatel512 opened this issue Dec 3, 2018 · 6 comments

Comments

@jaypatel512
Copy link

Hey folks,

We are currently sending some messages, and based on a flag disconnect the clients if the conversation/process is finished. If so, we call client.disconnect() on it to make sure the client no longer keeps the connection open.

However, this is causing client to disconnect even before receiving the messages. I was hoping to have a client.disconnect(flushBeforeDisconnectBoolean) or a simple method as client.flush that sends all messages by blocking the call.

Is there a way to send messages and then wait for it to be delivered before disconnecting the client ?

@jaypatel512
Copy link
Author

This hints the need for completable futures for a bunch of client calls :) I am learning that and I can probably help in near future with that :)

@sbordet
Copy link
Member

sbordet commented Dec 3, 2018

I don't understand. If you want to disconnect, then you want to disconnect and therefore you cannot receive messages.
Blocking is not a great idea. How long do you want to wait?
Can you please detail your case?

@jaypatel512
Copy link
Author

@sbordet !

This is server disconnecting the client. We have an event that basically ends a chat, and if we receive that, we disconnect all clients after sending them the last message (e.g. "You are removed from the room".).

Our server sends the message, and then calls disconnect.

What I was hoping to accomplish here was:

serverSession.send(message, onDelivered() {
serverSession.disconnect();
});

Similar to this. CallbackFuture would be a perfect example for this.

@sbordet
Copy link
Member

sbordet commented Dec 4, 2018

@jaypatel512 this is already possible in CometD 4:

serverChannel.publish(null, data, Promise.complete((r, x) -> serverSession.disconnect()));

If you like Java's CompletableFuture, use Promise.Completable (a subclass of Java's CompletableFuture):

Promise.Completable<Boolean> completable = new Promise.Completable<>();
serverSession.deliver(null, "/channel", data, completable);
completable.thenAccept(b -> serverSession.disconnect());

@jaypatel512
Copy link
Author

Hey @sbordet !

We found an interesting bug with this in case of long-polling architecture. In some cases, we are seeing the messages being delivered before disconnect happens, and in some cases it disconnects beforehand.

The working theory in our head is, in case of long-polling (with multipleClient enabled), the messages is delivered to Server Queue successfully, and promise is returned once that is done. It seems like the promise is not waiting for the messages to be picked up by the browser before returning the promise.

I am still looking into the source to look at it, but not sure if you have an explanation on top of your head.

Thanks as always.

@sbordet
Copy link
Member

sbordet commented Dec 11, 2018

You have to clarify what you mean exactly by "disconnect". CometD disconnect message? TCP disconnect?

The server side promise cannot wait for the browser. The server side promise is completed as soon as the TCP write succeeds (or fails). Whether the message will arrive to the client, that is unknown to the server, and the server cannot wait.
It may still happen, however, that the messages and the disconnect message (from server to client) travel onto 2 different TCP connections and therefore there is no guarantee of ordering between the messages and the disconnect message.

The ack extension guarantees ordered delivery.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants