Skip to content

Commit 7cb4d78

Browse files
committed
Bug 1980177: Skip over origins and log a telemetry for origins with unknown metadata when performing idb maintenance.r=asuth,dom-storage-reviewers,toolkit-telemetry-reviewers,TravisLong
Created few telemetry metrics to track the number of times we need to load metadata from metadata file, restore metadata and the number of times we encounter an unknown metadata. This patch also introduces new QuotaManager::LoadFullOriginMetadataWithRestoreAndStatus which is similar to LoadFullOriginMetadataWithRestore but also returns whether the metadata file was restored or not. Differential Revision: https://phabricator.services.mozilla.com/D260057
1 parent 56106a9 commit 7cb4d78

File tree

5 files changed

+118
-14
lines changed

5 files changed

+118
-14
lines changed

dom/indexedDB/ActorsParent.cpp

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@
148148
#include "mozilla/dom/quota/UniversalDirectoryLock.h"
149149
#include "mozilla/dom/quota/UsageInfo.h"
150150
#include "mozilla/fallible.h"
151+
#include "mozilla/glean/DomIndexedDBMetrics.h"
151152
#include "mozilla/ipc/BackgroundParent.h"
152153
#include "mozilla/ipc/BackgroundUtils.h"
153154
#include "mozilla/ipc/InputStreamParams.h"
@@ -13199,7 +13200,7 @@ RefPtr<UniversalDirectoryLockPromise> Maintenance::OpenStorageDirectory(
1319913200
QuotaManager* quotaManager = QuotaManager::Get();
1320013201
MOZ_ASSERT(quotaManager);
1320113202

13202-
// Return a shared lock for <profile>/storage/*/*/idb
13203+
// Return a shared lock for <profile>/storage/repo(aPersistenceScope)/*/idb
1320313204
return quotaManager->OpenStorageDirectory(
1320413205
aPersistenceScope, OriginScope::FromNull(),
1320513206
ClientStorageScope::CreateFromClient(Client::IDB),
@@ -13363,6 +13364,11 @@ nsresult Maintenance::DirectoryWork() {
1336313364
continue;
1336413365
}
1336513366

13367+
// For all non-persistent types, temporary storage must already be
13368+
// initialized
13369+
MOZ_DIAGNOSTIC_ASSERT(
13370+
persistent || quotaManager->IsTemporaryStorageInitializedInternal());
13371+
1336613372
// XXX persistenceType == PERSISTENCE_TYPE_PERSISTENT shouldn't be a special
1336713373
// case...
1336813374
const auto persistenceTypeString =
@@ -13418,6 +13424,8 @@ nsresult Maintenance::DirectoryWork() {
1341813424
if (!persistent &&
1341913425
!quotaManager->IsTemporaryOriginInitializedInternal(
1342013426
metadata)) {
13427+
glean::idb_maintenance::fallback_fullrestore_metadata.Add();
13428+
1342113429
// XXX GetOriginMetadata, which skips loading the metadata file
1342213430
// and instead relies on parsing the origin directory name and
1342313431
// reconstructing the principal, may produce a different origin
@@ -13430,10 +13438,26 @@ nsresult Maintenance::DirectoryWork() {
1343013438
// In the future, it would be useful to report anonymized
1343113439
// origin strings via telemetry to help investigate and
1343213440
// eventually fix this mismatch.
13433-
QM_TRY_UNWRAP(
13434-
metadata,
13435-
quotaManager->LoadFullOriginMetadataWithRestore(originDir),
13441+
13442+
QM_TRY_INSPECT(
13443+
const auto& fullmetadataAndStatus,
13444+
quotaManager->LoadFullOriginMetadataWithRestoreAndStatus(
13445+
originDir),
1343613446
Ok{});
13447+
13448+
metadata = fullmetadataAndStatus.first;
13449+
if (fullmetadataAndStatus.second) {
13450+
/* metadata was restored */
13451+
glean::idb_maintenance::metadata_restored.Add();
13452+
}
13453+
13454+
QM_TRY(OkIf(quotaManager->IsTemporaryOriginInitializedInternal(
13455+
metadata)),
13456+
/* unexpected but still fine to just skip it */ Ok{},
13457+
/* increment telemetry in case of failure */
13458+
[](const auto&) {
13459+
glean::idb_maintenance::unknown_metadata.Add();
13460+
});
1343713461
}
1343813462

1343913463
// We now use a dedicated repository for private browsing
@@ -13508,7 +13532,6 @@ nsresult Maintenance::DirectoryWork() {
1350813532
if (!persistent) {
1350913533
auto maybeOriginStateMetadata =
1351013534
quotaManager->GetOriginStateMetadata(metadata);
13511-
1351213535
auto originStateMetadata = maybeOriginStateMetadata.extract();
1351313536

1351413537
// Skip origin maintenance if the origin hasn't been accessed

dom/indexedDB/metrics.yaml

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
# This Source Code Form is subject to the terms of the Mozilla Public
2+
# License, v. 2.0. If a copy of the MPL was not distributed with this
3+
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
4+
5+
# Adding a new metric? We have docs for that!
6+
# https://firefox-source-docs.mozilla.org/toolkit/components/glean/user/new_definitions_file.html
7+
8+
---
9+
$schema: moz://mozilla.org/schemas/glean/metrics/2-0-0
10+
$tags:
11+
- 'Core :: Storage: IndexedDB'
12+
13+
idb.maintenance:
14+
fallback_fullrestore_metadata:
15+
type: counter
16+
description: >
17+
Tracks the number of times we need to fallback to restore metadata object by reading
18+
metadata-v2 file on disk when performing idle-maintenance on IDB database.
19+
bugs:
20+
- https://bugzilla.mozilla.org/show_bug.cgi?id=1980177
21+
data_reviews:
22+
- https://bugzilla.mozilla.org/show_bug.cgi?id=1980177
23+
data_sensitivity:
24+
- technical
25+
notification_emails:
26+
- storage-telemetry@mozilla.com
27+
expires: never
28+
29+
metadata_restored:
30+
type: counter
31+
description: >
32+
Counts the number of times we failed to load metadata object and had to restore it
33+
bugs:
34+
- https://bugzilla.mozilla.org/show_bug.cgi?id=1980177
35+
data_reviews:
36+
- https://bugzilla.mozilla.org/show_bug.cgi?id=1980177
37+
data_sensitivity:
38+
- technical
39+
notification_emails:
40+
- storage-telemetry@mozilla.com
41+
expires: never
42+
43+
unknown_metadata:
44+
type: counter
45+
description: >
46+
Increments upon encountering an unknown metadata object when performing
47+
idle-maintenance on IDB database. During idle-maintenance, we iterate
48+
over idb directories/files and construct a metadata object by calling
49+
QuotaManager::GetOriginMetadata or LoadFullOriginMetadataWithRestore.
50+
It seems that in some cases, the metadata object we get here can be
51+
invalid and this metric is used to track those instances.
52+
bugs:
53+
- https://bugzilla.mozilla.org/show_bug.cgi?id=1980177
54+
data_reviews:
55+
- https://bugzilla.mozilla.org/show_bug.cgi?id=1980177
56+
data_sensitivity:
57+
- technical
58+
notification_emails:
59+
- storage-telemetry@mozilla.com
60+
expires: never

dom/quota/ActorsParent.cpp

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3755,6 +3755,16 @@ Result<FullOriginMetadata, nsresult> QuotaManager::LoadFullOriginMetadata(
37553755

37563756
Result<FullOriginMetadata, nsresult>
37573757
QuotaManager::LoadFullOriginMetadataWithRestore(nsIFile* aDirectory) {
3758+
QM_TRY_INSPECT(const auto& result,
3759+
LoadFullOriginMetadataWithRestoreAndStatus(aDirectory));
3760+
3761+
// The first element of the pair is the FullOriginMetadata, the second is the
3762+
// restore status.
3763+
return result.first;
3764+
}
3765+
3766+
Result<std::pair<FullOriginMetadata, bool /* restore status */>, nsresult>
3767+
QuotaManager::LoadFullOriginMetadataWithRestoreAndStatus(nsIFile* aDirectory) {
37583768
// XXX Once the persistence type is stored in the metadata file, this block
37593769
// for getting the persistence type from the parent directory name can be
37603770
// removed.
@@ -3767,16 +3777,23 @@ QuotaManager::LoadFullOriginMetadataWithRestore(nsIFile* aDirectory) {
37673777

37683778
const auto& persistenceType = maybePersistenceType.value();
37693779

3770-
QM_TRY_RETURN(QM_OR_ELSE_WARN(
3771-
// Expression.
3772-
LoadFullOriginMetadata(aDirectory, persistenceType),
3773-
// Fallback.
3774-
([&aDirectory, &persistenceType,
3775-
this](const nsresult rv) -> Result<FullOriginMetadata, nsresult> {
3776-
QM_TRY(MOZ_TO_RESULT(RestoreDirectoryMetadata2(aDirectory)));
3780+
bool restored = false;
37773781

3778-
QM_TRY_RETURN(LoadFullOriginMetadata(aDirectory, persistenceType));
3779-
})));
3782+
QM_TRY_INSPECT(
3783+
const auto& fullOriginMetadata,
3784+
QM_OR_ELSE_WARN(
3785+
// Expression.
3786+
LoadFullOriginMetadata(aDirectory, persistenceType),
3787+
// Fallback.
3788+
([&aDirectory, &persistenceType, &restored,
3789+
this](const nsresult rv) -> Result<FullOriginMetadata, nsresult> {
3790+
restored = true;
3791+
QM_TRY(MOZ_TO_RESULT(RestoreDirectoryMetadata2(aDirectory)));
3792+
3793+
QM_TRY_RETURN(LoadFullOriginMetadata(aDirectory, persistenceType));
3794+
})));
3795+
3796+
return std::make_pair(fullOriginMetadata, restored);
37803797
}
37813798

37823799
Result<OriginMetadata, nsresult> QuotaManager::GetOriginMetadata(

dom/quota/QuotaManager.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,9 @@ class QuotaManager final : public BackgroundThreadObject {
304304
Result<FullOriginMetadata, nsresult> LoadFullOriginMetadataWithRestore(
305305
nsIFile* aDirectory);
306306

307+
Result<std::pair<FullOriginMetadata, bool /* restore status */>, nsresult>
308+
LoadFullOriginMetadataWithRestoreAndStatus(nsIFile* aDirectory);
309+
307310
Result<OriginMetadata, nsresult> GetOriginMetadata(nsIFile* aDirectory);
308311

309312
Result<Ok, nsresult> RemoveOriginDirectory(nsIFile& aDirectory);

toolkit/components/glean/metrics_index.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
"dom/canvas/metrics.yaml",
3030
"dom/crypto/metrics.yaml",
3131
"dom/geolocation/metrics.yaml",
32+
"dom/indexedDB/metrics.yaml",
3233
"dom/localstorage/metrics.yaml",
3334
"dom/media/eme/metrics.yaml",
3435
"dom/media/hls/metrics.yaml",

0 commit comments

Comments
 (0)