Page MenuHomePhabricator

[identity] Query reserved usernames in FindUserIdentities RPC
ClosedPublic

Authored by bartek on Jul 29 2024, 11:38 AM.
Tags
None
Referenced Files
F3227073: D12924.id42980.diff
Mon, Nov 11, 8:33 PM
F3227060: D12924.id42980.diff
Mon, Nov 11, 8:27 PM
Unknown Object (File)
Sat, Nov 9, 11:36 PM
Unknown Object (File)
Sat, Nov 9, 7:45 PM
Unknown Object (File)
Wed, Nov 6, 10:49 PM
Unknown Object (File)
Mon, Oct 28, 9:41 AM
Unknown Object (File)
Sat, Oct 19, 8:23 AM
Unknown Object (File)
Fri, Oct 18, 9:46 PM
Subscribers

Details

Summary

Addresses server-side of ENG-8771.
Modified FindUserIdentities RPC to also look up the reserved usernames table.

Depends on D12922, D12923

Test Plan
  • Have a few users registered to identity
  • Manually added a few items to the reserved usernames table.
  • Call the RPC with user IDs from both tables, and also a few non-existent IDs
  • Verify that response contains usernames for both types of users

Also verified that previous methods that base on modified DatabaseClient::query_reserved_usernames_table() work the same way as before.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

bartek held this revision as a draft.
bartek published this revision for review.Jul 29 2024, 12:01 PM
bartek added inline comments.
services/identity/src/database.rs
1259–1265 ↗(On Diff #42912)

Introduced this to be able to monitor how big the numbers actually are and if we need to worry about performance

1267–1277 ↗(On Diff #42912)

Decided to do a traditional for loop instead of spawning threads, to avoid issues similar to our last OOM incident.
However, if the sequential query becomes too slow, I can try splitting this into chunks and spawn threads for each chunk of let's say 5-10 requests

Unfortunately, we cannot do anything like BatchGet here because we're querying an index.

services/identity/src/grpc_services/authenticated.rs
875–880 ↗(On Diff #42912)

I hope this will limit the number of queried user IDs over time. The more users registered on Identity we have, the less users will stay in the reserved usernames table

This revision is now accepted and ready to land.Jul 29 2024, 10:23 PM

Rename to reserved_user_identifiers