diff --git a/src/GameCult.Caching.MessagePack/DirectoryMessagePackBackingStore.cs b/src/GameCult.Caching.MessagePack/DirectoryMessagePackBackingStore.cs index a3ecf051..60b51057 100644 --- a/src/GameCult.Caching.MessagePack/DirectoryMessagePackBackingStore.cs +++ b/src/GameCult.Caching.MessagePack/DirectoryMessagePackBackingStore.cs @@ -311,17 +311,14 @@ public sealed class DirectoryMessagePackBackingStore : CacheBackingStore var holdsIds = written.ToDictionary(key => key, key => Entries[key].HoldsIds, StringComparer.Ordinal); var header = HeaderForWrite( currentManifest.FormatVersion, - holdsIds.Values.Select(holds => (bool?)holds), - currentIndex.Values.Where(record => !written.Contains(record.Key)), + holdsIds.Values.Select(holds => (bool?)holds).ToArray(), + copiesAny: currentIndex.Values.Any(record => !written.Contains(record.Key)), directoryStore: true, holdsVariants: false); WriteManifest(targetCatalog, currentIndex.Values .OrderBy(record => record.Key, StringComparer.Ordinal) .ToArray(), header); - foreach (var key in written) - Entries[key].StoredHoldsIds = holdsIds[key]; - DeleteUnreferencedRecordPages(currentIndex.Values); _dirtyKeys.Clear(); diff --git a/src/GameCult.Caching/Contracts/cultcache-persistence-format.md b/src/GameCult.Caching/Contracts/cultcache-persistence-format.md index b879d09d..9dfd0c1c 100644 --- a/src/GameCult.Caching/Contracts/cultcache-persistence-format.md +++ b/src/GameCult.Caching/Contracts/cultcache-persistence-format.md @@ -182,7 +182,7 @@ A record that resolves to no local type is a foreign record, for example one of have, or one renamed without declaring its old id. A store never destroys or relabels it: the store carries it byte for byte under its own id and its own catalog entry, lists it as foreign (`CacheBackingStore.ForeignRecords` in C#), and the cache never holds it. Writes of other records proceed and copy it forward as the file holds it. A write that would replace or remove it is refused with the typed schema conflict -naming its key and id, and nothing is written. The refused keys are reloaded as the store holds them (a foreign record is not held), and their staged changes are forgotten. A marked header stays marked while a store carries a record the writer cannot show to hold no element id. Declaring its id on a type claims it at the next load. +naming its key and id, and nothing is written. The refused keys are reloaded as the store holds them (a foreign record is not held), and their staged changes are forgotten. A write that carries a record forward keeps a marked header marked. Declaring its id on a type claims it at the next load. A reader resolves a record's schema id to the entry that owns it, that entry's own `schemaId`, and only when no entry owns the id to an entry that lists it as a compatible id (the first such entry @@ -311,12 +311,14 @@ The headers are `v1` (no variant, no element ids), `v2` (holds a variant) and `v an element id, variants or not). A single-file store declares `v3` when any record it holds carries an element id: a non-empty `[CultElementId]` value in a record's document, or in a variant's override values. A store that cannot read a variant's override values -(no codec attached) counts the variant as holding one. A whole-store writer decides by -what the store holds now, so the file drops back to `v1` (or `v2` with a variant) when -the last id is taken away; a writer that touches only part of the file cannot see the -rest, so a file already declared `v3` stays `v3` when it merges onto it. A directory -store's manifest says `cultcache.store.v5.directory-content-addressed-pages` under the -same rule, and `v4` otherwise. +(no codec attached) counts the variant as holding one. An element id is an ordinary keyed +string in the payload, so no reader can tell from a record's bytes whether it holds one; +a writer knows it only for the records it encodes. A write that copies any record forward +therefore keeps a `v3` header it found. Only a write that encodes every record the store +will hold decides by content, and drops to `v1` (or `v2` with a variant) when none holds +an id. A directory store's manifest says +`cultcache.store.v5.directory-content-addressed-pages` under the same rule, and `v4` +otherwise. A single-file store is exactly one MessagePack array of three slots: the header, the schema catalog and the records. Every runtime refuses a file that is truncated, has bytes after the diff --git a/src/GameCult.Caching/CultCache.cs b/src/GameCult.Caching/CultCache.cs index e1a6934f..7f585ab4 100644 --- a/src/GameCult.Caching/CultCache.cs +++ b/src/GameCult.Caching/CultCache.cs @@ -526,11 +526,6 @@ namespace GameCult.Caching // True while this record holds element ids minted when it loaded, that no write has persisted (see MintElementIds). internal bool IdsInMemoryOnly { get; set; } - // Whether the stored bytes of this record hold an element id: read when a store loaded the record, before any id was minted - // for it, and set when a store wrote it. Null when not known (a variant is not read at load). HoldsIds is what the document - // holds now; this is what the store holds, so a document changed in place and never written does not change it. - internal bool? StoredHoldsIds { get; set; } - // True when some element this record persists holds a non-empty id, and the only thing a store reads to mark its header. // It is read off the stored form each time it is asked, so a store asks it as it writes: a document changed after it was // admitted, or loaded and changed in place, writes the header its content needs. A plain record persists its document; a @@ -3462,24 +3457,17 @@ namespace GameCult.Caching return directoryStore ? DirectoryFormatV4 : holdsVariants ? CultPersistedStoreSnapshot.FormatV2 : CultPersistedStoreSnapshot.FormatV1; } - // The header a write leaves, for either store kind: HeaderFor decides, given what the write can see. Marked when a record it - // writes holds an id; kept marked while the store carries a record it cannot show holds none (an opaque payload it writes, or - // a copied record that is not known id-free). A copied record is known id-free when this store holds - // that key under the same stored identity as the file and knows its stored bytes hold no id (StoredHoldsIds). - protected string HeaderForWrite(string? durableHeader, IEnumerable written, IEnumerable copied, bool directoryStore, bool holdsVariants) - { - var holds = written.ToArray(); - var holdsIds = holds.Any(value => value == true); - var seen = HeaderFor(holdsIds, durableHeader, wholeStore: true, directoryStore, holdsVariants); - var unseen = HeaderFor(holdsIds, durableHeader, wholeStore: false, directoryStore, holdsVariants); - return seen == unseen || (holds.All(value => value != null) && copied.All(KnownIdFree)) ? seen : unseen; - } - - private bool KnownIdFree(CultPersistedRecord record) => - Entries.TryGetValue(record.Key, out var held) && - held.StoredSchemaId == record.SchemaId && - held.StoredAt == record.StoredAt && - held.StoredHoldsIds == false; + // The header a write leaves, for either store kind: HeaderFor decides. A record's bytes do not say whether it holds an + // element id (an id is an ordinary keyed string), so only the store header carries that fact. A write that copies any + // record forward keeps a marked header; a write that encodes every record the store will hold decides by content. An + // opaque payload (null) never marks an unmarked store, and keeps a marked one marked. + protected static string HeaderForWrite(string? durableHeader, IReadOnlyCollection written, bool copiesAny, bool directoryStore, bool holdsVariants) => + HeaderFor( + holdsIds: written.Any(value => value == true), + durableHeader, + wholeStore: !copiesAny && written.All(value => value != null), + directoryStore, + holdsVariants); // Once attached, a hold on the cache's gate: the store and its cache share one lock, so a load calling back into // the cache can never take the gate after the store lock, and a direct call publishes what it loaded when it @@ -3591,8 +3579,7 @@ namespace GameCult.Caching resolution.Descriptor, document) { - StoredSchemaId = record.SchemaId, - StoredHoldsIds = CultElementIds.Holds(document) + StoredSchemaId = record.SchemaId }; } @@ -3957,8 +3944,8 @@ namespace GameCult.Caching { FormatVersion = HeaderForWrite( durable?.FormatVersion, - upserts.Select(staged => staged.HoldsIds), - copied, + upserts.Select(staged => staged.HoldsIds).ToArray(), + copiesAny: copied.Length > 0, directoryStore: false, holdsVariants: records.Any(record => record.Variant != null)), SchemaCatalog = CultSchemaCatalogEntry.Derive( @@ -4074,7 +4061,6 @@ namespace GameCult.Caching continue; entry.StoredSchemaId = write.Record.SchemaId; entry.StoredAt = write.Record.StoredAt; - entry.StoredHoldsIds = write.HoldsIds; } Cache?.Wrote(staged.Select(write => write.Record)); diff --git a/tests/GameCult.Caching.Tests/ElementIdTests.cs b/tests/GameCult.Caching.Tests/ElementIdTests.cs index d7a47875..cfed4d27 100644 --- a/tests/GameCult.Caching.Tests/ElementIdTests.cs +++ b/tests/GameCult.Caching.Tests/ElementIdTests.cs @@ -84,6 +84,43 @@ namespace GameCult.Caching.Tests File.WriteAllBytes(path, CultDocumentMessagePackSerialization.SerializeSnapshot(snapshot)); } + // An element id is an ordinary keyed string, so a record's bytes do not say whether it holds one: only the store header does. + // A cache whose type cannot see the ids (a later shape with no reels, declared compatible or resolved by name) that copies the + // record forward leaves the store marked, the record untouched, and its ids readable by the type that wrote them. + [Test] + public void ANarrowerTypeFlushingAnotherRecordKeepsAMarkedStoreMarked([Values] bool declared, [Values] bool directory) + { + var path = PathOf("narrow.cc"); + var deckSchemaId = Registry.GetRequired(typeof(IdDeck)).SchemaId; + var deck = Deck("r"); + using (var owner = CultCacheMessagePack.Create(path, new CultCacheOpenOptions { Registry = Registry, UseDirectoryStore = directory })) + owner.Commit(batch => batch.Upsert(typeof(IdDeck), deck, new CultRecordKey("r"))); + var marked = directory ? "cultcache.store.v5.directory-content-addressed-pages" : CultPersistedStoreSnapshot.FormatV3; + var before = CultDocumentMessagePackSerialization.DeserializeSnapshot(File.ReadAllBytes(path)); + Assert.That(before.FormatVersion, Is.EqualTo(marked)); + + var narrow = EmittedDocumentTypes.Emit( + "NarrowDeck" + (declared ? "Declared" : "ByName"), + "tests.element_id_deck", + declared ? "tests.element_id_deck.v9" : "tests.element_id_deck.v8", + new[] { new EmittedDocumentTypes.Field("Name", typeof(string), 0, IsName: true) }, + declared ? new[] { deckSchemaId } : null); + using (var cache = CultCacheMessagePack.Create(path, new CultCacheOpenOptions { Registry = CultDocumentRegistry.ForTypes(new[] { narrow }), UseDirectoryStore = directory })) + { + Assert.That(cache.Get(new CultRecordKey("r")), Is.Not.Null, "the narrower type reads the record it will copy"); + cache.UpsertAsync(narrow, EmittedDocumentTypes.New(narrow, ("Name", "y")), new CultRecordKey("y")).GetAwaiter().GetResult(); + cache.FlushAllBackingStores(); + } + + var after = CultDocumentMessagePackSerialization.DeserializeSnapshot(File.ReadAllBytes(path)); + Assert.That(after.FormatVersion, Is.EqualTo(marked)); + var kept = before.Records.Single(record => record.Key == "r"); + var copied = after.Records.Single(record => record.Key == "r"); + Assert.That($"{copied.SchemaId}|{copied.StoredAt}", Is.EqualTo($"{kept.SchemaId}|{kept.StoredAt}")); + using var reader = CultCacheMessagePack.Create(path, new CultCacheOpenOptions { Registry = Registry, UseDirectoryStore = directory }); + Assert.That(reader.Get(new CultRecordKey("r"))!.Reels.Select(reel => reel.Id), Is.EqualTo(deck.Reels.Select(reel => reel.Id))); + } + private static CultPersistedRecord DiskRecord(string path, string key) => CultDocumentMessagePackSerialization.DeserializeSnapshot(File.ReadAllBytes(path)).Records.Single(record => record.Key == key); diff --git a/tests/GameCult.Caching.Tests/WriteSetTests.cs b/tests/GameCult.Caching.Tests/WriteSetTests.cs index bdd354fc..da8f9424 100644 --- a/tests/GameCult.Caching.Tests/WriteSetTests.cs +++ b/tests/GameCult.Caching.Tests/WriteSetTests.cs @@ -311,10 +311,10 @@ namespace GameCult.Caching.Tests Write(path, snapshot); } - // A marked header says the store holds an element id. A writer that copies only records it knows hold none leaves it - // unmarked (marked-by-content); one that copies a record it cannot show holds none keeps the mark. + // A marked header says the store holds an element id, and a record's bytes cannot say whether it does. A write that copies + // any record forward therefore keeps the mark, whatever this cache wrote or read of that record. [Test] - public void AMarkedHeaderIsDroppedOnlyWhenEveryCopiedRecordIsKnownToHoldNoId([Values] bool anotherWriterAddedARecord) + public void AMarkedHeaderIsKeptWhileTheWriteCopiesAnyRecord([Values] bool anotherWriterAddedARecord) { var path = PathOf("mark.cc"); Seed(path, false, "a"); @@ -329,11 +329,24 @@ namespace GameCult.Caching.Tests Land(cache, commit: false, typeof(WsItem), new WsItem { Name = "name-y" }, new CultRecordKey("y")); - Assert.That(Read(path).FormatVersion, Is.EqualTo(anotherWriterAddedARecord ? CultPersistedStoreSnapshot.FormatV3 : CultPersistedStoreSnapshot.FormatV1)); + Assert.That(Read(path).FormatVersion, Is.EqualTo(CultPersistedStoreSnapshot.FormatV3)); } - // A copied record is known to hold no id only while the store holds it as this cache read it: another writer changed it after - // the load, so the cache cannot say what it holds now. + // Only a write that encodes every record the store will hold can say none holds an id. + [Test] + public void AMarkedHeaderIsDroppedOnlyByAWriteThatEncodesEveryRecord([Values] bool commit) + { + var path = PathOf("mark-all.cc"); + Seed(path, false, "a"); + MarkAs(path, CultPersistedStoreSnapshot.FormatV3); + using var cache = Open(path, false); + + Land(cache, commit, typeof(WsItem), new WsItem { Name = "name-a", Note = "again" }, A); + + Assert.That(Read(path).FormatVersion, Is.EqualTo(CultPersistedStoreSnapshot.FormatV1)); + } + + // Another writer changed a copied record after this cache read it, and the mark stays: a copied record is not read to decide it. [Test] public void AMarkedHeaderIsKeptWhenAnotherWriterChangedACopiedRecordAfterTheLoad() { @@ -350,9 +363,9 @@ namespace GameCult.Caching.Tests Assert.That(Read(path).FormatVersion, Is.EqualTo(CultPersistedStoreSnapshot.FormatV3)); } - // What this cache wrote it knows: the record it stored without ids counts as holding none the next time it copies it. + // A copied record stays copied whoever wrote it: what this cache wrote earlier does not make the next write one that decides by content. [Test] - public void AMarkedHeaderIsDroppedWhenEveryRecordThisCacheWroteIsKnownToHoldNoId() + public void AMarkedHeaderIsKeptWhenEveryCopiedRecordWasWrittenByThisCache() { var path = PathOf("mark-wrote.cc"); Seed(path, false, "a"); @@ -362,7 +375,7 @@ namespace GameCult.Caching.Tests Land(cache, commit: false, typeof(WsItem), new WsItem { Name = "name-z" }, new CultRecordKey("z")); - Assert.That(Read(path).FormatVersion, Is.EqualTo(CultPersistedStoreSnapshot.FormatV1)); + Assert.That(Read(path).FormatVersion, Is.EqualTo(CultPersistedStoreSnapshot.FormatV3)); } // A batch is the write that lands last: it replaces a staged write of the same key. @@ -544,19 +557,19 @@ namespace GameCult.Caching.Tests // Two writers whose clocks put a record's storedAt ahead of both mint the same storedAt over it. The second to write // replaces the first's different bytes, so it is stored later than the first's, and the first's copy is told it moved. - // The equal storedAt is the case, and the mint is from the stored one, not from the writer's clock (which is behind it). + // The equal storedAt is the case, and the mint is from the stored one, not from the writer's clock (which is behind it). Either store kind. [Test] - public void AWriteOverARecordAtItsOwnStoredAtIsStoredLaterWhateverTheClockSays([Values] bool commit) + public void AWriteOverARecordAtItsOwnStoredAtIsStoredLaterWhateverTheClockSays([Values] bool directory, [Values] bool commit) { var path = PathOf("skew.cc"); - Seed(path, false, "k"); + Seed(path, directory, "k"); var k = new CultRecordKey("k"); var ahead = DateTimeOffset.UtcNow.AddHours(1).ToString("O", CultureInfo.InvariantCulture); var snapshot = Read(path); snapshot.Records.Single(record => record.Key == "k").StoredAt = ahead; Write(path, snapshot); - using var a = Open(path, false); - using var b = Open(path, false); + using var a = Open(path, directory); + using var b = Open(path, directory); if (commit) b.Commit(batch => batch.Upsert(typeof(WsItem), new WsItem { Name = "name-k", Note = "by-b" }, k)); else @@ -582,6 +595,11 @@ namespace GameCult.Caching.Tests batch.Expect(k, seenByB); batch.Upsert(typeof(WsItem), new WsItem { Name = "name-k", Note = "stale" }, k); }), Is.False, "B's copy of k is not what the store holds"); + Assert.That(a.Commit(batch => + { + batch.Expect(k, a.Get(k)!); + batch.Upsert(typeof(WsItem), new WsItem { Name = "name-k", Note = "again" }, k); + }), Is.True, "A's own condition holds without a pull: the cache took the storedAt the write gave it"); } // Another writer removes a base and its variant together; this cache, still holding both, adds a variant of that base. @@ -620,7 +638,27 @@ namespace GameCult.Caching.Tests Land(a, commit, typeof(WsItem), new WsItem { Name = "name-x", Note = "edited" }, X))!; Assert.That(refusal.RecordKey, Is.EqualTo("x")); - Assert.That(refusal.InnerException, Is.Not.Null, "the reload that failed is the cause"); + Assert.That(refusal.InnerException, Is.TypeOf().And.Message.Contains("variant v is based on it"), "the reload that failed is the cause"); + Assert.That(File.ReadAllBytes(path), Is.EqualTo(before)); + Assert.That(a.Get(X), Is.Not.Null, "the reload failed, so the cache is unchanged"); + } + + // A write that finds a record it holds gone foreign is refused as a conflict, and when the reload that refusal owes fails + // (x cannot go while variant v is based on it) the conflict carries that failure, and nothing changes. + [Test] + public void AWriteConflictWhoseReloadFailsCarriesThatCause([Values] bool commit) + { + var path = PathOf("conflict-reload.cc"); + Seed(path, false, "x"); + var a = Open(path, false); + AddVariant(a, "v", X); + MakeForeign(path, "x"); + var before = File.ReadAllBytes(path); + + var conflict = Assert.Throws(() => + Land(a, commit, typeof(WsItem), new WsItem { Name = "name-y" }, new CultRecordKey("y")))!; + + Assert.That(conflict.InnerException, Is.TypeOf().And.Message.Contains("variant v is based on it"), "the reload that failed is the cause"); Assert.That(File.ReadAllBytes(path), Is.EqualTo(before)); Assert.That(a.Get(X), Is.Not.Null, "the reload failed, so the cache is unchanged"); }