deltachat / notifiers

Notify devices
Apache License 2.0
6 stars 0 forks source link

Store token registration and last successful notification timestamp in the database #15

Closed link2xt closed 7 months ago

link2xt commented 9 months ago

We currently have >14k valid tokens that are notified without errors. I don't expect that we have this many iOS users.

Would be nice to store registration timestamp in addition to the token and not notify it if it is too old.

r10s commented 9 months ago

Would be nice to store registration timestamp in addition to the token and not notify it if it is too old.

i just discussed that with @zeitschlag:

so, expiring within a few days or even weeks will result in notifications not to work (esp. the new PUSH notification), as deltachat-ios does not have a chance to update the token.

expiration after several weeks or some months may make sense - but as it seems as if expired tokens result in an error anyways, that case is already caught.

I don't expect that we have this many iOS users.

we have over 60k downloads since we started in app-store and regularly more than 500 testflight users, so maybe sth. in the magnitude of 14k tokens is reasonable.

so, maybe wrt expiration everything's fine. but if the heartbeat does not work that way any longer, maybe we can also increase the timespan. it is somehow mainly needed to also remind apple to not forget "local wakeup" - when the app is really in used, i regularily get local wakeups every 20 minutes anyways.

with the "real PUSH", the heartbeat would become less important anyways, any maybe also can be tuned out over time

link2xt commented 9 months ago

I think we should at least store last registration date for the token and see how many tokens we have that are not re-registered for more than a month. E.g. I have no idea what will happen to the token if device is factory-reset or just gets broken while the app is still installed.

with the "real PUSH", the heartbeat would become less important anyways, any maybe also can be tuned out over time

This is not going to happen with non-chatmail providers, heartbeats are not going away in the future.

link2xt commented 9 months ago

but if the heartbeat does not work that way any longer, maybe we can also increase the timespan.

I think the only problem is that we currently send notifications one-by-one synchronously. I tried to fix it with #14 but this needs better error handling and pacing, trying to notify 14k tokens at once hits rate limits on the Apple side. We should track "last notification" timestamp for each token so we don't notify a token too frequently and postpone notifying tokens if we hit an temporary error ("GOAWAY" or something like that not indicating that token is broken).

Notifying 14k users every 20 minutes should not be a problem, pretty sure news apps, social media etc. send much more notifications than we do.

r10s commented 9 months ago

I think we should at least store last registration date for the token and see how many tokens we have that are not re-registered for more than a month.

makes sense, yes. at least to get a feeling and to gather some stats.

the main point is that we should not expire tokens within somehow reasonable days or weeks (user installed the app, gives the address to someone, they write back 4 weeks later where the keys for the flat are :)

E.g. I have no idea what will happen to the token if device is factory-reset or just gets broken while the app is still installed.

i read somewhere that on device-reinstall / factory-reset, deltachat-ios will get a new token. i'd assume the old token to result in an error when trying to use that on the apple sever.

hpk42 commented 9 months ago

Wouldn't WA or signal or telegram send more than 14k tokens to apple at once? What exactly is the rate limit about? Is it per connection? With 500 million active users there should be 100k iOS users getting notified at any time, no?

Note that https://developer.apple.com/library/archive/technotes/tn2265/_index.html#//apple_ref/doc/uid/DTS40010376-CH1-TNTAG44 claims there are no limits but might be outdated?

link2xt commented 9 months ago

Wouldn't WA or signal or telegram send more than 14k tokens to apple at once?

Why would they send them at once? This is only needed if a message arrives in a group with 14k users. Telegram with its channels or Twitter might do it, push notification server should just process the errors and backoff sending if it hits the limit.

link2xt commented 7 months ago

We are using sled which essentially provides a disk-backed map/dictionary structure. Currently we use it as a set, storing dummy values and using tokens as the keys: https://github.com/deltachat/notifiers/blob/9ebe037d8e905e5b9c947f9e40a5afaaa7055c38/src/server.rs#L33

To make it easy to find the least recently notified token we need to use timestamp of the next notification as the key and token as the value. We can ensure that we don't use the same key twice by appending unique id to the timestamp. When registering a new token we can schedule it as if it was just notified because when device is registering the key it is likely running Delta Chat in the foreground.

We also need to ensure that we don't have the same token registered multiple times. For that we need another dictionary, mapping the token to the scheduled timestamp. When token is scheduled, we store timestamp in this dictionary. When we retrieve token from the schedule, we can compare the latest stored timestamp and if it does not match, then we can drop scheduled token without notifying it.

link2xt commented 7 months ago

I implemented this in #28. I did not change current database scheme, actual implementation is simpler than described in previous post because there is no need to persist map from timestamp to token. It is reconstructed on application restart, so we only need to persist map from token to latest notification timestamp.