firehoseio / firehose

Build realtime Ruby web applications. Created by the fine folks at Poll Everywhere.
http://firehose.io/
MIT License
726 stars 72 forks source link

Firehose returns nill messages when catching clients up through sequences that don't exist #51

Closed ProTip closed 8 years ago

ProTip commented 8 years ago

If the difference between the clients last sequence and the current max sequence is greater than the items available nill messages will be returned.

This can occur when the buffer size is different from the default buffer size and is caused by indexing into the message list on a non-existent index. Example:

Real buffer size = 1 Client last_sequence = 5 Current sequence = 100 Calculated diff = 95 https://github.com/firehoseio/firehose/blob/master/lib/firehose/server/channel.rb#L41 message_list.length == 1 message_list[95] == nil https://github.com/firehoseio/firehose/blob/master/lib/firehose/server/channel.rb#L51

I propose that after receiving the list from Redis we check the minimum available sequence number. Example: Minimum available sequence = 100

if last_sequence < message_list[0].sequence
  last_sequence = message_list[0].sequence - 1
bradgessler commented 8 years ago

@ProTip the fix has been implemented on the https://github.com/firehoseio/firehose/tree/message-buffer branch via issue https://github.com/firehoseio/firehose/issues/53. Could you deploy to staging environment and try to recreate the nil message issue you experienced last week to verify if this bug was fixed?

ProTip commented 8 years ago

Yes, I will check this first thing in the morning.

ProTip commented 8 years ago

Looks like the nil messages are gone. 👍