Skip to content

Schema migration that drops held write acks #33

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

Jump to bottom
Merged
merged 2 commits into from
Sep 21, 2018
Merged

Schema migration that drops held write acks #33

merged 2 commits into from
Sep 21, 2018

Conversation

schmidt-sebastian
Copy link
Contributor

@schmidt-sebastian schmidt-sebastian commented Sep 20, 2018 •
edited
Loading

gsoltis
gsoltis suggested changes Sep 21, 2018
View reviewed changes
firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteSchema.java Outdated

SQLiteStatement indexDeleter =
db.compileStatement(
"DELETE FROM document_mutations WHERE uid = ? AND path = ? AND batch_id = ?");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the path? Don't we want to delete all rows from a batch? Could we do DELETE FROM document_mutations WHERE uid = ? AND batch_id = ? and then maybe not even need the serializer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whaaaaaa. Nice. I didn't even think about that.

@gsoltis gsoltis assigned schmidt-sebastian and unassigned gsoltis Sep 21, 2018
@google-oss-bot
Copy link
Contributor

@schmidt-sebastian: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
check 64df2c3 link /test check
connected-check 64df2c3 link /test connected-check

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

gsoltis
gsoltis approved these changes Sep 21, 2018
View reviewed changes
Copy link
Contributor

@gsoltis gsoltis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Do we need the serializer for the webclient?

@schmidt-sebastian
Copy link
Contributor Author

LGTM. Do we need the serializer for the webclient?

We do need it since we would have to do an index-skip-scan. The data is encoded as user-path-batchId.

@schmidt-sebastian schmidt-sebastian merged commit b5fcc0f into firestore-held-write-acks Sep 21, 2018
@schmidt-sebastian schmidt-sebastian deleted the mrschmidt-schemamigration branch October 15, 2018 17:25
@saurabhdtu saurabhdtu mentioned this pull request Nov 20, 2018
Closed
@firebase firebase locked and limited conversation to collaborators Oct 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gsoltis gsoltis gsoltis approved these changes

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@schmidt-sebastian @google-oss-bot @gsoltis