pageserver: make clearer that Generation::none shouldn't be used on t…

…enants

libs/utils/src/generation.rs

@@ -9,15 +9,11 @@ use serde::{Deserialize, Serialize};
 /// numbers are used.
 #[derive(Copy, Clone, Eq, PartialEq, PartialOrd, Ord, Hash)]
 pub enum Generation {
-    // Generations with this magic value will not add a suffix to S3 keys, and will not
-    // be included in persisted index_part.json.  This value is only to be used
-    // during migration from pre-generation metadata to generation-aware metadata,
-    // and should eventually go away.
-    //
-    // A special Generation is used rather than always wrapping Generation in an Option,
-    // so that code handling generations doesn't have to be aware of the legacy
-    // case everywhere it touches a generation.
+    // The None Generation is used in the metadata of layers written before generations were
+    // introduced.  A running Tenant always has a valid generation, but the layer metadata may
+    // include None generations.
     None,
+
     Valid(u32),
 }
 
@@ -113,7 +109,7 @@ impl Serialize for Generation {
         if let Self::Valid(v) = self {
             v.serialize(serializer)
         } else {
-            // We should never be asked to serialize a None or Broken.  Structures
+            // We should never be asked to serialize a None. Structures
             // that include an optional generation should convert None to an
             // Option<Generation>::None
             Err(serde::ser::Error::custom(

pageserver/src/tenant.rs

@@ -2470,6 +2470,8 @@ impl Tenant {
         remote_storage: GenericRemoteStorage,
         deletion_queue_client: DeletionQueueClient,
     ) -> Tenant {
+        debug_assert!(!attached_conf.location.generation.is_none());
+
         let (state, mut rx) = watch::channel(state);
 
         tokio::spawn(async move {

pageserver/src/tenant/secondary/heatmap_uploader.rs

@@ -368,9 +368,8 @@ async fn upload_tenant_heatmap(
 
     let generation = tenant.get_generation();
     if generation.is_none() {
-        // We do not expect this: generations were implemented before heatmap uploads.  However,
-        // handle it so that we don't have to make the generation in the heatmap an Option<>
-        // (Generation::none is not serializable)
+        // We do not expect this: None generations should only appear in historic layer metadata, not in running Tenants
+        debug_assert!(generation.is_none());
         tracing::warn!("Skipping heatmap upload for tenant with generation==None");
         return Ok(UploadHeatmapOutcome::Skipped);
     }