1095010 - Room sessions that timeout don't get included in subsequent diffs in GET /rooms?version= requests

Status: RESOLVED DUPLICATE of bug 1093569

(Hello (Loop) :: Server, defect)

Description

[Mark Banner (:standard8)]

STR:

1) Create two rooms, get the list of all rooms via GET /rooms
2) Join one of them room with POST /rooms/{token}
3) Don't send a refresh, let the session timeout.

At this stage, no push notification is sent, which is by design.

4) Join the second room
5) Receive push notification, and do a GET /rooms?version={version}

Expected Results

- Return information on the leave and the join

Actual Results

- Only returns information for the join

If a subsequent GET /rooms without the version parameter is performed, then the first room is correctly shown as empty.

Comment 1

[Alexis Metaireau (:alexis)]

Right, we need to provide this kind of mechanism. Currently, we don't notify room owners on room delete and on room expire.

We currently don't have any mechanism on the server side to trigger something when a key has expired.

Here are a few ways to implement that:

1. Queuing using sets + timestamps

  - Have the keys which are about to expire stored in a redis set. The set key would be a timestamp with some granurality, to the second for instance.
  - Every second, get all the keys which are about to expire, check if they really are expired and
    in case they are, trigger a push notification.

2. Use a redis pubsub with multiple channels

  - Each time we need to add a key to the expiracy, we publish it to the redis pub/sub. We could have
    different channels to be able to do some round robin with the workers;
  - Each worker registers to the pubsub and when a message is sent to their channel, they proceed.

And of course potential other possibilities!

Comment 2

[Maire Reavy [:mreavy]]

Hey Alexis -- I know you're actively working bug 1111579.  This bug is the other big bug we need fixed (after bug 1111579).  We believe it is causing bug 1109102.  Do you think you can work on this next (after bug 1111579 is resolved)?   Thanks!

Comment 3

[Alexis Metaireau (:alexis)]

Yes, stacking this one just after.

Comment 4

[Alexis Metaireau (:alexis)]

We currently don't have any way to trigger an action when a value expires like that.

The rationale behind that is we try to avoid having timers running on the server side, so we let redis handle the expiration for us. The way redis works is that you aren't notified when a key expires, and in our case, we aren't notified when a user leave in a non-explicit way.

That being said, we need to work-out a solution for these things :) I can see a few ways to go here:

Redis notifications
===================

Redis apparently supports notifications [0], but only since v2.8, and as per the amazon docs [1] we only can have 2.6.13 so far.

[0] http://redis.io/topics/notifications
[1] http://docs.aws.amazon.com/AmazonElastiCache/latest/UserGuide/CacheParameterGroups.Redis.html


Do not rely on TTL to expire presence in rooms
==============================================

We currently are tracking the presence in the rooms with a redis set. As the users are TTLed in the spec, we thought the best way to proceed was to add a TTL to their records. It turns out that it's not simple to actually have the key deletion trigger an event (without the aforementioned notifications).

As such, one way to solve the problem is to move away from the TTL feature of redis and use something different. For instance, it could make sense to have jobs running in background, making the key expire when they should. By doing that, we could let them also populate an information for the server to look after when the client issues a GET /rooms/?version.

I believe there might be other solutions open for us, so I'm ccing a bunch of people who hopefully might have good ideas ;)

Comment 5

[Tarek Ziadé (:tarek)]

(In reply to Alexis Metaireau (:alexis) from comment #1)
> Right, we need to provide this kind of mechanism. Currently, we don't notify
> room owners on room delete and on room expire.
> 
> We currently don't have any mechanism on the server side to trigger
> something when a key has expired.
> 


Reading http://redis.io/topics/notifications - we can have a notification triggered when a key expires
or is triggered.  see __keyevent@0__:expire

I would build a separate process that subscribes to this event. We need to be careful on its design to make sure it scales.

Comment 6

[Tarek Ziadé (:tarek)]

oops sorry I read your second comment afterward... 

This is quite unfortunate. I guess we have to set up our own alarm system. I'll think about it

Comment 7

[Tarek Ziadé (:tarek)]

Reading http://aws.amazon.com/fr/blogs/aws/amazon-elasticache-now-with-redis-286/ it seems that AWS does have 2.8, so maybe your doc link is not up to date. 

We should give it a shot asap and see if the notification feature is present.

Comment 8

[Tarek Ziadé (:tarek)]

Dean confirmed we're running 2.8.6.

Comment 9

[Alexis Metaireau (:alexis)]

oh yeah! Then we're golden with this; let's do it this way.

Comment 10

[Maire Reavy [:mreavy]]

Hi Alexis --- How close are we to fixing this bug?  I need a server fix for this to be deployed so I can verify that Bug 1109102 (a client-side bug) is resolved and there are no lingering blocker issues.  Fx35 goes to release Jan 13, and the last Beta build is Monday (29th of Dec) and the release candidate is built 5th of Jan.  So I have maybe a week to fix a client-side issue if there is one (due to how the desktop release process works).  Thanks.

Comment 11

[Rémy Hubscher (:natim)]

We will start working on this today, I hope it will be fast-forward enough for us to deliver before Fx35.

Comment 12

[Alexis Metaireau (:alexis)]

Maire, we're in code-freeze until 1st of January (due to holidays), so expect no deployment until then. Otherwise, that's one of the things I'm working on, so we should have a fix soon.