Save schema version on downgrade, add test to verify #2153
Conversation
Firestore/core/src/firebase/firestore/local/leveldb_migrations.cc
Outdated
Show resolved
Hide resolved
Firestore/core/src/firebase/firestore/local/leveldb_migrations.cc
Outdated
Show resolved
Hide resolved
| // detect it when we go to upgrade again, allowing us to rerun the | ||
| // data migrations. | ||
| if (from_version > to_version) { | ||
| LevelDbTransaction transaction(db, "Save downgrade version"); |
There was a problem hiding this comment.
Under the former transaction arrangement we could have used a single transaction to read the version and write the downgrade version. Is it worth trying to conserve transaction creation?
Normally I don't think this would matter, but we do these on every startup, and even though we're not blocking the main thread, these still contribute to the latency to first query.
Note that we can preserve the nice change to the tests by adding a local override in the tests that instantiates the transaction and then reads using the production code that doesn't.
There was a problem hiding this comment.
We could have, but we never did, and probably wouldn't. Some migrations run iteratively and commit periodically, so we don't run them all in a single transaction.
* master: (26 commits) Functions Interop (#2113) Add a travis cron job for CocoaPod symbol collision testing (#2154) Save schema version on downgrade, add test to verify (#2153) Silence Storage Unit Test `nil` warning. (#2150) Update versions for Release 5.14.0 (#2145) gRPC: fix cases where gRPC call could be finished twice (#2146) Fix Swizzler test warnings (#2144) Update Auth CHANGELOG.md (#2128) Make fuzz tests optional until they pass (#2143) Add support of Game Center sign in (#2127) Add test for deprecated FDLURLComponents init API. (#2133) fix a typo in integration test (#2131) Make fuzzing less verbose to avoid exceeding Travis log limit (#2126) Move to `domainURIPrefix` for FIRDynamicLinkComponents (#2119) Carthage instructions for new gRPCCertificates.bundle location (#2132) Fix pod lib lint GoogleUtilities.podspec --use-libraries regression (#2130) Avoid using default FIROptions directly. (#2124) Changelog entry for LRU GC (#2122) Revert "Add Firebase Source to Header Search Path" (#2123) Custom fdl domain (#2121) ...
Due to the nature of leveldb, our existing migrations are idempotent already. As with other platforms, this is not a guarantee that they will always be, but in general the issue is largely the same as with migrating protobufs.