[identity] fix add_reserved_usernames RPC
ClosedPublic
Actions

Authored by varun on Jul 25 2023, 2:06 PM.

Details

Reviewers

• jon
bartek

Commits

rCOMMf7bc1ceae2ea: [identity] fix add_reserved_usernames RPC

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

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

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

varun created this revision.Jul 25 2023, 2:06 PM

Herald added a subscriber: tomek. · View Herald TranscriptJul 25 2023, 2:07 PM

Harbormaster completed remote builds in B21199: Diff 29035.Jul 25 2023, 2:27 PM

varun requested review of this revision.Jul 25 2023, 2:27 PM

varun edited reviewers, added: bartek; removed: ashoat.Jul 25 2023, 2:54 PM

varun added a subscriber: ashoat.

LGTM, just one nit

services/identity/src/database.rs

708–717 ↗

(On Diff #29035)

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()