Return only generation when increment

Move comment above metrics handler (#12236 )
## Problem Comment is in incorrect place: `/metrics` code is above its description comment. ## Summary of changes - `/metrics` code is now below the comment
2026-05-25 17:10:38 +00:00 · 2025-06-14 14:54:24 +04:00 · 2025-06-13 18:18:51 +00:00 · 2025-06-13 17:47:09 +00:00
4 changed files with 16 additions and 17 deletions
--- a/storage_controller/src/http.rs
+++ b/storage_controller/src/http.rs
@@ -676,7 +676,7 @@ async fn handle_tenant_timeline_passthrough(
    service: Arc<Service>,
    req: Request<Body>,
 ) -> Result<Response<Body>, ApiError> {
-    let tenant_or_shard_id: TenantShardId = parse_request_param(&req, "tenant_shard_id")?;
+    let tenant_or_shard_id: TenantShardId = parse_request_param(&req, "tenant_id")?;
    check_permissions(&req, Scope::PageServerApi)?;
    maybe_rate_limit(&req, tenant_or_shard_id.tenant_id).await;

@@ -2070,10 +2070,10 @@ pub fn make_router(

    router
        .data(Arc::new(HttpState::new(service, auth, build_info)))
+        // Non-prefixed generic endpoints (status, metrics, profiling)
        .get("/metrics", |r| {
            named_request_span(r, measured_metrics_handler, RequestName("metrics"))
        })
-        // Non-prefixed generic endpoints (status, metrics, profiling)
        .get("/status", |r| {
            named_request_span(r, handle_status, RequestName("status"))
        })
@@ -2470,7 +2470,7 @@ pub fn make_router(
            },
        )
        // Tenant detail GET passthrough to shard zero:
-        .get("/v1/tenant/:tenant_shard_id", |r| {
+        .get("/v1/tenant/:tenant_id", |r| {
            tenant_service_handler(
                r,
                handle_tenant_timeline_passthrough,
@@ -2480,7 +2480,7 @@ pub fn make_router(
        // The `*` in the  URL is a wildcard: any tenant/timeline GET APIs on the pageserver
        // are implicitly exposed here.  This must be last in the list to avoid
        // taking precedence over other GET methods we might implement by hand.
-        .get("/v1/tenant/:tenant_shard_id/*", |r| {
+        .get("/v1/tenant/:tenant_id/*", |r| {
            tenant_service_handler(
                r,
                handle_tenant_timeline_passthrough,
@@ -2489,7 +2489,7 @@ pub fn make_router(
        })
        // Tenant timeline mark_invisible passthrough to shard zero
        .put(
-            "/v1/tenant/:tenant_shard_id/timeline/:timeline_id/mark_invisible",
+            "/v1/tenant/:tenant_id/timeline/:timeline_id/mark_invisible",
            |r| {
                tenant_service_handler(
                    r,
--- a/storage_controller/src/metrics.rs
+++ b/storage_controller/src/metrics.rs
@@ -141,11 +141,11 @@ pub(crate) struct StorageControllerMetricGroup {
        measured::CounterVec<ReconcileLongRunningLabelGroupSet>,

    /// Indicator of safekeeper reconciler queue depth, broken down by safekeeper, excluding ongoing reconciles.
-    pub(crate) storage_controller_safkeeper_reconciles_queued:
+    pub(crate) storage_controller_safekeeper_reconciles_queued:
        measured::GaugeVec<SafekeeperReconcilerLabelGroupSet>,

    /// Indicator of completed safekeeper reconciles, broken down by safekeeper.
-    pub(crate) storage_controller_safkeeper_reconciles_complete:
+    pub(crate) storage_controller_safekeeper_reconciles_complete:
        measured::CounterVec<SafekeeperReconcilerLabelGroupSet>,
 }

--- a/storage_controller/src/persistence.rs
+++ b/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(),
            )
--- a/storage_controller/src/service/safekeeper_reconciler.rs
+++ b/storage_controller/src/service/safekeeper_reconciler.rs
@@ -230,7 +230,7 @@ impl ReconcilerHandle {
        // increase it before putting into the queue.
        let queued_gauge = &METRICS_REGISTRY
            .metrics_group
-            .storage_controller_safkeeper_reconciles_queued;
+            .storage_controller_safekeeper_reconciles_queued;
        let label_group = SafekeeperReconcilerLabelGroup {
            sk_az: &sk_az,
            sk_node_id: &sk_node_id,
@@ -306,7 +306,7 @@ impl SafekeeperReconciler {

            let queued_gauge = &METRICS_REGISTRY
                .metrics_group
-                .storage_controller_safkeeper_reconciles_queued;
+                .storage_controller_safekeeper_reconciles_queued;
            queued_gauge.set(
                SafekeeperReconcilerLabelGroup {
                    sk_az: &req.safekeeper.skp.availability_zone_id,
@@ -547,7 +547,7 @@ impl SafekeeperReconcilerInner {

                    let complete_counter = &METRICS_REGISTRY
                        .metrics_group
-                        .storage_controller_safkeeper_reconciles_complete;
+                        .storage_controller_safekeeper_reconciles_complete;
                    complete_counter.inc(SafekeeperReconcilerLabelGroup {
                        sk_az: &req.safekeeper.skp.availability_zone_id,
                        sk_node_id: &req.safekeeper.get_id().to_string(),