Avoid initial version pinning #1519
Conversation
packages/library-base/src/commonMain/kotlin/io/realm/kotlin/internal/RealmImpl.kt
Outdated
Show resolved
Hide resolved
| val writerVersion = writer.version | ||
|
|
||
| if (initialRealmReference != null) { | ||
| if (initialRealmReference.isClosed()) return initialRealmReference |
There was a problem hiding this comment.
Can we even get here if the Realm is closed? I would assume we had other checks preventing this?
packages/library-base/src/commonMain/kotlin/io/realm/kotlin/internal/RealmImpl.kt
Outdated
Show resolved
Hide resolved
packages/library-base/src/commonMain/kotlin/io/realm/kotlin/internal/RealmImpl.kt
Outdated
Show resolved
Hide resolved
packages/library-base/src/commonMain/kotlin/io/realm/kotlin/internal/RealmImpl.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Hmm. I feel that this is a bit convoluted. There are some pretty nice clean ups splitting the version tracker's track and clean and how to select the most recent of notifier and writer versions. But couldn't we just rely on existing signalling and these new addition to solve the use case? Happy to jump on a call and discuss.
| /** | ||
| * Indicates whether the live realm has been initialized or not. | ||
| */ | ||
| val isInitialized: AtomicBoolean = atomic(false) |
There was a problem hiding this comment.
Why is the realmInitializer not good enough for this? Seems to be serving the exact same purpose 🤔
There was a problem hiding this comment.
I was runnning into some race conditions so I made it atomic, seems it is no longer an issue.
packages/library-base/src/commonMain/kotlin/io/realm/kotlin/internal/RealmImpl.kt
Outdated
Show resolved
Hide resolved
packages/library-base/src/commonMain/kotlin/io/realm/kotlin/internal/RealmImpl.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
There are some issues with the implementation.
Also noted that the VersionTracker.versions are actually not printing the garbage collecting versions even though they have not been closed 🙈 We should probably change it so that it was distinct if the version was no longer reference and/or closed.
packages/library-base/src/commonMain/kotlin/io/realm/kotlin/internal/VersionTracker.kt
Outdated
Show resolved
Hide resolved
packages/library-base/src/commonMain/kotlin/io/realm/kotlin/internal/VersionTracker.kt
Outdated
Show resolved
Hide resolved
packages/library-base/src/commonMain/kotlin/io/realm/kotlin/internal/RealmImpl.kt
Show resolved
Hide resolved
packages/library-base/src/commonMain/kotlin/io/realm/kotlin/internal/RealmImpl.kt
Show resolved
Hide resolved
packages/library-base/src/commonMain/kotlin/io/realm/kotlin/internal/RealmImpl.kt
Outdated
Show resolved
Hide resolved
| public data class VersionInfo(val main: VersionData?, val notifier: VersionData?, val writer: VersionData?) { | ||
| val all: Set<VersionId> = setOf(main, notifier, writer).mapNotNull { it?.versions }.flatten().toSet() | ||
| val allTracked: Set<VersionId> = setOf(main, notifier, writer).mapNotNull { it?.active }.flatten().toSet() | ||
| public data class VersionInfo(val notifier: VersionData?, val writer: VersionData?) { |
There was a problem hiding this comment.
Why don't we track the initial version anymore? Isn't it as important to know if the initial version is released as it is with the versions tracked by the notifier and writer 🤔
There was a problem hiding this comment.
I removed because I was testing it now in a different test case: initialVersionDereferencedAfterFirstWrite. But it is true that it is better to keep it.
packages/library-base/src/commonMain/kotlin/io/realm/kotlin/internal/VersionTracker.kt
Outdated
Show resolved
Hide resolved
packages/test-base/src/commonTest/kotlin/io/realm/kotlin/test/common/VersionTrackingTests.kt
Outdated
Show resolved
Hide resolved
…common/VersionTrackingTests.kt Co-authored-by: Claus Rørbech <[email protected]>
During the Realm initialization, we pinned the initial version then it was never released because the
VersionTrackerwas not cleaned up.In this PR we only track in the user-facing Realm the initial snapshot, and we do so until the notifier or writer has a snapshot available. We keep the version tracker, but now is triggered on each notifier/writer update, to allow flushing of the initial realm snapshot.