From b92c3c97a56c84af9b40197e34adc307a9d11a07 Mon Sep 17 00:00:00 2001 From: Ayu Ishii Date: Fri, 8 Apr 2022 23:59:50 +0000 Subject: [PATCH] Quota: Update comment on Commit()/ScheduledCommit() Address comments in https://crrev.com/c/3574803 Updates comments on the Commit()/ScheduledCommit() for bucket creation & deletion and the implication of these choices. As well as a TODO for a detailed plan on handling inconsistencies between the db and the file system directory. Bug: 1312955 Change-Id: Idfadf54a7a8789b60fa69847bd6d6b035874b605 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3577286 Reviewed-by: Austin Sullivan Commit-Queue: Ayu Ishii Cr-Commit-Position: refs/heads/main@{#990663} --- storage/browser/quota/quota_database.cc | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/storage/browser/quota/quota_database.cc b/storage/browser/quota/quota_database.cc index eca94c413c2875..70bd100d3029f0 100644 --- a/storage/browser/quota/quota_database.cc +++ b/storage/browser/quota/quota_database.cc @@ -561,11 +561,15 @@ QuotaError QuotaDatabase::DeleteBucketInfo(BucketId bucket_id) { // Scheduling this commit introduces the chance of inconsistencies // between the buckets table and data stored on disk in the file system. - // If there is a crash, or a battery failure before the transaction is + // If there is a crash or a battery failure before the transaction is // committed, the bucket directory may be deleted from the file system, // while an entry still may exist in the database. // - // TODO(crbug.com/1314129): Update to immediately commit transaction. + // While this is not ideal, this does not introduce any new edge case. + // We should check that bucket IDs have existing associated directories, + // because database corruption could result in invalid bucket IDs. + // TODO(crbug.com/1314567): For handling inconsistencies between the db and + // the file system. ScheduleCommit(); return QuotaError::kNone; @@ -1039,9 +1043,9 @@ QuotaErrorOr QuotaDatabase::CreateBucketInternal( int64_t bucket_id = db_->GetLastInsertRowId(); DCHECK_GT(bucket_id, 0); - // Commit so bucket data is persisted immediately. This reduces the chance of - // inconsistencies between the buckets table and data stored on disk in the - // filesystem. + // Commit immediately so that we persist the bucket metadata to disk before we + // inform other services / web apps (via the Buckets API) that we did so. + // Once informed, that promise should persist across power failures. Commit(); return BucketInfo(BucketId(bucket_id), storage_key, type, bucket_name,