Closed Bug 1093569 Opened 10 years ago Closed 9 years ago

[Rooms] Server doesn't update the client when a participant expires in a room

Categories

(Hello (Loop) :: Server, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: standard8, Assigned: alexis+bugs)

References

Details

(Whiteboard: [loop-server 0.15.0])

Attachments

(1 file)

56 bytes, text/x-github-pull-request
rhubscher
: review+
Details | Review
I'm currently working on the desktop client implementation of rooms.

STR:

1) Set config.rooms.participantTTL to something like 10 or 20 seconds.
2) Send a POST /rooms/{roomToken} with a "join" action and other appropriate data
3) Don't send anything else.

Expected Results:

- After 20 seconds (plus a factor), there's a push notification to the client to inform them that the connection has timed out and the room list is updated.

Actual Results:

- No push notification.

This is detailed in the original spec here:

https://wiki.mozilla.org/Loop/Architecture/Rooms#Room_Membership_and_Soft_State
This has been discussed with abr and we decided to not do this notification, instead we are doing it on the next even which is not participant expiration.

If we want to handle this expiration thing, we will need to add a queue that will be consumed and we decided it was a too big machinery for the benefit for now.

I NI Adam here for confirmation.
Flags: needinfo?(adam)
Rémy's recollection is correct here: the machinery of running active timers to proactively notify participants of room departures via push is relatively expensive, and is only a partial solution anyway (standalone clients don't get push notifications, so we need another solution for them anyway). I tried to capture this in the last part of the cited section, beginning with "Note that the server does not need to proactively track timeouts to make this scheme work."

As with standalone notification of participants joining a room, room departures will be discovered by participants via the SDK.
Flags: needinfo?(adam)
Actually, shortly after posting this, I realized that Mark is talking about keeping the room list up to date when the desktop user isn't in the room. There won't be an SDK notification in those cases. This may require some additional work to get working properly. I'm going to have to think about this a bit more and get back to you.
Blocks: 1095010
(In reply to Adam Roach (Travelling through Nov 18th) [:abr] from comment #3)
> Actually, shortly after posting this, I realized that Mark is talking about
> keeping the room list up to date when the desktop user isn't in the room.
> There won't be an SDK notification in those cases. This may require some
> additional work to get working properly. I'm going to have to think about
> this a bit more and get back to you.

Bug 1095010 shows a case where we currently never get informed of the update - we might fix that with this bug, or we might need a separate one.
Borrowing some parts of the conversation we've had in bug 1095010, we want to use the notification feature that's present from Redis 2.8 (that we have in production).

Specifically, the interesting bits from the documentation are:

> Keys with a time to live associated are expired by Redis in two ways:
> 
>     When the key is accessed by a command and is found to be expired.
>     Via a background system that looks for expired keys in background, incrementally, in order to be able to also collect keys that are never accessed.
> 
> The expired events are generated when a key is accessed and is found to be expired by one of the above systems, as a result there are no guarantees that the Redis server will be able to generate the expired event at the time the key time to live reaches the value of zero.
> 
> If no command targets the key constantly, and there are many keys with a TTL associated, there can be a significant delay between the time the key time to live drops to zero, and the time the expired event is generated.
> 
> Basically expired events are generated when the Redis server deletes the key and not when the time to live theorically reaches the value of zero.
 
Specifically this is what Redis does 10 times per second:

> Test 100 random keys from the set of keys with an associated expire.
> Delete all the keys found expired.
> If more than 25 keys were expired, start again from step 1.

Now, we need to find out if this can be tweaked somehow. From what I see on Amazon, it's not the case: http://docs.aws.amazon.com/AmazonElastiCache/latest/UserGuide/CacheParameterGroups.Redis.html
Assignee: nobody → alexis+bugs
Attached file link to github PR
Attachment #8547613 - Flags: review?(rhubscher)
Attachment #8547613 - Flags: review?(rhubscher) → review+
Hop, and this is now in master!
https://github.com/mozilla-services/loop-server/commit/50a1405da70f09194bb2bbb009b1edaed4152b70
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [loop-server 0.15.0]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: