Page MenuHomePhabricator
Differential D13640

[keyserver] cache findUserIdentities response
ClosedPublic
Actions

Authored by varun on Oct 8 2024, 9:36 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 15, 6:09 AM
Unknown Object (File)
Thu, Nov 14, 3:51 PM
Unknown Object (File)
Thu, Nov 14, 3:50 PM
Unknown Object (File)
Thu, Nov 14, 2:57 PM
Unknown Object (File)
Thu, Nov 14, 12:58 PM
Unknown Object (File)
Sat, Nov 9, 3:56 AM
Unknown Object (File)
Sat, Nov 9, 2:23 AM
Unknown Object (File)
Thu, Nov 7, 8:53 AM
View All 48 Files
Subscribers
tomek

Details

Reviewers
ashoat
Commits
rCOMMe22e879f9a0d: [keyserver] cache findUserIdentities response
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
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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

What's the point of the IIFE?

This revision is now accepted and ready to land.Oct 9 2024, 5:57 AM
ashoat added inline comments.Oct 9 2024, 5:58 AM
keyserver/src/updaters/thread-updaters.js
970 ↗(On Diff #44988)
  1. Can we invert this condition to early exit?
  2. Wondering – why don't we cache null results?
varun added inline comments.Oct 9 2024, 9:41 AM
keyserver/src/updaters/thread-updaters.js
970 ↗(On Diff #44988)
  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
varun updated this revision to Diff 45060.Oct 10 2024, 8:51 AM

exit early and remove iife

This revision was landed with ongoing or failed builds.Oct 10 2024, 9:10 AM
Closed by commit rCOMMe22e879f9a0d: [keyserver] cache findUserIdentities response (authored by varun). · Explain Why
This revision was automatically updated to reflect the committed changes.
varun added a commit: rCOMMe22e879f9a0d: [keyserver] cache findUserIdentities response.
Harbormaster completed remote builds in B32180: Diff 45060.Oct 10 2024, 9:26 AM

Revision Contents
Changeset List

PathSize
keyserver/
src/
updaters/
20 lines

Diff 45062

View Options

keyserver/src/updaters/thread-updaters.js

Loading...