Page MenuHomePhabricator

Capture backup logs at the end of each data store transaction.
ClosedPublic

Authored by marcin on Feb 1 2024, 4:36 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 26, 2:44 PM
Unknown Object (File)
Mon, Dec 23, 10:05 PM
Unknown Object (File)
Mon, Dec 23, 10:05 PM
Unknown Object (File)
Mon, Dec 23, 10:05 PM
Unknown Object (File)
Mon, Dec 23, 10:05 PM
Unknown Object (File)
Mon, Dec 23, 10:05 PM
Unknown Object (File)
Mon, Dec 23, 10:05 PM
Unknown Object (File)
Mon, Dec 23, 10:05 PM
Subscribers

Details

Summary

This differential captures backup log at the end of each data store transaction to ensure all changes made to data stores are persisted as backup logs.
How does it ensure that backup logs are created for each change:

  1. The transaction starts
  2. Operations on db are executed.
  3. We read current logID from the db.
  4. We use this logID to create backup logs file.
  5. We write to the database logID incremented by 1.
  6. Transaction is committed.

Now if the app crashes before transaction is committed but after log file is created we are left with too small logID in the db since incrementing logID was rolled back.. So next run of captureBackupLogs will use the same logID thus overwriting previous file (which must bo considered invalid since it contains changes that weren't actually persisted in the db).

The advantage of this approach is that we have consistency between logs and database. Unfortunately it removes the possibility to schedule writing log bytes to a file on separate thread. We can prioritize safety for now and if we see that app is too slow we can consider going back to my initial approach that assumed backup log creation outside of transaction.

Test Plan

This has already been tested in D10835 since the test plan of this diff inlcuded introducing this change.

Diff Detail

Repository
rCOMM Comm
Branch
marcin/eng-5264
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

marcin requested review of this revision.Feb 1 2024, 4:53 AM

Are you sure this is everything we want? What about olm table operations which are called directly from CommCoreModule?

native/cpp/CommonCpp/NativeModules/PersistentStorageUtilities/DataStores/BaseDataStore.h
97 ↗(On Diff #36516)

we shouldn't use this funciton anymore but probably it's better to add this here for safety

Are you sure this is everything we want? What about olm table operations which are called directly from CommCoreModule?

It was agreed during a discussion with @kamil and @michal that backing up olm data and metadata tables is unnecessary and in worst case scenario erroneous.

Add capturing logs for the sync method as well

for other methods that modify database without using ops, there is a task: ENG-6681

This revision is now accepted and ready to land.Feb 6 2024, 5:59 AM