Skip to content

Commit 02dd897

Browse files
committed
Add a schema migration that drops the query cache
This is a force fix for potential existence filter mismatches caused by #1548 The essential problem is that when resuming a query, the server is allowed to forget deletes. If the number of incorrectly synthesized deletes matches the number of server-forgotten deletes then the existence filter can give a false positive, preventing the cache from self healing. Dropping the query cache clears any client-side resume token which prevents a false positive existence filter mismatch. Note that the remote document cache and mutation queues are unaffected so any cached documents that do exist will still work while offline firebase/firebase-js-sdk#1019
1 parent 853b4de commit 02dd897

File tree

4 files changed

+157
-10
lines changed

4 files changed

+157
-10
lines changed

Firestore/Example/Tests/Local/FSTLevelDBMigrationsTests.mm

+80-5
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,22 @@
2323
#import "Firestore/Source/Local/FSTLevelDB.h"
2424
#import "Firestore/Source/Local/FSTLevelDBKey.h"
2525
#import "Firestore/Source/Local/FSTLevelDBMigrations.h"
26+
#import "Firestore/Source/Local/FSTLevelDBMutationQueue.h"
2627
#import "Firestore/Source/Local/FSTLevelDBQueryCache.h"
2728

2829
#include "Firestore/core/src/firebase/firestore/util/ordered_code.h"
30+
#include "Firestore/core/src/firebase/firestore/util/status.h"
31+
#include "Firestore/core/test/firebase/firestore/testutil/testutil.h"
2932
#include "leveldb/db.h"
3033

3134
#import "Firestore/Example/Tests/Local/FSTPersistenceTestHelpers.h"
3235

3336
NS_ASSUME_NONNULL_BEGIN
3437

38+
using firebase::firestore::FirestoreErrorCode;
3539
using firebase::firestore::local::LevelDbTransaction;
3640
using firebase::firestore::util::OrderedCode;
41+
using firebase::firestore::testutil::Key;
3742
using leveldb::DB;
3843
using leveldb::Options;
3944
using leveldb::Status;
@@ -94,11 +99,7 @@ - (void)testCountsQueries {
9499
}
95100
// Add a dummy entry after the targets to make sure the iteration is correctly bounded.
96101
// Use a table that would sort logically right after that table 'target'.
97-
std::string dummyKey;
98-
// Magic number that indicates a table name follows. Needed to mimic the prefix to the target
99-
// table.
100-
OrderedCode::WriteSignedNumIncreasing(&dummyKey, 5);
101-
OrderedCode::WriteString(&dummyKey, "targetA");
102+
std::string dummyKey = [self dummyKeyForTable:"targetA"];
102103
transaction.Put(dummyKey, "dummy");
103104
transaction.Commit();
104105
}
@@ -112,6 +113,80 @@ - (void)testCountsQueries {
112113
}
113114
}
114115

116+
#define ASSERT_NOT_FOUND(transaction, key) \
117+
do { \
118+
std::string unused_result; \
119+
Status status = transaction.Get(key, &unused_result); \
120+
XCTAssertTrue(status.IsNotFound()); \
121+
} while (0)
122+
123+
#define ASSERT_FOUND(transaction, key) \
124+
do { \
125+
std::string unused_result; \
126+
Status status = transaction.Get(key, &unused_result); \
127+
XCTAssertTrue(status.ok()); \
128+
} while (0)
129+
130+
- (void)testDropsTheQueryCache {
131+
NSString *userID = @"user";
132+
FSTBatchID batchID = 1;
133+
FSTTargetID targetID = 2;
134+
135+
FSTDocumentKey *key1 = Key("documents/1");
136+
FSTDocumentKey *key2 = Key("documents/2");
137+
138+
std::string targetKeys[] = {
139+
[FSTLevelDBTargetKey keyWithTargetID:targetID],
140+
[FSTLevelDBTargetDocumentKey keyWithTargetID:targetID documentKey:key1],
141+
[FSTLevelDBTargetDocumentKey keyWithTargetID:targetID documentKey:key2],
142+
[FSTLevelDBDocumentTargetKey keyWithDocumentKey:key1 targetID:targetID],
143+
[FSTLevelDBDocumentTargetKey keyWithDocumentKey:key2 targetID:targetID]};
144+
145+
std::string preservedKeys[] = {[self dummyKeyForTable:"targetA"],
146+
[FSTLevelDBMutationQueueKey keyWithUserID:userID],
147+
[FSTLevelDBMutationKey keyWithUserID:userID batchID:batchID]};
148+
149+
{
150+
// Setup some targets to be counted in the migration.
151+
LevelDbTransaction transaction(_db.get(), "testDropsTheQueryCache setup");
152+
[FSTLevelDBMigrations runMigrationsWithTransaction:&transaction upToVersion:1];
153+
154+
for (const std::string &key : targetKeys) {
155+
transaction.Put(key, "target");
156+
}
157+
158+
for (const std::string &key : preservedKeys) {
159+
transaction.Put(key, "preserved");
160+
}
161+
transaction.Commit();
162+
}
163+
164+
{
165+
LevelDbTransaction transaction(_db.get(), "testDropsTheQueryCache");
166+
[FSTLevelDBMigrations runMigrationsWithTransaction:&transaction upToVersion:2];
167+
168+
for (const std::string &key : targetKeys) {
169+
ASSERT_NOT_FOUND(transaction, key);
170+
}
171+
172+
for (const std::string &key : preservedKeys) {
173+
ASSERT_FOUND(transaction, key);
174+
}
175+
}
176+
}
177+
178+
/**
179+
* Creates the name of a dummy entry to make sure the iteration is correctly bounded.
180+
*/
181+
- (std::string)dummyKeyForTable:(const char *)tableName {
182+
std::string dummyKey;
183+
// Magic number that indicates a table name follows. Needed to mimic the prefix to the target
184+
// table.
185+
OrderedCode::WriteSignedNumIncreasing(&dummyKey, 5);
186+
OrderedCode::WriteString(&dummyKey, tableName);
187+
return dummyKey;
188+
}
189+
115190
@end
116191

117192
NS_ASSUME_NONNULL_END

Firestore/Source/Local/FSTLevelDBMigrations.h

+6
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@ typedef int32_t FSTLevelDBSchemaVersion;
3838
*/
3939
+ (void)runMigrationsWithTransaction:(firebase::firestore::local::LevelDbTransaction *)transaction;
4040

41+
/**
42+
* Runs any migrations needed to bring the given database up to the given schema version
43+
*/
44+
+ (void)runMigrationsWithTransaction:(firebase::firestore::local::LevelDbTransaction *)transaction
45+
upToVersion:(FSTLevelDBSchemaVersion)version;
46+
4147
@end
4248

4349
NS_ASSUME_NONNULL_END

Firestore/Source/Local/FSTLevelDBMigrations.mm

+63-5
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,16 @@
2424

2525
#include "Firestore/core/src/firebase/firestore/util/hard_assert.h"
2626
#include "absl/base/macros.h"
27+
#include "absl/memory/memory.h"
2728
#include "absl/strings/match.h"
2829
#include "leveldb/write_batch.h"
2930

3031
NS_ASSUME_NONNULL_BEGIN
3132

3233
// Current version of the schema defined in this file.
33-
static FSTLevelDBSchemaVersion kSchemaVersion = 2;
34+
static FSTLevelDBSchemaVersion kSchemaVersion = 3;
3435

3536
using firebase::firestore::local::LevelDbTransaction;
36-
using leveldb::DB;
3737
using leveldb::Iterator;
3838
using leveldb::Status;
3939
using leveldb::Slice;
@@ -87,6 +87,49 @@ static void AddTargetCount(LevelDbTransaction *transaction) {
8787
transaction->Put([FSTLevelDBTargetGlobalKey key], targetGlobal);
8888
}
8989

90+
/**
91+
* A class representing all migrations to prepare the LevelDB database for use.
92+
*
93+
* TODO(wilhuff): All the migrations should really be a part of this but to minimize the scope of
94+
* this change for now it's constrained to just the new one.
95+
*/
96+
class LevelDbSchema {
97+
public:
98+
explicit LevelDbSchema(leveldb::DB *db) : db_{db} {
99+
}
100+
101+
void DropQueryCache();
102+
103+
private:
104+
void DeleteEverythingWithPrefix(const std::string &prefix);
105+
106+
leveldb::DB *db_;
107+
std::unique_ptr<LevelDbTransaction> transaction_;
108+
};
109+
110+
void LevelDbSchema::DropQueryCache() {
111+
transaction_ = absl::make_unique<LevelDbTransaction>(db_, "Drop Query Cache");
112+
113+
DeleteEverythingWithPrefix([FSTLevelDBTargetKey keyPrefix]);
114+
DeleteEverythingWithPrefix([FSTLevelDBDocumentTargetKey keyPrefix]);
115+
DeleteEverythingWithPrefix([FSTLevelDBTargetDocumentKey keyPrefix]);
116+
117+
transaction_->Commit();
118+
}
119+
120+
void LevelDbSchema::DeleteEverythingWithPrefix(const std::string &prefix) {
121+
LevelDbTransaction reader(db_, "Reader");
122+
auto it = reader.NewIterator();
123+
124+
for (it->Seek(prefix); it->Valid() && absl::StartsWith(it->key(), prefix); it->Next()) {
125+
if (transaction_->changed_keys() >= 1000) {
126+
transaction_->Commit();
127+
transaction_ = absl::make_unique<LevelDbTransaction>(db_, "Drop Query Cache");
128+
}
129+
transaction_->Delete(it->key());
130+
}
131+
}
132+
90133
@implementation FSTLevelDBMigrations
91134

92135
+ (FSTLevelDBSchemaVersion)schemaVersionWithTransaction:
@@ -102,22 +145,37 @@ + (FSTLevelDBSchemaVersion)schemaVersionWithTransaction:
102145
}
103146

104147
+ (void)runMigrationsWithTransaction:(firebase::firestore::local::LevelDbTransaction *)transaction {
148+
[self runMigrationsWithTransaction:transaction upToVersion:kSchemaVersion];
149+
}
150+
151+
+ (void)runMigrationsWithTransaction:(firebase::firestore::local::LevelDbTransaction *)transaction
152+
upToVersion:(FSTLevelDBSchemaVersion)upToVersion {
105153
FSTLevelDBSchemaVersion currentVersion = [self schemaVersionWithTransaction:transaction];
106154
// Each case in this switch statement intentionally falls through. This lets us
107155
// start at the current schema version and apply any migrations that have not yet
108156
// been applied, to bring us up to current, as defined by the kSchemaVersion constant.
109157
switch (currentVersion) {
110158
case 0:
159+
if (upToVersion < 0) break;
111160
EnsureTargetGlobal(transaction);
112161
ABSL_FALLTHROUGH_INTENDED;
113162
case 1:
114163
// We're now guaranteed that the target global exists. We can safely add a count to it.
164+
if (upToVersion < 1) break;
115165
AddTargetCount(transaction);
116166
ABSL_FALLTHROUGH_INTENDED;
117-
default:
118-
if (currentVersion < kSchemaVersion) {
119-
SaveVersion(kSchemaVersion, transaction);
167+
case 2:
168+
if (upToVersion < 2) break;
169+
if (currentVersion != 0) {
170+
LevelDbSchema schema{transaction->database()};
171+
schema.DropQueryCache();
120172
}
173+
ABSL_FALLTHROUGH_INTENDED;
174+
default:
175+
break;
176+
}
177+
if (currentVersion < upToVersion) {
178+
SaveVersion(upToVersion, transaction);
121179
}
122180
}
123181

Firestore/core/src/firebase/firestore/local/leveldb_transaction.h

+8
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,14 @@ class LevelDbTransaction {
144144
*/
145145
static const leveldb::WriteOptions& DefaultWriteOptions();
146146

147+
leveldb::DB* database() const {
148+
return db_;
149+
}
150+
151+
size_t changed_keys() {
152+
return mutations_.size() + deletions_.size();
153+
}
154+
147155
/**
148156
* Remove the database entry (if any) for "key". It is not an error if "key"
149157
* did not exist in the database.

0 commit comments

Comments
 (0)