Page MenuHomePhabricator

[identity] fix add_reserved_usernames RPC
ClosedPublic

Authored by varun on Jul 25 2023, 2:06 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 2, 11:52 PM
Unknown Object (File)
Sat, Nov 2, 11:51 PM
Unknown Object (File)
Sat, Nov 2, 11:49 PM
Unknown Object (File)
Sat, Nov 2, 11:29 PM
Unknown Object (File)
Tue, Oct 22, 2:30 PM
Unknown Object (File)
Tue, Oct 22, 2:30 PM
Unknown Object (File)
Tue, Oct 22, 2:28 PM
Unknown Object (File)
Tue, Oct 22, 5:03 AM
Subscribers

Details

Summary

Since there aren't that many users in the users table currently, it's cheaper to just get all the usernames from DDB and then compare with the list from keyserver using HashSets.

Also fixed the BatchWriteRequests -- DynamoDB didn't like that we were sending more than 25 items per request, so I broke up the usernames list into chunks of 25. Also, I removed the retry logic for unprocessed items because we should rarely have those.

Test Plan
  1. Inserted 1000 new users with usernames into my local MariaDB users table.
  2. Ran the cron job that calls the AddReservedUsernames RPC.
  3. Confirmed that all the new usernames were in the prod DynamoDB reserved usernames table. Existing users (e.g. ashoat), were skipped.
  4. Cleared the DDB table.

Diff Detail

Repository
rCOMM Comm
Branch
fix-batch-write
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

varun requested review of this revision.Jul 25 2023, 2:27 PM
varun added a subscriber: ashoat.

LGTM, just one nit

services/identity/src/database.rs
708–717

Why are you creating a single-item vec just to convert it to HashMap?
You can use HashMap::from

let item = HashMap::from([(
  RESERVED_USERNAMES_TABLE_PARTITION_KEY.to_string(),
   AttributeValue::S(username.to_string())
])

Or use a different method of PutRequest::builder()

let put_request = PutRequest::builder()
  .item(
    RESERVED_USERNAMES_TABLE_PARTITION_KEY,
    AttributeValue::S(username.to_string())
  )
  .build();
WriteRequest::builder().put_request(put_request).build()
This revision is now accepted and ready to land.Jul 26 2023, 3:18 AM
services/identity/src/database.rs
708–717

yeah latter looks a lot cleaner

This revision was automatically updated to reflect the committed changes.