Return only generation when increment

pageserver/ctl/src/draw_timeline_dir.rs

@@ -20,7 +20,7 @@
 //!
 //! # local timeline dir
 //! ls test_output/test_pgbench\[neon-45-684\]/repo/tenants/$TENANT/timelines/$TIMELINE | \
-//!     grep "__" | cargo run --release --bin pagectl draw-timeline > out.svg
+//!     grep "__" | cargo run --release --bin pagectl draw-timeline-dir > out.svg
 //!
 //! # Layer map dump from `/v1/tenant/$TENANT/timeline/$TIMELINE/layer`
 //! (jq -r '.historic_layers[] | .layer_file_name' | cargo  run -p pagectl draw-timeline) < layer-map.json > out.svg
@@ -81,11 +81,7 @@ fn build_coordinate_compression_map<T: Ord + Copy>(coords: Vec<T>) -> BTreeMap<T
 fn parse_filename(name: &str) -> (Range<Key>, Range<Lsn>) {
     let split: Vec<&str> = name.split("__").collect();
     let keys: Vec<&str> = split[0].split('-').collect();
-
-    // Remove the temporary file extension, e.g., remove the `.d20a.___temp` part from the following filename:
-    // 000000067F000040490000404A00441B0000-000000067F000040490000404A00441B4000__000043483A34CE00.d20a.___temp
-    let lsns = split[1].split('.').collect::<Vec<&str>>()[0];
-    let mut lsns: Vec<&str> = lsns.split('-').collect();
+    let mut lsns: Vec<&str> = split[1].split('-').collect();
 
     // The current format of the layer file name: 000000067F0000000400000B150100000000-000000067F0000000400000D350100000000__00000000014B7AC8-v1-00000001
 

pageserver/ctl/src/layers.rs

@@ -13,7 +13,7 @@ use pageserver::{page_cache, virtual_file};
 use pageserver_api::key::Key;
 use utils::id::{TenantId, TimelineId};
 
-use crate::layer_map_analyzer::{LayerFile, parse_filename};
+use crate::layer_map_analyzer::parse_filename;
 
 #[derive(Subcommand)]
 pub(crate) enum LayerCmd {
@@ -38,8 +38,6 @@ pub(crate) enum LayerCmd {
         /// The id from list-layer command
         id: usize,
     },
-    /// Dump all information of a layer file locally
-    DumpLayerLocal { path: PathBuf },
     RewriteSummary {
         layer_file_path: Utf8PathBuf,
         #[clap(long)]
@@ -133,7 +131,15 @@ pub(crate) async fn main(cmd: &LayerCmd) -> Result<()> {
             }
 
             for (idx, layer_file) in to_print {
-                print_layer_file(idx, &layer_file);
+                println!(
+                    "[{:3}]  key:{}-{}\n       lsn:{}-{}\n       delta:{}",
+                    idx,
+                    layer_file.key_range.start,
+                    layer_file.key_range.end,
+                    layer_file.lsn_range.start,
+                    layer_file.lsn_range.end,
+                    layer_file.is_delta,
+                );
             }
             Ok(())
         }
@@ -153,7 +159,16 @@ pub(crate) async fn main(cmd: &LayerCmd) -> Result<()> {
                 let layer = layer?;
                 if let Ok(layer_file) = parse_filename(&layer.file_name().into_string().unwrap()) {
                     if *id == idx {
-                        print_layer_file(idx, &layer_file);
+                        // TODO(chi): dedup code
+                        println!(
+                            "[{:3}]  key:{}-{}\n       lsn:{}-{}\n       delta:{}",
+                            idx,
+                            layer_file.key_range.start,
+                            layer_file.key_range.end,
+                            layer_file.lsn_range.start,
+                            layer_file.lsn_range.end,
+                            layer_file.is_delta,
+                        );
 
                         if layer_file.is_delta {
                             read_delta_file(layer.path(), &ctx).await?;
@@ -168,18 +183,6 @@ pub(crate) async fn main(cmd: &LayerCmd) -> Result<()> {
             }
             Ok(())
         }
-        LayerCmd::DumpLayerLocal { path } => {
-            if let Ok(layer_file) = parse_filename(path.file_name().unwrap().to_str().unwrap()) {
-                print_layer_file(0, &layer_file);
-
-                if layer_file.is_delta {
-                    read_delta_file(path, &ctx).await?;
-                } else {
-                    read_image_file(path, &ctx).await?;
-                }
-            }
-            Ok(())
-        }
         LayerCmd::RewriteSummary {
             layer_file_path,
             new_tenant_id,
@@ -244,15 +247,3 @@ pub(crate) async fn main(cmd: &LayerCmd) -> Result<()> {
         }
     }
 }
-
-fn print_layer_file(idx: usize, layer_file: &LayerFile) {
-    println!(
-        "[{:3}]  key:{}-{}\n       lsn:{}-{}\n       delta:{}",
-        idx,
-        layer_file.key_range.start,
-        layer_file.key_range.end,
-        layer_file.lsn_range.start,
-        layer_file.lsn_range.end,
-        layer_file.is_delta,
-    );
-}

pageserver/page_api/src/model.rs

@@ -26,7 +26,7 @@ use utils::lsn::Lsn;
 use crate::proto;
 
 /// A protocol error. Typically returned via try_from() or try_into().
-#[derive(thiserror::Error, Clone, Debug)]
+#[derive(thiserror::Error, Debug)]
 pub enum ProtocolError {
     #[error("field '{0}' has invalid value '{1}'")]
     Invalid(&'static str, String),
@@ -487,7 +487,6 @@ impl From<GetPageStatusCode> for i32 {
 
 // Fetches the size of a relation at a given LSN, as # of blocks. Only valid on shard 0, other
 // shards will error.
-#[derive(Clone, Copy, Debug)]
 pub struct GetRelSizeRequest {
     pub read_lsn: ReadLsn,
     pub rel: RelTag,
@@ -531,7 +530,6 @@ impl From<GetRelSizeResponse> for proto::GetRelSizeResponse {
 }
 
 /// Requests an SLRU segment. Only valid on shard 0, other shards will error.
-#[derive(Clone, Copy, Debug)]
 pub struct GetSlruSegmentRequest {
     pub read_lsn: ReadLsn,
     pub kind: SlruKind,

storage_controller/src/metrics.rs

@@ -136,8 +136,7 @@ pub(crate) struct StorageControllerMetricGroup {
 
     pub(crate) storage_controller_leadership_status: measured::GaugeVec<LeadershipStatusGroupSet>,
 
-    /// Indicator of stucked (long-running) reconciles, broken down by tenant, shard and sequence.
-    /// The metric is automatically removed once the reconciliation completes.
+    /// HTTP request status counters for handled requests
     pub(crate) storage_controller_reconcile_long_running:
         measured::CounterVec<ReconcileLongRunningLabelGroupSet>,
 

storage_controller/src/persistence.rs

@@ -715,10 +715,10 @@ impl Persistence {
         node_id: NodeId,
     ) -> anyhow::Result<Generation> {
         use crate::schema::tenant_shards::dsl::*;
-        let updated = self
+        let generation_value = self
             .with_measured_conn(DatabaseOperation::IncrementGeneration, move |conn| {
                 Box::pin(async move {
-                    let updated = diesel::update(tenant_shards)
+                    let generation_value: Option<i32> = diesel::update(tenant_shards)
                         .filter(tenant_id.eq(tenant_shard_id.tenant_id.to_string()))
                         .filter(shard_number.eq(tenant_shard_id.shard_number.0 as i32))
                         .filter(shard_count.eq(tenant_shard_id.shard_count.literal() as i32))
@@ -726,20 +726,19 @@ impl Persistence {
                             generation.eq(generation + 1),
                             generation_pageserver.eq(node_id.0 as i64),
                         ))
-                        // TODO: only returning() the generation column
-                        .returning(TenantShardPersistence::as_returning())
+                        .returning(generation)
                         .get_result(conn)
                         .await?;
 
-                    Ok(updated)
+                    Ok(generation_value)
                 })
             })
             .await?;
 
         // Generation is always non-null in the rseult: if the generation column had been NULL, then we
         // should have experienced an SQL Confilict error while executing a query that tries to increment it.
-        debug_assert!(updated.generation.is_some());
-        let Some(g) = updated.generation else {
+        debug_assert!(generation_value.is_some());
+        let Some(g) = generation_value else {
             return Err(DatabaseError::Logical(
                 "Generation should always be set after incrementing".to_string(),
             )

storage_controller/src/service.rs

@@ -123,7 +123,7 @@ pub(crate) const STARTUP_RECONCILE_TIMEOUT: Duration = Duration::from_secs(30);
 
 /// How long a node may be unresponsive to heartbeats before we declare it offline.
 /// This must be long enough to cover node restarts as well as normal operations: in future
-pub const MAX_OFFLINE_INTERVAL_DEFAULT: Duration = Duration::from_secs(10);
+pub const MAX_OFFLINE_INTERVAL_DEFAULT: Duration = Duration::from_secs(30);
 
 /// How long a node may be unresponsive to heartbeats during start up before we declare it
 /// offline.