Page MenuHomePhabricator
Differential D13640

[keyserver] cache findUserIdentities response
AcceptedPublic
Actions

Authored by varun on Tue, Oct 8, 9:36 PM.

Details

Reviewers
ashoat
Summary

when possible, we should get the user identity in question from the redis cache. if we can't get it, then we call identity and -- in the background -- cache the response

Depends on D13639

Test Plan

called getUserFarcasterID in a loop 10 times with a 10 second sleep in between each call and confirmed that only the first invocation of the function led to an identity service API call

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

varun created this revision.Tue, Oct 8, 9:36 PM
Herald added a subscriber: tomek. · View Herald TranscriptTue, Oct 8, 9:36 PM
Harbormaster completed remote builds in B32118: Diff 44988.Tue, Oct 8, 9:53 PM
varun requested review of this revision.Tue, Oct 8, 9:53 PM
ashoat accepted this revision.Wed, Oct 9, 5:57 AM
ashoat added inline comments.
keyserver/src/updaters/thread-updaters.js
971–975

What's the point of the IIFE?

This revision is now accepted and ready to land.Wed, Oct 9, 5:57 AM
ashoat added inline comments.Wed, Oct 9, 5:58 AM
keyserver/src/updaters/thread-updaters.js
970
  1. Can we invert this condition to early exit?
  2. Wondering – why don't we cache null results?
varun added inline comments.Wed, Oct 9, 9:41 AM
keyserver/src/updaters/thread-updaters.js
970
  1. yeah
  2. to avoid overcomplicating the API. we could store null results but then redisCache.getUserIdentity and the above function would have to distinguish between null and undefined. this seems error-prone and i'm not sure it's worth the trouble. in this case, we only call getUserFarcasterID with the viewer's userID, so we expect the identity service to have this user's identity. the other place where we might want to use this cache is in updateRelationships, where we call findUserIdentities. caching null results might be more useful here but i'm not sure

Revision Contents
Changeset List

PathSize
keyserver/
src/
updaters/
24 lines

Diff 44988

View Options

keyserver/src/updaters/thread-updaters.js

Loading...