wandenberg / nginx-push-stream-module

A pure stream http push technology for your Nginx setup. Comet made easy and really scalable.
Other
2.22k stars 295 forks source link

Long polling - end of batch indicator #26

Closed sebcante closed 12 years ago

sebcante commented 12 years ago

Hi wandenberg,

This is a small feature request to make the client of pushstream.js aware when it is the end of a batch of messages (long polling)

Added a End of batch flag for client of pushtream.js to know when it is finished receiving a long polling batch of messages. End of batch flag is set to true for all transports except long polling when the client receives a batch of messages at once in between 2 long polling request. This is to help client of pushstream.js to implement some filter logic at batch level (for example discard stale messages).

Use case : high push rate, longPollingInterval medium high

Let me know if this could be incorporated in pushstream.js or may be if there is smarter way to do this.

cheers

-seb

wandenberg commented 12 years ago

Hi seb,

I didn't understand the use case clearly. Why you will discard a message just because it isn't the last message? The last message to one user could not be the last message to another user, and with that you will have different behaviors to different users.

I understand that change add more flexibility to the application, but I'm afraid that it could cause some misunderstanding and can bring some work to me in the future :)

It is really needed?

On Mon, Apr 30, 2012 at 1:08 AM, sebcante < reply@reply.github.com

wrote:

Hi wandenberg,

This is a small feature request to make the client of pushstream.js aware when it is the end of a batch of messages (long polling)

Added a End of batch flag for client of pushtream.js to know when it is finished receiving a long polling batch of messages. End of batch flag is set to true for all transports except long polling when the client receives a batch of messages at once in between 2 long polling request. This is to help client of pushstream.js to implement some filter logic at batch level (for example discard stale messages).

Use case : high push rate, longPollingInterval medium high

Let me know if this could be incorporated in pushstream.js or may be if there is smarter way to do this.

cheers

-seb

You can merge this Pull Request by running:

git pull https://github.com/sebcante/nginx-push-stream-modulelp_end_of_batch_indicator

Or you can view, comment on it, or merge it online at:

https://github.com/wandenberg/nginx-push-stream-module/pull/26

-- Commit Summary --

  • Added a End of batch flag for client of pushtream.js to know when it is finished receiving a long polling batch. End of batch flag is set to true for all transport except long polling when the client can receive a batch of messages at once in between 2 long polling request. This is to help client pushstream.js to implement some filter logic at batch level (for example discard stale messages)

-- File Changes --

M misc/js/pushstream.js (16)

-- Patch Links --

https://github.com/wandenberg/nginx-push-stream-module/pull/26.patch https://github.com/wandenberg/nginx-push-stream-module/pull/26.diff


Reply to this email directly or view it on GitHub: https://github.com/wandenberg/nginx-push-stream-module/pull/26

sebcante commented 12 years ago

Hi wanderberg,

sorry for the confusion. I ll try to explain a bit more.

For example where the publisher sends real time vote results every seconds, each message (push) would override each other as the last one would be the most current. For http streaming, the client receives each push realtime and display them real time, no issue here. In case of long polling where longPollingInterval is configured to 4-5 seconds, the long polling client would receive in one long polling response 4-5 messages. 4 would be stale, the client would need to display just the last one .

The reason why i would like to get the end of batch indicator, would be to be able to discard stale messages for long polling client.

May be there is the another to achieve this. I will have another think and come back to you.

cheers

-seb

wandenberg commented 12 years ago

I applied the patch, only removed the comments, I plan to add more tests to pushstream.js, add documentation, with the use of this variable, and make it part of module code base, not a "misc" anymore.

As I said, I understand the flexibility of this change, only expect to not have problems with that in the feature and all developers understand that it is only informative. :D

cheers

sebcante commented 12 years ago

thanks wanderberg!