RCORE-2006 Reuse realm file for sync schema migrations #7487
Conversation
Pull Request Test Coverage Report for Build daniel.tabacaru_782Details
💛 - Coveralls |
| @@ -832,7 +832,7 @@ void Group::remove_table(size_t table_ndx, TableKey key) | |||
| // tables. Such a behaviour is deemed too obscure, and we shall therefore | |||
| // require that a removed table does not contain foreign origin backlink | |||
| // columns. | |||
| if (table->is_cross_table_link_target()) | |||
| if (!ignore_backlinks && table->is_cross_table_link_target()) | |||
There was a problem hiding this comment.
Does this leave the db in a state where there are tables out there with invalid links? I guess in the place this is used right now that may not be important to consider because we remove all tables in a single WT, but what are the implications if an SDK started using this?
There was a problem hiding this comment.
indeed. we remove all tables so it should not cause any issue. I don't think we expose this to the SDKs (had a look in spec.yml and there is no reference). I could also have a private method and friend the class (or some other utility)
There was a problem hiding this comment.
Java was the only SDK which exposed table schema mutations directly and everything else only does it by setting the object store schema. Should probably add a comment in the declaration that ignore_backlinks=true will leave things in an invalid state just in case someone ends up using it in the future.
| @@ -270,4 +232,4 @@ void AsyncOpenTask::wait_for_bootstrap_or_complete(AsyncOpenCallback&& callback, | |||
| } | |||
| } | |||
|
|
|||
| } // namespace realm | |||
| } // namespace realm | |||
| // Delete all public tables (and their columns). | ||
| const bool ignore_backlinks = true; | ||
| for (const auto& tk : tr->get_table_keys()) { | ||
| // Do not remove the metadata tables |
There was a problem hiding this comment.
Do we actually want to leave the private tables? There shouldn't be anything that needs to be preserved, and deleting and recreating them is closer in behavior to what deleting the file did.
There was a problem hiding this comment.
Not really. I initially deleted those as well, but there was an issue with it. Just got an idea how I may be able to overcome that.
What, How & Why?
In the previous design of schema migrations, the RealmCoordinator was closed so the realm file can be deleted. This worked under the assumption that only one AsyncOpenTask was in progress at any time. Since apps may open a realm file multiple times at once, we've changed it so there is no need to delete the realm file and instead reuse the same file.
Upon uploading the unsynced changes, the client will:
Pause the sync session
Once the sync client releases the realm file:
Resume the session (the client asks for a new file ident)
Note: This still works under the assumption that a realm file is opened only asynchronously for a migration to work
Fixes #7442, #7325.
☑️ ToDos
[ ] 📝 Changelog update[ ] C-API, if public C++ API changed[ ]bindgen/spec.yml, if public C++ API changed