Skip to content

Index-free: Drop last limbo free snapshot if re-upgraded #734

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 23, 2019
Merged

Index-free: Drop last limbo free snapshot if re-upgraded #734

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
223fc1c
Drop last limbo free snapshot if re-upgraded
schmidt-sebastian Aug 23, 2019
65ff93d
Throw exception
schmidt-sebastian Aug 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 45 additions & 8 deletions firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteSchema.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package com.google.firebase.firestore.local;

import static com.google.firebase.firestore.util.Assert.fail;
import static com.google.firebase.firestore.util.Assert.hardAssert;

import android.content.ContentValues;
Expand All @@ -26,7 +27,9 @@
import androidx.annotation.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.firebase.firestore.model.ResourcePath;
import com.google.firebase.firestore.proto.Target;
import com.google.firebase.firestore.util.Consumer;
import com.google.protobuf.InvalidProtocolBufferException;
import java.util.ArrayList;
import java.util.List;

Expand Down Expand Up @@ -136,7 +139,16 @@ void runMigrations(int fromVersion, int toVersion) {
}

if (fromVersion < 9 && toVersion >= 9) {
addReadTime();
if (!hasReadTime()) {
addReadTime();
} else {
// Index-free queries rely on the fact that documents updated after a query's last limbo
// free snapshot version are persisted with their read-time. If a customer upgrades to
// schema version 9, downgrades and then upgrades again, some queries may have a last limbo
// free snapshot version despite the fact that not all updated document have an associated
// read time.
dropLastLimboFreeSnapshotVersion();
}
}

/*
Expand Down Expand Up @@ -363,14 +375,39 @@ private void addSequenceNumber() {
}
}

private boolean hasReadTime() {
boolean hasReadTimeSeconds = tableContainsColumn("remote_documents", "read_time_seconds");
boolean hasReadTimeNanos = tableContainsColumn("remote_documents", "read_time_nanos");

hardAssert(
hasReadTimeSeconds == hasReadTimeNanos,
"Table contained just one of read_time_seconds or read_time_nanos");

return hasReadTimeSeconds && hasReadTimeNanos;
}

private void addReadTime() {
if (!tableContainsColumn("remote_documents", "read_time_seconds")) {
hardAssert(
!tableContainsColumn("remote_documents", "read_time_nanos"),
"Table contained read_time_nanos, but is missing read_time_seconds");
db.execSQL("ALTER TABLE remote_documents ADD COLUMN read_time_seconds INTEGER");
db.execSQL("ALTER TABLE remote_documents ADD COLUMN read_time_nanos INTEGER");
}
db.execSQL("ALTER TABLE remote_documents ADD COLUMN read_time_seconds INTEGER");
db.execSQL("ALTER TABLE remote_documents ADD COLUMN read_time_nanos INTEGER");
}

private void dropLastLimboFreeSnapshotVersion() {
new SQLitePersistence.Query(db, "SELECT target_id, target_proto FROM targets")
.forEach(
cursor -> {
int targetId = cursor.getInt(0);
byte[] targetProtoBytes = cursor.getBlob(1);

try {
Target targetProto = Target.parseFrom(targetProtoBytes);
targetProto = targetProto.toBuilder().clearLastLimboFreeSnapshotVersion().build();
db.execSQL(
"UPDATE targets SET target_proto = ? WHERE target_id = ?",
new Object[] {targetProto.toByteArray(), targetId});
} catch (InvalidProtocolBufferException e) {
throw fail("Failed to decode Query data for target %s", targetId);
}
});
}

/**
Expand Down
73 changes: 73 additions & 0 deletions firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteSchemaTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static com.google.firebase.firestore.testutil.TestUtil.path;
import static com.google.firebase.firestore.testutil.TestUtil.query;
import static com.google.firebase.firestore.testutil.TestUtil.version;
import static com.google.firebase.firestore.util.Assert.fail;
import static java.util.Arrays.asList;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
Expand All @@ -36,10 +37,13 @@
import com.google.firebase.firestore.model.DocumentKey;
import com.google.firebase.firestore.model.ResourcePath;
import com.google.firebase.firestore.proto.MaybeDocument;
import com.google.firebase.firestore.proto.Target;
import com.google.firebase.firestore.proto.WriteBatch;
import com.google.firebase.firestore.remote.RemoteSerializer;
import com.google.firestore.v1.Document;
import com.google.firestore.v1.Write;
import com.google.protobuf.InvalidProtocolBufferException;
import com.google.protobuf.Timestamp;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -436,6 +440,68 @@ public void existingDocumentsRemainReadableAfterIndexFreeMigration() {
assertResultsContain(results, "coll/new");
}

@Test
public void dropsLastLimboFreeSnapshotIfPreviouslyDowngraded() {
schema.runMigrations(0, 9);

db.execSQL(
"INSERT INTO targets (target_id, canonical_id, target_proto) VALUES (?,?, ?)",
new Object[] {1, "foo", createDummyQueryTargetWithLimboFreeVersion(1).toByteArray()});
db.execSQL(
"INSERT INTO targets (target_id, canonical_id, target_proto) VALUES (?, ?, ?)",
new Object[] {2, "bar", createDummyQueryTargetWithLimboFreeVersion(2).toByteArray()});
db.execSQL(
"INSERT INTO targets (target_id, canonical_id, target_proto) VALUES (?,?, ?)",
new Object[] {3, "baz", createDummyQueryTargetWithLimboFreeVersion(3).toByteArray()});

schema.runMigrations(0, 8);
schema.runMigrations(8, 9);

int rowCount =
new SQLitePersistence.Query(db, "SELECT target_id, target_proto FROM targets")
.forEach(
cursor -> {
int targetId = cursor.getInt(0);
byte[] targetProtoBytes = cursor.getBlob(1);

try {
Target targetProto = Target.parseFrom(targetProtoBytes);
assertEquals(targetId, targetProto.getTargetId());
assertFalse(targetProto.hasLastLimboFreeSnapshotVersion());
} catch (InvalidProtocolBufferException e) {
fail("Failed to decode Query data");
}
});

assertEquals(3, rowCount);
}

@Test
public void keepsLastLimboFreeSnapshotIfNotDowngraded() {
schema.runMigrations(0, 9);

db.execSQL(
"INSERT INTO targets (target_id, canonical_id, target_proto) VALUES (?,?, ?)",
new Object[] {1, "foo", createDummyQueryTargetWithLimboFreeVersion(1).toByteArray()});

// Make sure that we don't drop the lastLimboFreeSnapshotVersion if we are already on schema
// version 9.
schema.runMigrations(9, 9);

new SQLitePersistence.Query(db, "SELECT target_proto FROM targets")
.forEach(
cursor -> {
byte[] targetProtoBytes = cursor.getBlob(0);

try {
Target targetProto = Target.parseFrom(targetProtoBytes);
assertTrue(targetProto.hasLastLimboFreeSnapshotVersion());
} catch (InvalidProtocolBufferException e) {
fail("Failed to decode Query data");
}
});
}

private SQLiteRemoteDocumentCache createRemoteDocumentCache() {
DatabaseId databaseId = DatabaseId.forProject("foo");
LocalSerializer serializer = new LocalSerializer(new RemoteSerializer(databaseId));
Expand All @@ -460,6 +526,13 @@ private byte[] createDummyDocument(String name) {
.toByteArray();
}

private Target createDummyQueryTargetWithLimboFreeVersion(int targetId) {
return Target.newBuilder()
.setTargetId(targetId)
.setLastLimboFreeSnapshotVersion(Timestamp.newBuilder().setSeconds(42))
.build();
}

private void assertResultsContain(
ImmutableSortedMap<DocumentKey, com.google.firebase.firestore.model.Document> actualResults,
String... docs) {
Expand Down