run the users-refresher-lambda every minute
#123
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
twrichards
force-pushed
the
trigger-user-refresher-on-permission-change
branch
14 times, most recently
from
4622d45
to
4c46d85
Compare
twrichards
force-pushed
the
trigger-user-refresher-on-permission-change
branch
from
4c46d85
to
71d3566
Compare
users-refresher-lambda/src/index.ts
Outdated
| ); | ||
|
|
||
| if (!emailsOfUsersWithPinboardPermission) { | ||
| throw Error("Could not get list of users with 'pinboard' permission."); | ||
| } | ||
|
|
||
| const isNightlyFullRun = now.getHours() === 0 && now.getMinutes() === 0; |
There was a problem hiding this comment.
Would it be worth adding a parameter to allow manually running a full run in case of errors/emergencies?
There was a problem hiding this comment.
yeah I actually was thinking about something like that. Perhaps two schedules...
- every minute, sends some
isProcessPermissionChangesOnly: trueflag in the payload - every 24 hours, no payload (i.e. same as a manual run)
There was a problem hiding this comment.
done in c4c5d67
twrichards
force-pushed
the
trigger-user-refresher-on-permission-change
branch
from
01b4e8e
to
cefb868
Compare
…ually pinboard permission changes before hitting Google APIs (except the midnight invocation, where we do a full update to bump the TTLs on the records)
…ffected) email rather than iterating through all the guardian users (500 at a time)
twrichards
force-pushed
the
trigger-user-refresher-on-permission-change
branch
from
cefb868
to
9364c2e
Compare
…rocessPermissionChangesOnly: true` in the payload) and every 24 hours (no payload, so should do full run)
twrichards
force-pushed
the
trigger-user-refresher-on-permission-change
branch
from
9364c2e
to
c4c5d67
Compare
andrew-nowak
approved these changes
Mar 31, 2022
|
Overdue on PROD (merged by @twrichards 5 minutes and 4 seconds ago) What's gone wrong? |
|
Seen on PROD (merged by @twrichards 6 minutes and 8 seconds ago) Please check your changes! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://trello.com/c/RIfC9Vqc/550-run-pinboard-user-refresher-lambda-on-permission-change
Previously the
users-refresher-lambdaran on a 6 hourly schedule (to keep load on google APIs low), which kept our list of users (needed for name resolution, avatars and to populate the mentions suggestions) somewhat up to date. However the recent live blog pilots highlighted the fact that we still needed to run this lambda manually after a permission change to avoid waiting up to 6 hours before users could safely use pinboard.Initially we hoped to trigger the
users-refresher-lambdawhenever permissions actually change (see #118) however without adding lots of complex infrastructure (including making changes to guardian/permissions) this is not possible.What does this change?
Instead, here we increase the frequency of the
users-refresher-lambdaschedule to every minute and with a payload of{isProcessPermissionChangesOnly: true}which checks there are actually pinboard permission changes and looks-up only those against the Google APIs. Note that users who have the permission removed are marked asisMentionable: falseWe also introduce another schedule, which daily does a full run to ensure users who've left are marked as
isMentionable: false.Mention suggestions are now only populated with users with
isMentionable: true. This new approach replaces the previous TTL concept (see #46 and #50) and retains user rows indefinitely, preferring a flag which is more explicit (and also makes the impending move away from Dynamo to a relational DB easier).How to test
With this deployed to CODE
NO CHANGE TO PINBOARD PERMISSIONS, exiting earlyin the logsDETECTED PINBOARD PERMISSIONS ADDED FORin the logs (followed by the usual log lines about upserting).DETECTED PINBOARD PERMISSIONS REMOVED FORin the logs (followed by the usual log lines about upserting).FULL RUNin the logs (followed by the usual log lines about upserting).How can we measure success?
No dev intervention required when CP add new users to pinboard, and they can start using pinboard safely within a minute.
Have we considered potential risks?
This will cost a touch more, but since pinboard currently costs only a few cents per month - this is OK.