1009970 - Consider decrementing current_load column when executing 'back-channel' deletes.

Status: RESOLVED FIXED

(Cloud Services Graveyard :: Server: Token, defect)

Description

[Bob Micheletto [:bobm]]

Presently Token Server does not decrement the 'current_load' column when deleting users with the 'replaced-at' column set.  Consider decrementing this column, as well as incrementing the 'available' column.

Comment 1

[Ryan Kelly [:rfkelly]]

Another option would be to do this as a periodic maintenance task, where we calculate the actual load of each node with something like:

   SELECT COUNT(*) FROM users WHERE nodeid=blah AND replaced_at IS NULL

And then update the "available" and "current_load" columns to match.  It may even be possible to make a single query out of this with a group-by or a sub-select.

We could even do both I guess, it's not like any of the cleanup operations are in a fast-path.

Comment 2

[Ryan Kelly [:rfkelly]]

I tried this out of curiosity in stage:

  UPDATE nodes SET current_load = (SELECT COUNT(*) FROM users WHERE nodeid=id AND replaced_at IS NULL);

It runs in about 2.5 seconds there.  I imagine it would take a little longer in prod :-)

Maybe worth investigating some more, but I'll push ahead on the "decrement on row cleanup" idea for now.

Comment 3

[Ryan Kelly [:rfkelly]]

Attachment: ts-free-slot-on-node.diff

This is what the later would look like.  It's an extra query but the delete_user_record call is only ever done from our background script, so I'm not too worries about doing it more efficiently.

It does highlight the unfortunately fact they we process these old records one item at a time, though...

Comment 4

[Toby Elliott [:telliott]]

Comment on attachment 8444955 [details] [diff] [review]
ts-free-slot-on-node.diff

Review of attachment 8444955 [details] [diff] [review]:
-----------------------------------------------------------------

Code is fine, but I'm not sure it's worth worrying about. Deletes are likely to be an infrequent event, and the algorithm is sufficiently broad that this kind of work around the margins isn't likely to have an impact.

Comment 5

[Ryan Kelly [:rfkelly]]

Committed in https://github.com/mozilla-services/tokenserver/commit/362cb2e6751fd2e2efaca0d115349fbb8640d2cc since it won't do any harm, and might help keep things a bit more sensible from ops POV.