j3k0 / ganomede-notifications

Long-pull notification service for Ganomede
0 stars 0 forks source link

Single token per type:device #26

Closed elmigranto closed 8 years ago

elmigranto commented 8 years ago

Closes #25

elmigranto commented 8 years ago

So I've ran into this strange issue where code in Token.coffee can be compiled with coffee -c src/, but fails to load with require('coffee-script/register');. Any advice? just rewrote the thing

elmigranto commented 8 years ago

This should be ready. I don't have an ability to test actual sending atm, certainly not on physical Android device. Will try to run a sim in a couple of days, but that code part should stay intact anyways, since I only switched how tokens are stored.

To upgrade redis, run DB_FROM=X DB_TO=Y coffee src/push-api/token-storage.coffee; it will:

j3k0 commented 8 years ago

To upgrade redis, run DB_FROM=X DB_TO=Y coffee src/push-api/token-storage.coffee

From what I understand, this will make a backup of DB_FROM to DB_TO and upgrade DB_FROM.

Naive update procedure:

  1. stop notification/v1 server modules + workers
  2. run the database upgrade
  3. upgrade notification/v1 server modules to latest + workers
  4. restart the services.

Issue with that:

So I have some questions:

So there's another option..

  1. upgrade and restart server modules and workers first (without downtime).
  2. run the database upgrade (while keeping server modules running).

If it is possible not to remove the older tokens from DB, I can rollback if something went wrong with the server update. If not, remove them as a second step.

I'd love a flow like that:

  1. upgrade + restart server instances
  2. upgrade database (which keeps both old format and new format)
  3. cleanup old token keys

Is that easy and possible?

elmigranto commented 8 years ago

Okay, sure, I'll change it.

Is there any issues of having the older format notifications tokens lying around in the DB?

I wanted to keep format of redis keys as it was, but I guess changing it won't hurt, so we can keep both versions in DB and eventually remove older ones. Let's switch new version to use different naming scheme, so we won't have any downtime except restart of node processes. (So we'll have desired upgrade scenario — restart, upgrade data, cleanup.)

elmigranto commented 8 years ago

Changes made:

To upgrade:

Order doesn't really matter, though running upgrade script will overwrite existing values in hashes.

j3k0 commented 8 years ago

I know it's been a while but I'm finally running the upgrade:

working with { db: 0,
  newPrefix: 'notifications:push-tokens:data-v2',
  oldPrefix: 'notifications:push-tokens',
  mask: 'notifications:push-tokens:*' }
looking for keys to upgrade…
converting 343786 keys…
saving 345051 upgraded tokens…
Done! Upgraded 345051 tokens: { nUpdated: 429, nCreated: 344622 }

Seems expected to you?

I'm wondering what's the difference between nUpdated and nCreated.

note: it's the first time I'm running the script.

elmigranto commented 8 years ago

It's results for HSET:

My guess would be that some users had multiple tokens per device and we ended up overwriting same hash key with different tokens. If you'd like we can look into this and I'll write some redis scripts to count old tokens.


Basically it was "user" -> ["ios:xxx", "ios:yyy"] and we first wrote "xxx" under "user" -> "ios", and "yyy" after that, though I'm not sure on actual order, but it probably was most recently registred token gets written last.

j3k0 commented 8 years ago

If I understood well, basically we may have lost 429 out 345051 tokens... no big deal.

It could be because I upgraded the notification service before running the upgrade script. Crossing finger that it'll work now 🙏

elmigranto commented 8 years ago

basically we may have lost 429 out 34551 tokens

Well kind of. Same user had multiple tokens for the same device type and we only took latest one and discarded others. Kinda what we were aiming for in #25, though, so expected.