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

Events published in same millisecond aren't seen by clients #19

Closed
concreted opened this issue May 7, 2020 · 3 comments
Closed

Events published in same millisecond aren't seen by clients #19

concreted opened this issue May 7, 2020 · 3 comments
Labels

Comments

@concreted
Copy link

When two events are published to the LongpollManager in the same millisecond, the behavior I observed is that clients will receive the first event only. The other events in the same millisecond can be seen by trying a request again with the same since_time, but if clients listen again using the timestamp received (as advised in https://github.com/jcuga/golongpoll#http-subscription-handler), they will never see those events.

For example, here is sample output from a client listening to a simple golongpoll app that is hardcoded to publish two events in quick succession every 10s:

# Initial call
> curl -i "localhost:8081/events?category=connectivity&timeout=60&since_time=1588816900233"
HTTP/1.1 200 OK
Cache-Control: no-cache, no-store, must-revalidate
Content-Type: application/json
Expires: 0
Pragma: no-cache
Date: Thu, 07 May 2020 02:01:50 GMT
Content-Length: 97

{"events":[{"timestamp":1588816910237,"category":"connectivity","data":{"message":"message 1"}}]}

# If calling again with the same since_time, I see the event with duplicate timestamp:
> curl -i "localhost:8081/events?category=connectivity&timeout=60&since_time=1588816900233"
HTTP/1.1 200 OK
Cache-Control: no-cache, no-store, must-revalidate
Content-Type: application/json
Expires: 0
Pragma: no-cache
Date: Thu, 07 May 2020 02:01:51 GMT
Content-Length: 182

{"events":[{"timestamp":1588816910237,"category":"connectivity","data":{"message":"message 1"}},{"timestamp":1588816910237,"category":"connectivity","data":{"message":"message 2"}}]}                         

# If calling again with the received timestamp, the "message 2" message is never received.
> curl -i "localhost:8081/events?category=connectivity&timeout=60&since_time=1588816910237"
HTTP/1.1 200 OK
Cache-Control: no-cache, no-store, must-revalidate
Content-Type: application/json
Expires: 0
Pragma: no-cache
Date: Thu, 07 May 2020 02:02:00 GMT
Content-Length: 97

{"events":[{"timestamp":1588816920241,"category":"connectivity","data":{"message":"message 1"}}]}

Is this behavior expected? I am planning to work around this by forcing a 1ms delay between publishes, but perhaps this could be handled in the LongpollManager by aggregating all events in the same millisecond and sending them together?

@jcuga jcuga added the bug label Jan 12, 2021
@jcuga
Copy link
Owner

jcuga commented Jan 23, 2021

@concreted this is absolutely a bug. The real fix would be to use a counter instead of timestamp to pick up where the previous longpoll results left off, but that would be a breaking change to any existing clients.

I will see if I can come up with a fix that still allows clients to use the since_time query parameter that doesn't require major refactoring.

I plan on revisiting this library and making a new version addressing all of the feedback I've received thus far, so if I can't find an easy fix for this now, then it will be addressed in the next major version.

Thanks for reporting this.

jcuga pushed a commit that referenced this issue Feb 14, 2021
… tests that weren't testing said functions
jcuga pushed a commit that referenced this issue Feb 17, 2021
- Added last_id to subscription handler
- Updated getEventsSince to consider lastEventID when searching events
that match sinceTime
- Removed deprecated CreateManager, CreateCustomManager functions
jcuga pushed a commit that referenced this issue Apr 15, 2021
… tests that weren't testing said functions
jcuga pushed a commit that referenced this issue Apr 15, 2021
- Added last_id to subscription handler
- Updated getEventsSince to consider lastEventID when searching events
that match sinceTime
- Removed deprecated CreateManager, CreateCustomManager functions
@jcuga
Copy link
Owner

jcuga commented Apr 29, 2021

Fixed in v1.3.0

@jcuga jcuga closed this as completed Apr 29, 2021
@jcuga
Copy link
Owner

jcuga commented Apr 29, 2021

The fix requires using the new last_id subscription request parameter (documented in HttpLongPollAPI.md).

The updated golang client and new js-client already use this new param Only custom clients will need to update to use this to get the fixed behavior.

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

No branches or pull requests

2 participants