Improve assertions

libs/pageserver_api/src/shard.rs

@@ -158,7 +158,8 @@ impl ShardIdentity {
         key_to_shard_number(self.count, self.stripe_size, key)
     }
 
-    /// Return true if the key should be ingested by this shard
+    /// Return true if the key is stored only on this shard. This does not include
+    /// global keys, see is_key_global().
     ///
     /// Shards must ingest _at least_ keys which return true from this check.
     pub fn is_key_local(&self, key: &Key) -> bool {
@@ -171,7 +172,7 @@ impl ShardIdentity {
     }
 
     /// Return true if the key should be stored on all shards, not just one.
-    fn is_key_global(&self, key: &Key) -> bool {
+    pub fn is_key_global(&self, key: &Key) -> bool {
         if key.is_slru_block_key() || key.is_slru_segment_size_key() || key.is_aux_file_key() {
             // Special keys that are only stored on shard 0
             false

pageserver/src/tenant/timeline.rs

@@ -5889,13 +5889,17 @@ impl<'a> TimelineWriter<'a> {
         }
 
         // In debug builds, assert that we don't write any keys that don't belong to this shard.
+        // We don't assert this in release builds, since key ownership policies may change over
+        // time. Stray keys will be removed during compaction.
         if cfg!(debug_assertions) {
             for metadata in &batch.metadata {
                 if let ValueMeta::Serialized(metadata) = metadata {
                     let key = Key::from_compact(metadata.key);
                     assert!(
-                        self.shard_identity.is_key_local(&key),
-                        "writing non-local key {key}",
+                        self.shard_identity.is_key_local(&key)
+                            || self.shard_identity.is_key_global(&key),
+                        "key {key} does not belong on shard {}",
+                        self.shard_identity.shard_index()
                     );
                 }
             }

pageserver/src/tenant/timeline/compaction.rs

@@ -1174,10 +1174,10 @@ impl Timeline {
                     .await
                     .map_err(CompactionError::Other)?;
             } else {
+                let shard = self.shard_identity.shard_index();
                 let owner = self.shard_identity.get_shard_number(&key);
-                let this = self.shard_identity.shard_index();
                 if cfg!(debug_assertions) {
-                    panic!("found key {key} belonging to shard {owner} on shard {this}");
+                    panic!("key {key} does not belong on shard {shard}, owned by {owner}");
                 }
                 debug!("dropping key {key} during compaction (it belongs on shard {owner})");
             }
@@ -2051,7 +2051,8 @@ impl Timeline {
                 // a whole layer file as handling key spaces (ranges).
                 if cfg!(debug_assertions) {
                     let shard = self.shard_identity.shard_index();
-                    panic!("key {key} does not belong on shard {shard}");
+                    let owner = self.shard_identity.get_shard_number(&key);
+                    panic!("key {key} does not belong on shard {shard}, owned by {owner}");
                 }
                 continue;
             }