diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b712765..60b59df 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -508,6 +508,12 @@ jobs: grep -q '^test result: ok. 1 passed' /tmp/guard.log \ || { echo "::error::a_hold_reaches_the_digest_and_an_object_under_none_spells_zeros did not run"; exit 1; } + - name: Filing a record over an existing key replaces it + run: | + set -o pipefail + cargo test --locked --lib \n tests::filing_a_record_over_an_existing_key_replaces_it -- --exact \n | tee /tmp/guard.log + grep -q '^test result: ok. 1 passed' /tmp/guard.log \n || { echo "::error::filing_a_record_over_an_existing_key_replaces_it did not run"; exit 1; } + - name: A digest equals the text it is a digest of and refuses a corrupt tag run: | set -o pipefail diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3d45a35..52d56bd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -55,9 +55,9 @@ Two jobs, both on a pinned toolchain: | job | what it does | |---|---| | **build & docs** | `cargo build --all-targets --locked` and `--all-features`, then `cargo doc --no-deps --locked` and `--all-features`, all with warnings denied | -| **format and rollback guards** | 51 individual tests, each run by name with `--exact` | +| **format and rollback guards** | 52 individual tests, each run by name with `--exact` | -**CI does not run the suite.** It compiles everything and runs those 51 guards. A green +**CI does not run the suite.** It compiles everything and runs those 52 guards. A green tick means the tree builds and those guards hold; it does not mean the suite passed. Run `tools/suite_gate.sh` yourself. diff --git a/README.md b/README.md index 4ac44d0..3e8bbc1 100644 --- a/README.md +++ b/README.md @@ -238,20 +238,21 @@ At three replicas with the metadata counted, measured at 26-character keys: |---:|---:|---:|---:| | 64 KiB | 1,000 | **3.01x** | 3.00x | | 4 KiB | 4,000 | **3.09x** | 3.00x | -| 1 KiB | 8,000 | **3.38x** | 3.02x | +| 1 KiB | 8,000 | **3.36x** | 3.02x | The data term is exactly **3.00x** -- the replication floor for a store that copies every page to every replica -- so everything above it is metadata, which -is **346 bytes an object plus 164,256 fixed** whatever the object size. +is **330 bytes an object plus 164,240 fixed** whatever the object size. Those bytes are not all in one place. **218 of them are a storage node's own** -- the -object's entry in its map, and its key in the payload index -- and **128 are the metadata +object's entry in its map, and its key in the payload index -- and **112 are the metadata service's**, which keeps its own record of every object with its own versioning, deletion markers and change log. This crate models both services in one process, so the audit counts both; a deployment runs them on different machines, where a storage node carries 218 bytes -an object rather than 346. +an object rather than 330. -The metadata service's record is **a name, two stamps and a digest**. It does not restate the +The metadata service's record is **a name, two stamps and a digest**, and the collection +holding it is ordered by that name rather than keeping a second copy of it. It does not restate the object: it holds an eight-byte digest of the object's rendered metadata, so it can prove a record matches the object without carrying a second copy of it. What that costs is a restore that is no longer self-sufficient -- a snapshot carries the digest, not the characters, so @@ -275,9 +276,9 @@ which is more hardware rather than less space. replicas to eight and fails if any of them undercuts that. Counted with the metadata, a thousand objects of 1 KiB measure -**2.03x striped against 3.51x replicated**, both read out of `footprint_audit()` on a +**2.01x striped against 3.50x replicated**, both read out of `footprint_audit()` on a store nothing has read back yet. What separates the total from the 1.5x data term is the -metadata line above, which striping does not change: the same 346 bytes an object either +metadata line above, which striping does not change: the same 330 bytes an object either way. Striping moves the payload and nothing else. Both numbers are for the same object count, so the comparison is between two @@ -352,12 +353,12 @@ cargo test --release --lib measure_what_a_mutation_batch_snapshot_costs -- --ign #### A delete does not reclaim, it trades Writing 500 objects of 1 KiB and then deleting every one leaves the store -**26.0% larger** than when it held them: +**26.1% larger** than when it held them: | | bytes | |---|---:| -| holding 500 objects | 1,799,649 | -| after deleting all 500 | 2,268,662 | +| holding 500 objects | 1,791,633 | +| after deleting all 500 | 2,260,646 | The payload index is given back in full. What replaces it is **822 bytes of idempotency receipt per delete** across three replicas, bounded only by diff --git a/docs/footprint.md b/docs/footprint.md index de7888a..f545131 100644 --- a/docs/footprint.md +++ b/docs/footprint.md @@ -22,7 +22,7 @@ Measured at three replicas, 26-character keys, with the metadata counted: |---:|---:|---:|---:| | 64 KiB | 1,000 | **3.01x** | 3.00x | | 4 KiB | 4,000 | **3.09x** | 3.00x | -| 1 KiB | 8,000 | **3.38x** | 3.02x | +| 1 KiB | 8,000 | **3.36x** | 3.02x | Reproduce any row with the measurement harness: @@ -67,7 +67,7 @@ are bounded rather than per object: | objects | 100 | 1,000 | 4,000 | 8,000 | |---:|---:|---:|---:|---:| -| metadata per object | 554 B | 510 B | 387 B | **366 B** | +| metadata per object | 538 B | 494 B | 371 B | **350 B** | Both change feeds cap at 1,024 records, so together they are **163,866 bytes** once full -- the two intercepts in the table below, which is where that figure is @@ -75,7 +75,7 @@ checked rather than derived from a percentage of a moving total. Everything else is exactly linear, which makes the total a line once the feeds are full: ``` -metadata = 346 bytes an object + 164,256 bytes fixed (remainder: zero) +metadata = 330 bytes an object + 164,240 bytes fixed (remainder: zero) ``` reproducing every total above exactly, at every count where the feeds have @@ -92,24 +92,24 @@ feeds at all: | object change feed | 106,522 | 1,024 records at its bound | | metadata change log | 57,344 | 1,024 records at its bound | | inside `objects` | 62 | the content-type and storage-class text, shared by every object | -| inside `meta.records` | 108 | one record more than there are objects — the bucket's | +| inside `meta.records` | 92 | one record more than there are objects — the bucket's | | `meta.entities` | 84 | | | `chunks` | 104 | | | payload index | 32 | the chunk identity, paid once per chunk rather than once per entry | -| **fixed total** | **164,256** | | +| **fixed total** | **164,240** | | `every_metadata_leg_is_a_line_and_the_intercepts_are_named` fits each leg over three store sizes and fails unless every one is a line and the fixed terms sum to the total's — so a leg that starts depending on the object count, or a fixed byte that stops being named, is caught rather than absorbed. -That 346 is two items and a rounding error: +That 330 is two items and a rounding error: | | per object | share | |---|---:|---:| -| `objects` — the structured metadata | 210 B | 60.7% | -| `meta.records` — the metadata digest record | 128 B | 37.0% | -| payload index keys | 8 B | 2.3% | +| `objects` — the structured metadata | 210 B | 63.6% | +| `meta.records` — the metadata digest record | 112 B | 33.9% | +| payload index keys | 8 B | 2.4% | Those two halves no longer hold the same facts twice. `objects` is a binary struct — 136 bytes inline, with the object id, length, etag, content type, storage class @@ -164,7 +164,7 @@ shared between holders is charged once per allocation — the characters and the two counts in front of them — so `mapkey_capacity` against `mapkey_len` is that header rather than slack. -### What the 346 bytes are, and what each remaining part would cost to remove +### What the 330 bytes are, and what each remaining part would cost to remove The slope is not one thing. Every leg below is fitted from the same two object counts the published line is fitted from, and they sum to it exactly: @@ -175,23 +175,27 @@ objects.mapkey_inline 32 two shared-text holders, which is what keys t objects.mapkey_heap 42 the key's characters: ONE allocation, shared with the value objects.metadata_inline 136 ObjectMetadata itself, one extent held inline objects.extents_bytes 0 the one extent is INLINE above, so it owns no heap -meta.records.struct_inline 72 the map key's holder, plus MetadataRecord -meta.records.mapkey_capacity 56 the record key's characters: ONE allocation, shared +meta.records.struct_inline 56 MetadataRecord, and no map key beside it +meta.records.mapkey_capacity 56 the record key's characters: ONE allocation, ONE holder meta.records.value_capacity 0 the value is an eight-byte digest, held INLINE above payloads.key_bytes 8 the payload index's key -METADATA LINE TOTAL 346 +METADATA LINE TOTAL 330 ``` `the_published_metadata_decomposition_sums_to_the_line` measures each one and fails if this block disagrees with the tree or if the parts stop adding up. -**Two of these are already one allocation each, and that is load-bearing rather than -incidental.** Both maps name the identity twice -- once as the map key and once inside the -value -- and both times the two holders share. `objects.inner_id_heap` and -`meta.records.record_key_capacity` therefore read **zero**, and -`the_identity_each_map_names_twice_is_one_allocation` pins it: unsharing either would add -back the characters plus a 16-byte header for every object, silently, with no test failing -on the behaviour. +**The objects map names the identity twice and the two holders share**, so +`objects.inner_id_heap` reads **zero**, and +`the_identity_each_map_names_twice_is_one_allocation` pins it: unsharing it would add back the +characters plus a 16-byte header for every object, silently, with no test failing on the +behaviour. + +`meta.records` no longer names its key twice at all. The collection is a set ordered by each +record's own key, so `meta.records.record_key_capacity` reads zero **by construction** rather +than by a sharing discipline — a type is doing the work a tripwire used to, and the figure is +kept because it is what the next reader will look for and because reintroducing a second +holder would move it. The difference between `mapkey_capacity` and `mapkey_len` is not slack to reclaim. Shared text has no spare capacity; the difference is the strong and weak counts in front of the @@ -199,7 +203,7 @@ characters, sixteen bytes once per allocation. #### The reductions that are left, and what each one costs -Each is priced against the 346. Two rows have come **off** this list, and both came off the +Each is priced against the 330. Two rows have come **off** this list, and both came off the same way -- by being measured instead of argued. Narrowing an extent's offset and length to 32 bits was priced at 8 bytes an object and a migration, until the migration turned out not to exist: the snapshot still writes two `u64` and only the in-memory form is narrow. Interning @@ -209,8 +213,8 @@ decision and one representation. | | saves | what it costs | |---|---:|---| -| The metadata records are a second store of the same objects | **128** | It is the whole of `meta.records`. They no longer RESTATE the objects map — a record holds a name, two stamps and a digest, and the digest is what lets it prove agreement without a second copy. What is left is a separate metadata service's own store, with its own versioning, deletion markers and change log, modelled in this process; removing it means collapsing two services into one, which is a design decision and not a footprint one. | -| A one-word text handle | **40**, or **24** without unsafe code | `SharedText` is `Arc`, a fat pointer, so every holder spends two words to name one allocation. Seven holders an object carry it -- two keying `objects`, three inside `ObjectMetadata` and two around a metadata record -- so one word each is **56 bytes of inline saving**, measured by construction rather than multiplied, because a narrowed field can leave its padding behind instead of closing up. The allocation pays part of it back by carrying the length the handle stops carrying: one word per allocation, two allocations an object, **net 40**. The shape that needs no unsafe code is `Arc>`, which puts the length in a second allocation instead and nets **24**. Both add a dependent load to a string read, and THAT LOAD IS NOW PRICED. Measured in release over 200,000 names visited in a shuffled order -- shuffled because a constant stride is the pattern a prefetcher exists to hide -- the thin handle costs **2.6x to 3.1x on a length read** and **1.1x to 1.3x on a character read**, four to nine nanoseconds either way. The length case is the whole story: `Arc` carries the length IN THE HANDLE and touches no heap at all, where `Arc>` must reach the box first. The comparison favours the thin handle, whose index is half the size and so has the better locality, and it still loses. `measure_what_a_one_word_text_handle_costs_to_read` prints it. So the trade is 24 bytes an object -- 6.9% of the metadata line -- against roughly tripling the cost of asking a name its length, and the saving is largest for the small objects that do the most string reads per stored byte. | +| The metadata records are a second store of the same objects | **112** | It is the whole of `meta.records`. They no longer RESTATE the objects map — a record holds a name, two stamps and a digest, and the digest is what lets it prove agreement without a second copy. What is left is a separate metadata service's own store, with its own versioning, deletion markers and change log, modelled in this process; removing it means collapsing two services into one, which is a design decision and not a footprint one. | +| A one-word text handle | **32**, or **16** without unsafe code | `SharedText` is `Arc`, a fat pointer, so every holder spends two words to name one allocation. Six holders an object carry it -- two keying `objects`, three inside `ObjectMetadata` and one in a metadata record -- so one word each is **48 bytes of inline saving**, measured by construction rather than multiplied, because a narrowed field can leave its padding behind instead of closing up. The allocation pays part of it back by carrying the length the handle stops carrying: one word per allocation, two allocations an object, **net 32**. The shape that needs no unsafe code is `Arc>`, which puts the length in a second allocation instead and nets **16**. Both add a dependent load to a string read, and THAT LOAD IS NOW PRICED. Measured in release over 200,000 names visited in a shuffled order -- shuffled because a constant stride is the pattern a prefetcher exists to hide -- the thin handle costs **2.6x to 3.1x on a length read** and **1.1x to 1.3x on a character read**, four to nine nanoseconds either way. The length case is the whole story: `Arc` carries the length IN THE HANDLE and touches no heap at all, where `Arc>` must reach the box first. The comparison favours the thin handle, whose index is half the size and so has the better locality, and it still loses. `measure_what_a_one_word_text_handle_costs_to_read` prints it. So the trade is 16 bytes an object -- 4.8% of the metadata line -- against roughly tripling the cost of asking a name its length, and the saving is largest for the small objects that do the most string reads per stored byte. | **Both figures above are measurements now, and the difference has mattered three times.** The extent row was priced at 8 bytes and a migration, and the migration turned out not to diff --git a/src/protocol/metadata/leases_maintenance/lease_records.rs b/src/protocol/metadata/leases_maintenance/lease_records.rs index 4501f66..00d4470 100644 --- a/src/protocol/metadata/leases_maintenance/lease_records.rs +++ b/src/protocol/metadata/leases_maintenance/lease_records.rs @@ -13,6 +13,90 @@ pub struct MetadataRecord { pub deleted: bool, } +/// A metadata record ordered and identified BY ITS OWN KEY, so the collection holding it +/// does not keep a second copy of that key. +/// +/// # EQUALITY IS THE KEY ALONE, AND THAT IS NOT A PREFERENCE +/// +/// `BTreeSet` requires `Ord` and `Eq` to agree: if `a == b` then `a.cmp(&b)` must be `Equal`, +/// or the set's ordering invariant is unspecified behaviour rather than a bug you can see. The +/// ordering has to be the key, because **the key order is the listing contract** -- it is what +/// `list_objects` serves a prefix and a marker from. So equality is the key too. +/// +/// This type therefore says NOTHING about whether two records hold the same value. Ask the +/// record, through [`KeyedRecord::record`]. Nothing outside the collection should compare these +/// to each other, which is why the record is private rather than a public field. +/// +/// # AND FILING USES `replace`, NOT `insert` +/// +/// `BTreeSet::insert` KEEPS the element already there and returns `false`; it does not +/// overwrite. Filing a new version of a record with `insert` would therefore leave the old +/// version in place and report success. +/// `filing_a_record_over_an_existing_key_replaces_it` is the guard for that. +#[derive(Clone, Debug)] +pub struct KeyedRecord(MetadataRecord); + +impl KeyedRecord { + pub fn new(record: MetadataRecord) -> Self { + Self(record) + } + + pub fn record(&self) -> &MetadataRecord { + &self.0 + } + + pub fn into_record(self) -> MetadataRecord { + self.0 + } + + pub fn key(&self) -> &SharedText { + &self.0.key + } +} + +impl Ord for KeyedRecord { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.0.key.cmp(&other.0.key) + } +} + +impl PartialOrd for KeyedRecord { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Eq for KeyedRecord {} + +impl PartialEq for KeyedRecord { + fn eq(&self, other: &Self) -> bool { + self.0.key == other.0.key + } +} + +/// So the collection can be searched by `&str` without building a record. +/// +/// `Borrow` requires this type's `Ord` to agree with `str`'s, which it does: `SharedText` +/// orders by its characters. +impl std::borrow::Borrow for KeyedRecord { + fn borrow(&self) -> &str { + self.0.key.as_str() + } +} + +/// The same, for callers holding the key as a shared handle rather than as characters. +impl std::borrow::Borrow for KeyedRecord { + fn borrow(&self) -> &SharedText { + &self.0.key + } +} + +impl From for KeyedRecord { + fn from(record: MetadataRecord) -> Self { + Self(record) + } +} + impl MetadataRecord { pub fn to_control_fields(&self, field_prefix: &str) -> BTreeMap { let mut fields = BTreeMap::new(); diff --git a/src/services/meta/footprint.rs b/src/services/meta/footprint.rs index 97c5ac2..17245f5 100644 --- a/src/services/meta/footprint.rs +++ b/src/services/meta/footprint.rs @@ -143,10 +143,11 @@ fn meta_transaction_heap(ledger: &mut SharedTextLedger, transaction: &MetadataTr impl MatrixMetaService { pub(crate) fn footprint_bytes(&self, ledger: &mut SharedTextLedger) -> MetaServiceFootprint { - let mut records = (self.records.len() as u64).saturating_mul( - (std::mem::size_of::() + std::mem::size_of::()) as u64, - ); - for (key, record) in self.records.iter() { + // ONE ENTRY IS ONE RECORD. The collection is ordered by each record's own key, so + // there is no map-key handle beside it any more. + let mut records = (self.records.len() as u64) + .saturating_mul(std::mem::size_of::() as u64); + for (key, record) in self.records_iter() { records = records .saturating_add(ledger.charge(key)) .saturating_add(meta_record_heap(ledger, record)); diff --git a/src/services/meta/maintenance_persistence/maintenance_transactions.rs b/src/services/meta/maintenance_persistence/maintenance_transactions.rs index 2e014fb..ac81445 100644 --- a/src/services/meta/maintenance_persistence/maintenance_transactions.rs +++ b/src/services/meta/maintenance_persistence/maintenance_transactions.rs @@ -122,7 +122,7 @@ impl MatrixMetaService { } } for record in &prepared.compacted_tombstones { - match self.records.get(&record.key) { + match self.record(record.key.as_str()) { Some(current) if current == record => {} _ => { return Err(ObjectError::PreconditionFailed( @@ -144,7 +144,7 @@ impl MatrixMetaService { } } for record in &prepared.compacted_tombstones { - self.records.remove(&record.key); + self.records.remove(record.key.as_str()); } if prepared.tombstone_report.compacted_records > 0 { self.compacted_tombstone_count = prepared.tombstone_report.compacted_tombstone_count; diff --git a/src/services/meta/maintenance_persistence/mutation_preflight.rs b/src/services/meta/maintenance_persistence/mutation_preflight.rs index 23363ec..44ea6d5 100644 --- a/src/services/meta/maintenance_persistence/mutation_preflight.rs +++ b/src/services/meta/maintenance_persistence/mutation_preflight.rs @@ -40,8 +40,8 @@ impl MatrixMetaService { /// mutation's own copy would otherwise become a second one. pub(crate) fn filed_record_key(&self, key: SharedText) -> SharedText { self.records - .get_key_value(key.as_str()) - .map(|(existing, _)| existing.clone()) + .get(key.as_str()) + .map(|held| held.key().clone()) .unwrap_or(key) } @@ -49,11 +49,51 @@ impl MatrixMetaService { /// keeps. `BTreeMap::insert` keeps the key it already holds, so without /// this an update leaves the map key and the record's own naming of it in /// two allocations. + /// The record at a key, without the collection's wrapper. + pub(crate) fn record(&self, key: &str) -> Option<&MetadataRecord> { + self.records.get(key).map(KeyedRecord::record) + } + + /// Every record with its key, in key order -- the shape the map this replaced had. + pub(crate) fn records_iter(&self) -> impl Iterator { + self.records + .iter() + .map(|held| (held.key(), held.record())) + } + + /// Every record, in key order. + pub(crate) fn records_values(&self) -> impl Iterator { + self.records.iter().map(KeyedRecord::record) + } + + /// The records in a span of keys, with their keys. + pub(crate) fn records_range( + &self, + range: R, + ) -> impl Iterator + where + R: std::ops::RangeBounds, + { + self.records + .range::(range) + .map(|held| (held.key(), held.record())) + } + + /// File a record under a key. + /// + /// `replace`, NOT `insert`: `BTreeSet::insert` keeps the element already there, so filing a + /// new version with it would leave the old one in place and report success. + /// `filing_a_record_over_an_existing_key_replaces_it` is the guard. pub(crate) fn file_record(&mut self, key: SharedText, mut record: MetadataRecord) { - if let Some((existing, _)) = self.records.get_key_value(key.as_str()) { - record.key = existing.clone(); + // The record names the key it is filed under, through the allocation the collection + // already holds -- so a record filed over an existing one keeps that allocation + // rather than adding a second copy of the same characters. + if let Some(existing) = self.records.get(key.as_str()) { + record.key = existing.key().clone(); + } else { + record.key = key; } - self.records.insert(key, record); + self.records.replace(KeyedRecord::new(record)); } fn preflight_mutations( @@ -76,8 +116,7 @@ impl MatrixMetaService { self.check_shard_fence(&mutation.key)?; self.check_lease_fence(&mutation.key, lease_owner, lease_epoch, now_ms)?; let base_version = self - .records - .get(&mutation.key) + .record(mutation.key.as_str()) .map_or(0, |record| record.version); read_versions .entry(mutation.key.to_string()) @@ -191,8 +230,7 @@ impl MatrixMetaService { self.check_shard_owner_serving(&mutation.key, expected_owner, expected_epoch, now_ms)?; self.check_lease_fence(&mutation.key, lease_owner, lease_epoch, now_ms)?; let base_version = self - .records - .get(&mutation.key) + .record(mutation.key.as_str()) .map_or(0, |record| record.version); read_versions .entry(mutation.key.to_string()) @@ -226,8 +264,7 @@ impl MatrixMetaService { validate_meta_key(key)?; self.check_shard_fence(key)?; let current_version = self - .records - .get(key.as_str()) + .record(key.as_str()) .map_or(0, |record| record.version); if current_version != *expected_version { return Err(ObjectError::PreconditionFailed( @@ -242,7 +279,7 @@ impl MatrixMetaService { /// without copying the service. pub(crate) fn capture_record_undo(&self, key: &str) -> MetaRecordUndo { MetaRecordUndo { - previous_record: self.records.get(key).cloned(), + previous_record: self.record(key).cloned(), key: key.to_string(), next_commit_index: self.next_commit_index, next_change_sequence: self.next_change_sequence, @@ -303,8 +340,7 @@ impl MatrixMetaService { now_ms: u64, ) -> Result { let current_version = self - .records - .get(&mutation.key) + .record(mutation.key.as_str()) .map_or(0, |record| record.version); if current_version != mutation.expected_version { return Err(ObjectError::PreconditionFailed( @@ -324,7 +360,7 @@ impl MatrixMetaService { deleted: mutation.delete, }; self.next_commit_index = self.next_commit_index.saturating_add(1); - self.records.insert(key.clone(), record.clone()); + self.file_record(key.clone(), record.clone()); self.append_metadata_change(MetadataChange { sequence: self.next_change_sequence, commit_index: self.next_commit_index.saturating_sub(1), diff --git a/src/services/meta/maintenance_persistence/prefix_checkpoint/prefix_plan.rs b/src/services/meta/maintenance_persistence/prefix_checkpoint/prefix_plan.rs index cee0d3e..c4dba62 100644 --- a/src/services/meta/maintenance_persistence/prefix_checkpoint/prefix_plan.rs +++ b/src/services/meta/maintenance_persistence/prefix_checkpoint/prefix_plan.rs @@ -18,8 +18,7 @@ impl MatrixMetaService { self.check_shard_fence(prefix)?; self.check_lease_fence(prefix, lease_owner, lease_epoch, now_ms)?; let keys: Vec = self - .records - .iter() + .records_iter() .filter(|(key, record)| key.starts_with(prefix) && !record.deleted) .map(|(key, _)| key.to_string()) .take(max_records) @@ -33,8 +32,7 @@ impl MatrixMetaService { key: key.clone().into(), record_value: RecordValue::empty(), expected_version: self - .records - .get(key.as_str()) + .record(key.as_str()) .map_or(0, |record| record.version), delete: true, }) @@ -67,8 +65,7 @@ impl MatrixMetaService { self.check_shard_fence(prefix)?; self.check_lease_fence(prefix, lease_owner, lease_epoch, now_ms)?; let keys: Vec = self - .records - .iter() + .records_iter() .filter(|(key, record)| key.starts_with(prefix) && !record.deleted) .map(|(key, _)| key.to_string()) .take(max_records) @@ -79,8 +76,7 @@ impl MatrixMetaService { key: key.clone().into(), record_value: RecordValue::empty(), expected_version: self - .records - .get(key.as_str()) + .record(key.as_str()) .map_or(0, |record| record.version), delete: true, }) @@ -171,8 +167,7 @@ impl MatrixMetaService { let mut payload_bytes = 0_u64; let mut mutations = Vec::new(); for (key, record) in self - .records - .iter() + .records_iter() .filter(|(key, record)| key.starts_with(prefix) && !record.deleted) { if mutations.len() >= max_records { diff --git a/src/services/meta/maintenance_persistence/snapshot_lifecycle/export_fingerprint.rs b/src/services/meta/maintenance_persistence/snapshot_lifecycle/export_fingerprint.rs index de87142..4afaa25 100644 --- a/src/services/meta/maintenance_persistence/snapshot_lifecycle/export_fingerprint.rs +++ b/src/services/meta/maintenance_persistence/snapshot_lifecycle/export_fingerprint.rs @@ -21,7 +21,12 @@ fn metadata_fingerprint_reserve_hint(service: &MatrixMetaService) -> usize { impl MatrixMetaService { pub fn export_snapshot(&self) -> MetaSnapshot { let mut snapshot = MetaSnapshot { - records: self.records.clone(), + // The collection is ordered by each record's own key; the snapshot states the + // key once in front of each record, so it rebuilds the pairs. + records: self + .records_iter() + .map(|(key, record)| (key.clone(), record.clone())) + .collect(), leases: self.leases.clone(), lease_receipts: self.lease_receipts.clone(), coordinator_config: self.coordinator_config.clone(), @@ -80,7 +85,7 @@ impl MatrixMetaService { } append_u64(&mut material, self.records.len() as u64); - for (key, record) in &self.records { + for (key, record) in self.records_iter() { append_string(&mut material, key); append_string(&mut material, &etag(record.value.as_text().as_bytes())); append_u64(&mut material, record.version); diff --git a/src/services/meta/maintenance_persistence/snapshot_lifecycle/install.rs b/src/services/meta/maintenance_persistence/snapshot_lifecycle/install.rs index 511afa5..a9c89c0 100644 --- a/src/services/meta/maintenance_persistence/snapshot_lifecycle/install.rs +++ b/src/services/meta/maintenance_persistence/snapshot_lifecycle/install.rs @@ -5,7 +5,13 @@ impl MatrixMetaService { snapshot.validate()?; self.metadata_snapshot_assembler.reset(); *self.snapshot_export_cache.borrow_mut() = None; - self.records = snapshot.records; + // The snapshot's pairs become records ordered by their own key. A record already + // names the key it was filed under, so the pair's key is dropped rather than kept. + self.records = snapshot + .records + .into_values() + .map(KeyedRecord::new) + .collect(); self.leases = snapshot.leases; self.lease_receipts = snapshot.lease_receipts; self.coordinator_config = snapshot.coordinator_config; diff --git a/src/services/meta/maintenance_persistence/snapshot_tombstone_retention/tombstone_collection.rs b/src/services/meta/maintenance_persistence/snapshot_tombstone_retention/tombstone_collection.rs index 0c11710..83624b7 100644 --- a/src/services/meta/maintenance_persistence/snapshot_tombstone_retention/tombstone_collection.rs +++ b/src/services/meta/maintenance_persistence/snapshot_tombstone_retention/tombstone_collection.rs @@ -39,7 +39,7 @@ impl MatrixMetaService { let mut remove_keys = Vec::new(); let start = after_key.unwrap_or(prefix).to_string(); let mut last_selected_key: Option = None; - for (key, record) in self.records.range::(( + for (key, record) in self.records_range(( std::ops::Bound::Included(start.as_str()), std::ops::Bound::Unbounded, )) { diff --git a/src/services/meta/maintenance_persistence/snapshot_tombstone_retention/tombstone_compaction.rs b/src/services/meta/maintenance_persistence/snapshot_tombstone_retention/tombstone_compaction.rs index 022aa51..66b68a8 100644 --- a/src/services/meta/maintenance_persistence/snapshot_tombstone_retention/tombstone_compaction.rs +++ b/src/services/meta/maintenance_persistence/snapshot_tombstone_retention/tombstone_compaction.rs @@ -66,7 +66,7 @@ impl MatrixMetaService { )?; let compacted: Vec = remove_keys .iter() - .filter_map(|key| self.records.get(key.as_str()).cloned()) + .filter_map(|key| self.record(key.as_str()).cloned()) .collect(); if compacted.len() != remove_keys.len() { return Err(ObjectError::Corruption( @@ -137,7 +137,7 @@ impl MatrixMetaService { && prepared .compacted .iter() - .all(|record| !self.records.contains_key(&record.key)) + .all(|record| !self.records.contains(record.key.as_str())) { return Ok(prepared.report); } @@ -175,7 +175,7 @@ impl MatrixMetaService { return Ok(prepared.report); } for record in &prepared.compacted { - match self.records.get(&record.key) { + match self.record(record.key.as_str()) { Some(current) if current == record => {} _ => { return Err(ObjectError::PreconditionFailed( @@ -185,7 +185,7 @@ impl MatrixMetaService { } } for record in &prepared.compacted { - self.records.remove(&record.key); + self.records.remove(record.key.as_str()); } self.next_commit_index = self.next_commit_index.saturating_add(1); self.compacted_tombstone_count = prepared.report.compacted_tombstone_count; diff --git a/src/services/meta/records_leases_shards/automatic_balancing.inc.rs b/src/services/meta/records_leases_shards/automatic_balancing.inc.rs index 4878ee8..143b415 100644 --- a/src/services/meta/records_leases_shards/automatic_balancing.inc.rs +++ b/src/services/meta/records_leases_shards/automatic_balancing.inc.rs @@ -70,7 +70,7 @@ impl MatrixMetaService { report.scanned_shards = report.scanned_shards.saturating_add(1); let mut keys = Vec::new(); let mut payload_bytes = 0u64; - for (key, record) in self.records.range::(( + for (key, record) in self.records_range(( std::ops::Bound::Included(shard.start_key.as_str()), std::ops::Bound::Unbounded, )) { diff --git a/src/services/meta/records_leases_shards/construction_receipts/construction.rs b/src/services/meta/records_leases_shards/construction_receipts/construction.rs index f686393..58f1855 100644 --- a/src/services/meta/records_leases_shards/construction_receipts/construction.rs +++ b/src/services/meta/records_leases_shards/construction_receipts/construction.rs @@ -14,7 +14,7 @@ impl MatrixMetaService { placement_commit_index: 1, placement_snapshot_export_cache: RefCell::new(None), root_state_fingerprint_cache: RefCell::new(None), - records: BTreeMap::new(), + records: BTreeSet::new(), leases: BTreeMap::new(), lease_receipts: BTreeMap::new(), coordinator_config: MetadataCoordinatorConfig::default(), diff --git a/src/services/meta/records_leases_shards/construction_receipts/handoff_apply.rs b/src/services/meta/records_leases_shards/construction_receipts/handoff_apply.rs index 6b1a4c8..3e445c2 100644 --- a/src/services/meta/records_leases_shards/construction_receipts/handoff_apply.rs +++ b/src/services/meta/records_leases_shards/construction_receipts/handoff_apply.rs @@ -114,7 +114,7 @@ impl MatrixMetaService { "metadata prepared shard handoff deleted record carries value".to_string(), )); } - if let Some(existing) = self.records.get(&record.key) { + if let Some(existing) = self.record(record.key.as_str()) { if existing.version > record.version { return Err(ObjectError::PreconditionFailed( "metadata prepared shard handoff would overwrite newer record".to_string(), @@ -136,7 +136,7 @@ impl MatrixMetaService { )); } for record in &installed { - let should_emit_change = self.records.get(&record.key) != Some(record); + let should_emit_change = self.record(record.key.as_str()) != Some(record); self.file_record(record.key.clone(), record.clone()); if should_emit_change { self.append_metadata_change(MetadataChange { diff --git a/src/services/meta/records_leases_shards/construction_receipts/handoff_prepare.rs b/src/services/meta/records_leases_shards/construction_receipts/handoff_prepare.rs index 826dd94..f82c649 100644 --- a/src/services/meta/records_leases_shards/construction_receipts/handoff_prepare.rs +++ b/src/services/meta/records_leases_shards/construction_receipts/handoff_prepare.rs @@ -54,7 +54,7 @@ impl MatrixMetaService { let mut payload_bytes = 0u64; let mut truncated = false; let mut next_marker = None; - for (key, record) in self.records.range::(( + for (key, record) in self.records_range(( std::ops::Bound::Included(shard.start_key.as_str()), std::ops::Bound::Unbounded, )) { diff --git a/src/services/meta/records_leases_shards/readiness.inc.rs b/src/services/meta/records_leases_shards/readiness.inc.rs index b93e655..80c3bfe 100644 --- a/src/services/meta/records_leases_shards/readiness.inc.rs +++ b/src/services/meta/records_leases_shards/readiness.inc.rs @@ -160,8 +160,7 @@ impl MatrixMetaService { .count() as u64, metadata_records: self.records.len() as u64, deleted_metadata_records: self - .records - .values() + .records_values() .filter(|record| record.deleted) .count() as u64, retained_change_records: self.changes.len() as u64, diff --git a/src/services/meta/retention_transactions_routes/namespace_leases/applied_detection.rs b/src/services/meta/retention_transactions_routes/namespace_leases/applied_detection.rs index 5e5a7f0..251f997 100644 --- a/src/services/meta/retention_transactions_routes/namespace_leases/applied_detection.rs +++ b/src/services/meta/retention_transactions_routes/namespace_leases/applied_detection.rs @@ -58,7 +58,7 @@ impl MatrixMetaService { && prepared .compacted_tombstones .iter() - .all(|record| !self.records.contains_key(&record.key)) + .all(|record| !self.records.contains(record.key.as_str())) && (prepared.tombstone_report.compacted_records == 0 || (self.compacted_tombstone_count >= prepared.tombstone_report.compacted_tombstone_count diff --git a/src/services/meta/retention_transactions_routes/shard_routes.inc.rs b/src/services/meta/retention_transactions_routes/shard_routes.inc.rs index 6925fc1..2fa7b9a 100644 --- a/src/services/meta/retention_transactions_routes/shard_routes.inc.rs +++ b/src/services/meta/retention_transactions_routes/shard_routes.inc.rs @@ -71,7 +71,7 @@ impl MatrixMetaService { fn metadata_shard_record_stats(&self, shard: &MetadataShard) -> MetadataShardRecordStats { let mut stats = MetadataShardRecordStats::default(); - for (key, record) in self.records.range::(( + for (key, record) in self.records_range(( std::ops::Bound::Included(shard.start_key.as_str()), std::ops::Bound::Unbounded, )) { diff --git a/src/services/meta/service_state.inc.rs b/src/services/meta/service_state.inc.rs index 5887939..264c03f 100644 --- a/src/services/meta/service_state.inc.rs +++ b/src/services/meta/service_state.inc.rs @@ -31,7 +31,9 @@ pub struct MatrixMetaService { pub(crate) placement_commit_index: u64, placement_snapshot_export_cache: RefCell>, root_state_fingerprint_cache: RefCell>, - pub(crate) records: BTreeMap, + /// Ordered by each record's own key, which the record already holds -- see + /// `KeyedRecord`. A map would keep that key twice. + pub(crate) records: BTreeSet, pub(crate) leases: BTreeMap, lease_receipts: BTreeMap, pub(crate) coordinator_config: MetadataCoordinatorConfig, diff --git a/src/services/meta/sessions_changefeed_snapshot/maintenance_planning.inc.rs b/src/services/meta/sessions_changefeed_snapshot/maintenance_planning.inc.rs index e701643..dfaf222 100644 --- a/src/services/meta/sessions_changefeed_snapshot/maintenance_planning.inc.rs +++ b/src/services/meta/sessions_changefeed_snapshot/maintenance_planning.inc.rs @@ -150,7 +150,7 @@ impl MatrixMetaService { }; let compacted_tombstones: Vec = tombstone_keys .iter() - .filter_map(|key| self.records.get(key.as_str()).cloned()) + .filter_map(|key| self.record(key.as_str()).cloned()) .collect(); if compacted_tombstones.len() != tombstone_keys.len() { return Err(ObjectError::Corruption( diff --git a/src/services/meta/sessions_changefeed_snapshot/record_batches/apply_batch.rs b/src/services/meta/sessions_changefeed_snapshot/record_batches/apply_batch.rs index f0ea6b1..156defb 100644 --- a/src/services/meta/sessions_changefeed_snapshot/record_batches/apply_batch.rs +++ b/src/services/meta/sessions_changefeed_snapshot/record_batches/apply_batch.rs @@ -86,8 +86,7 @@ impl MatrixMetaService { )); } let current_version = self - .records - .get(&mutation.key) + .record(mutation.key.as_str()) .map_or(0, |record| record.version); if image.key != mutation.key || image.deleted != mutation.delete diff --git a/src/services/meta/sessions_changefeed_snapshot/record_batches/prepare_batch.rs b/src/services/meta/sessions_changefeed_snapshot/record_batches/prepare_batch.rs index 0e70c15..ee9cdaf 100644 --- a/src/services/meta/sessions_changefeed_snapshot/record_batches/prepare_batch.rs +++ b/src/services/meta/sessions_changefeed_snapshot/record_batches/prepare_batch.rs @@ -66,8 +66,7 @@ impl MatrixMetaService { let mut images = Vec::with_capacity(mutations.len()); for mutation in &mutations { let base_version = self - .records - .get(&mutation.key) + .record(mutation.key.as_str()) .map_or(0, |record| record.version); let current_version = projected_versions .get(&mutation.key) diff --git a/src/services/meta/sessions_changefeed_snapshot/record_batches/records_read_set.rs b/src/services/meta/sessions_changefeed_snapshot/record_batches/records_read_set.rs index 347633e..b3f623a 100644 --- a/src/services/meta/sessions_changefeed_snapshot/record_batches/records_read_set.rs +++ b/src/services/meta/sessions_changefeed_snapshot/record_batches/records_read_set.rs @@ -47,8 +47,7 @@ impl MatrixMetaService { } pub fn get_record(&self, key: &str) -> Result { - self.records - .get(key) + self.record(key) .filter(|record| !record.deleted) .cloned() .ok_or_else(|| ObjectError::NotFound(format!("metadata record {}", key))) @@ -72,7 +71,7 @@ impl MatrixMetaService { } let mut page = MetadataRecordPage::default(); let mut next_key = None; - for (key, record) in self.records.range::(( + for (key, record) in self.records_range(( std::ops::Bound::Included(prefix), std::ops::Bound::Unbounded, )) { @@ -131,7 +130,7 @@ impl MatrixMetaService { } self.check_shard_fence(&condition.key)?; report.checked_records = report.checked_records.saturating_add(1); - let current = self.records.get(condition.key.as_str()); + let current = self.record(condition.key.as_str()); let current_version = current.map_or(0, |record| record.version); if current_version != condition.expected_version { return Err(ObjectError::PreconditionFailed( diff --git a/src/services/meta/sessions_changefeed_snapshot/transaction_raft/apply.rs b/src/services/meta/sessions_changefeed_snapshot/transaction_raft/apply.rs index 7eaf7e2..68bf420 100644 --- a/src/services/meta/sessions_changefeed_snapshot/transaction_raft/apply.rs +++ b/src/services/meta/sessions_changefeed_snapshot/transaction_raft/apply.rs @@ -137,8 +137,7 @@ impl MatrixMetaService { self.check_transaction_read_versions(current)?; for record in &prepared.receipt.records { let current_version = self - .records - .get(&record.key) + .record(record.key.as_str()) .map_or(0, |existing| existing.version); if record.version != current_version.saturating_add(1) || record.update_time_ms != prepared.receipt.update_time_ms diff --git a/src/store/core/construction_control/bucket_create.rs b/src/store/core/construction_control/bucket_create.rs index 0689cd6..3f01493 100644 --- a/src/store/core/construction_control/bucket_create.rs +++ b/src/store/core/construction_control/bucket_create.rs @@ -66,8 +66,7 @@ impl MatrixObjectStore { let key = meta_bucket_key(bucket); let expected_version = self .meta_service - .records - .get(key.as_str()) + .record(key.as_str()) .map_or(0, |record| record.version); self.meta_service.commit_record( &key, diff --git a/src/store/core/construction_control/bucket_delete.rs b/src/store/core/construction_control/bucket_delete.rs index 420926b..a57dedf 100644 --- a/src/store/core/construction_control/bucket_delete.rs +++ b/src/store/core/construction_control/bucket_delete.rs @@ -88,8 +88,7 @@ impl MatrixObjectStore { let key = meta_bucket_key(bucket); let expected_version = self .meta_service - .records - .get(key.as_str()) + .record(key.as_str()) .map_or(0, |record| record.version); match self .meta_service diff --git a/src/store/core/footprint_audit.rs b/src/store/core/footprint_audit.rs index b861398..4e8f6c5 100644 --- a/src/store/core/footprint_audit.rs +++ b/src/store/core/footprint_audit.rs @@ -497,14 +497,12 @@ impl MatrixObjectStore { // `the_metadata_breakdown_accounts_for_every_byte_of_its_total` pins both // readings, so neither can drift back into the other. { - let records = &self.meta_service.records; - let n = records.len() as u64; + let n = self.meta_service.records.len() as u64; + // NO MAP KEY HANDLE. The collection is ordered by each record's OWN key, so an + // entry is the record and nothing beside it. push( "meta.records.struct_inline", - n.saturating_mul( - (std::mem::size_of::() + std::mem::size_of::()) - as u64, - ), + n.saturating_mul(std::mem::size_of::() as u64), ); push("meta.records.record_size", std::mem::size_of::() as u64); let mut mapkey_len = 0u64; @@ -515,12 +513,15 @@ impl MatrixObjectStore { let mut key_text_duplicated = 0u64; let mut key_text_shared = 0u64; let mut detail_record_ledger = SharedTextLedger::default(); - for (key, record) in records.iter() { + for (key, record) in self.meta_service.records_iter() { mapkey_len = mapkey_len.saturating_add(key.len() as u64); - // Shared text has no spare capacity, and the record's own - // naming of the key is the map key's allocation, so it adds - // nothing -- charged in holder order by one ledger, as the - // objects breakdown above is. + // THE KEY THE COLLECTION ORDERS BY *IS* THE RECORD'S OWN -- there is no + // second handle to share with any more, so `record_key_capacity`, + // `key_text_duplicated` and `key_text_shared` below are now true BY + // CONSTRUCTION rather than by a sharing discipline. They are kept because + // they are what the document publishes and what the next reader will look + // for, and because a change that reintroduced a second holder would move + // them -- but a type is doing the work a tripwire used to. mapkey_capacity = mapkey_capacity.saturating_add(detail_record_ledger.charge(key)); record_key_capacity = record_key_capacity diff --git a/src/store/core/raft_apply_persistence/delete_multipart_projection/projection_changes.rs b/src/store/core/raft_apply_persistence/delete_multipart_projection/projection_changes.rs index b36d8cd..a02eeef 100644 --- a/src/store/core/raft_apply_persistence/delete_multipart_projection/projection_changes.rs +++ b/src/store/core/raft_apply_persistence/delete_multipart_projection/projection_changes.rs @@ -12,8 +12,7 @@ impl MatrixObjectStore { let key = meta_object_key(object_id); let expected_version = self .meta_service - .records - .get(key.as_str()) + .record(key.as_str()) .map_or(0, |record| record.version); self.meta_service.commit_record( &key, diff --git a/src/store/core/storage_service/listing_metadata_audit.inc.rs b/src/store/core/storage_service/listing_metadata_audit.inc.rs index 703a337..60756d2 100644 --- a/src/store/core/storage_service/listing_metadata_audit.inc.rs +++ b/src/store/core/storage_service/listing_metadata_audit.inc.rs @@ -111,8 +111,7 @@ impl MatrixObjectStore { for (record_key, projection) in self .meta_service - .records - .range::(( + .records_range(( std::ops::Bound::Included(projection_prefix.as_str()), std::ops::Bound::Unbounded, )) @@ -188,8 +187,7 @@ impl MatrixObjectStore { let projection_key = meta_object_key(object_id); let projection = self .meta_service - .records - .get(projection_key.as_str()) + .record(projection_key.as_str()) .cloned(); if projection.as_ref().map_or(true, |record| record.deleted) { audit.missing_projection_count = audit.missing_projection_count.saturating_add(1); diff --git a/src/tests.rs b/src/tests.rs index 8106fd6..9e79cfa 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -30511,12 +30511,15 @@ fn metaserver_projects_object_metadata_and_enforces_leases() { .unwrap(); assert_eq!(clean_audit.issue_count, 0); assert_eq!(clean_audit.audited_objects, 1); - store + // A collection ordered by the record's own key has no `get_mut`: changing a record in + // place could change where it belongs. Take it out, change it, put it back. + let mut stale = store .meta_service - .records - .get_mut(meta_object_key(&metadata.object_id).as_str()) - .unwrap() - .value = "stale-projection".to_string().into(); + .record(meta_object_key(&metadata.object_id).as_str()) + .expect("the record the put filed") + .clone(); + stale.value = "stale-projection".to_string().into(); + store.meta_service.file_record(stale.key.clone(), stale); let stale_audit = store .audit_object_metadata("bucket", "", None, 10, 1024, MetadataReadOptions::default()) .unwrap(); @@ -34382,7 +34385,7 @@ fn measure_what_a_metadata_record_value_actually_holds() { "application/octet-stream", ) .unwrap(); - for (key, record) in store.meta_service.records.iter() { + for (key, record) in store.meta_service.records_iter() { println!("RECORDVALUE key_len={} key={}", key.len(), key.as_str()); println!( "RECORDVALUE value_len={} value={}", @@ -34845,7 +34848,7 @@ fn measure_how_much_of_the_metadata_projection_is_recomputable() { let mut drifted = (0u64, 0u64); let mut no_object = (0u64, 0u64); let mut not_an_object_key = (0u64, 0u64); - for (key, record) in store.meta_service.records.iter() { + for (key, record) in store.meta_service.records_iter() { let text: &str = key.as_str(); let bytes = record.value.len() as u64; let Some(rest) = text.strip_prefix("object/") else { @@ -35078,7 +35081,7 @@ fn measure_how_many_places_hold_one_object_key() { } let mut record_map_key: std::collections::BTreeSet = Default::default(); let mut record_own_key: std::collections::BTreeSet = Default::default(); - for (key, record) in store.meta_service.records.iter() { + for (key, record) in store.meta_service.records_iter() { record_map_key.insert(key.allocation_id()); record_own_key.insert(record.key.allocation_id()); } @@ -35566,6 +35569,88 @@ fn the_snapshot_decoder_accepts_the_version_before_this_one_and_refuses_the_one_ } } +#[test] +fn filing_a_record_over_an_existing_key_replaces_it() { + // The collection holding metadata records is a SET ordered by each record's own key, which + // is how it stops holding a second copy of that key. That buys sixteen bytes an object and + // introduces one hazard, and the hazard is not in the ordering: + // + // `BTreeSet::insert` KEEPS the element already there and returns `false`. + // + // A map's `insert` overwrites. So filing a new version of a record with `insert` would + // leave the OLD version in the collection, return a value that reads like success, and + // leave every byte figure, every count and every ordering assertion correct. A write would + // simply not take effect. `file_record` uses `replace`, and this is what says so. + let mut store = MatrixObjectStore::new(test_options()).unwrap(); + store.create_bucket("bucket").unwrap(); + store + .put_object("bucket", "object", b"first".to_vec(), "text/plain") + .unwrap(); + + let key = meta_object_key(&ObjectId::new("bucket", "object")); + let before = store + .meta_service + .record(key.as_str()) + .expect("the put files a record") + .clone(); + let count_before = store.meta_service.records.len(); + + // A second write at the same key. + store + .put_object("bucket", "object", b"second payload".to_vec(), "text/plain") + .unwrap(); + let after = store + .meta_service + .record(key.as_str()) + .expect("the record is still there") + .clone(); + + assert_eq!( + store.meta_service.records.len(), + count_before, + "a second write at one key must not add a second record" + ); + assert_ne!( + before.version, after.version, + "the filed record must be the NEW one -- `insert` on a set keeps the old element and \ + reports success, so a write would silently not take effect" + ); + assert_ne!( + before.value, after.value, + "and its value must be the new object's, not the first object's" + ); + assert_eq!( + after.value, + RecordValue::of_object(&store.head_object("bucket", "object").unwrap()), + "the filed record must agree with the object as it now stands" + ); + + // A RECORD IS ALWAYS FILED UNDER ITS OWN KEY, which is the invariant the set makes + // impossible to break: there is no second key to disagree with. Asserted rather than + // assumed, because it is what every caller that reads a key off the iteration relies on. + for (key, record) in store.meta_service.records_iter() { + assert_eq!( + key.as_str(), + record.key.as_str(), + "the key a record is found under is the key it holds" + ); + assert!( + key.shares_with(&record.key), + "and it is the same allocation, not a second copy of the characters" + ); + } + + // AND THE KEY'S CHARACTERS ARE NOT COPIED ON AN UPDATE. `file_record` makes the incoming + // record name the key through the allocation the collection already holds; without that, + // every rewrite of an object would leave the record naming its key through a fresh + // allocation and the audit's key figure would climb with the write count rather than with + // the object count. + assert!( + before.key.shares_with(&after.key), + "a record filed over an existing one keeps the allocation already there" + ); +} + #[test] fn a_digest_equals_the_text_it_is_a_digest_of_and_refuses_a_corrupt_tag() { // A record at an object key holds a DIGEST; a record anywhere else holds TEXT. Two things @@ -36148,8 +36233,7 @@ fn the_metadata_record_key_is_one_allocation_across_its_holders() { let store = store_with_keys_of_length(64); let (map_key, record) = store .meta_service - .records - .iter() + .records_iter() .find(|(key, _)| key.starts_with("object/")) .expect("a put writes a projection record"); assert_eq!(map_key.as_str(), record.key.as_str()); @@ -37516,9 +37600,13 @@ fn measure_what_a_one_word_text_handle_would_save() { std::mem::size_of::(), ), ( + // ONE HANDLE, NOT TWO. The collection is ordered by the record's own key, so the + // leg is the record alone -- it was `SharedText + RecordControl` while a map key + // sat beside each record, and leaving that in would have published a saving of + // sixteen where eight is available. "meta.records.struct_inline", - std::mem::size_of::() + std::mem::size_of::(), - std::mem::size_of::() + std::mem::size_of::(), + std::mem::size_of::(), + std::mem::size_of::(), ), ]; @@ -37528,8 +37616,8 @@ fn measure_what_a_one_word_text_handle_would_save() { saved += control - thin; } - // Padding is why this had to be measured rather than multiplied. Seven - // handles at eight bytes each is fifty-six, and the structs do not + // Padding is why this had to be measured rather than multiplied. Six + // handles at eight bytes each is forty-eight, and the structs do not // necessarily give all of it back: a narrower field can leave padding // behind it instead of closing up. println!("HANDLE inline saving per object = {saved}");