Page MenuHomePhabricator

[identity] Implement transactional device list updates
ClosedPublic

Authored by bartek on Dec 7 2023, 12:32 AM.
Tags
None
Referenced Files
F2151504: D10219.id34369.diff
Sun, Jun 30, 12:50 PM
F2150176: D10219.id34504.diff
Sun, Jun 30, 9:38 AM
F2149771: D10219.id.diff
Sun, Jun 30, 8:44 AM
Unknown Object (File)
Wed, Jun 26, 2:17 PM
Unknown Object (File)
Tue, Jun 25, 12:53 PM
Unknown Object (File)
Tue, Jun 25, 12:53 PM
Unknown Object (File)
Tue, Jun 25, 12:53 PM
Unknown Object (File)
Tue, Jun 25, 12:53 PM
Subscribers

Details

Summary

One of the most important commits in this stack. Added code that allows us to handle transactional device list updates. Adding/removing devices should generate a new device list (and in the future sign it), while keeping previous ones in database, to have its full history.

The transaction goes as follows:

  1. Previous device list timestamp is fetched from database (user table)
  2. All user's current devices are fetched from database (device table)
  3. A caller-defined function is called to:
    • Mutably modify the "current devices"
    • Return a database update to be performed in transaction
  4. A new device list is generated from the modified "current devices" (with new timestamp)
  5. The database update is performed in transaction:
    • Current devices are updated (closure-returned DDB operation is performed)
    • New device list is inserted with new timestamp
    • The timestamp in user table is updated to the new device list timestamp
      • Previous timestamp is checked to be equal to the one fetched in step 1. Transaction fails on mismatch

Depends on D10218

Test Plan

Having this closure:

transact_update_devicelist(db, user_id, |ref mut devices| {
    let new_device = DeviceRow {
      // omitted for clarity
    };
    devices.push(new_device.clone());

    Ok(TransactWriteItem::builder().put(Put::builder()
     .table_name("identity-devices")
     .set_item(Some(new_device.into()))
     .build()
    ))
  }).await
`

Tested a few scenarios, including:

  • Normal, the transaction succeeds
  • Malfolmed DDB operation - the function returned Error::AwsSdk(err) with my mistake
  • Added code that concurrently modifies the timestamp in the users table - transaction fails with DeviceListError::ConcurrentUpdate

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek held this revision as a draft.
bartek published this revision for review.Dec 7 2023, 1:07 AM
bartek added inline comments.
services/identity/src/database/device_list.rs
465–475 ↗(On Diff #34369)

This can be extracted to a separate function if requested.
Generally AWS errors are cumbersome to work with.

This revision is now accepted and ready to land.Dec 11 2023, 9:06 AM
  • Move function into impl DatabaseClient
  • Make the closure take ordered device IDs instead of randomly-ordered full device data