From 982fce1e727211009e002db8675d4a3c73b21439 Mon Sep 17 00:00:00 2001 From: arpad-m Date: Sat, 15 Jul 2023 04:11:25 +0200 Subject: [PATCH] Fix rustdoc warnings and test cargo doc in CI (#4711) ## Problem `cargo +nightly doc` is giving a lot of warnings: broken links, naked URLs, etc. ## Summary of changes * update the `proc-macro2` dependency so that it can compile on latest Rust nightly, see https://github.com/dtolnay/proc-macro2/pull/391 and https://github.com/dtolnay/proc-macro2/issues/398 * allow the `private_intra_doc_links` lint, as linking to something that's private is always more useful than just mentioning it without a link: if the link breaks in the future, at least there is a warning due to that. Also, one might enable [`--document-private-items`](https://doc.rust-lang.org/cargo/commands/cargo-doc.html#documentation-options) in the future and make these links work in general. * fix all the remaining warnings given by `cargo +nightly doc` * make it possible to run `cargo doc` on stable Rust by updating `opentelemetry` and associated crates to version 0.19, pulling in a fix that previously broke `cargo doc` on stable: https://github.com/open-telemetry/opentelemetry-rust/pull/904 * Add `cargo doc` to CI to ensure that it won't get broken in the future. Fixes #2557 ## Future work * Potentially, it might make sense, for development purposes, to publish the generated rustdocs somewhere, like for example [how the rust compiler does it](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_driver/index.html). I will file an issue for discussion. --- .cargo/config.toml | 5 ++ .github/workflows/build_and_test.yml | 5 ++ Cargo.lock | 58 +++++++------------ Cargo.toml | 10 ++-- compute_tools/src/pg_helpers.rs | 2 +- control_plane/src/background_process.rs | 2 +- control_plane/src/broker.rs | 3 +- control_plane/src/endpoint.rs | 4 +- control_plane/src/safekeeper.rs | 3 +- libs/metrics/src/metric_vec_duration.rs | 2 +- libs/pageserver_api/src/models.rs | 6 +- libs/pageserver_api/src/reltag.rs | 3 +- libs/postgres_ffi/src/relfile_utils.rs | 4 +- libs/pq_proto/src/framed.rs | 6 +- libs/remote_storage/src/lib.rs | 6 +- libs/tenant_size_model/src/calculation.rs | 2 +- libs/tracing-utils/src/http.rs | 2 +- libs/utils/src/http/json.rs | 2 +- libs/utils/src/lib.rs | 4 +- libs/utils/src/lock_file.rs | 9 +-- libs/utils/src/logging.rs | 2 +- libs/utils/src/seqwait.rs | 6 +- libs/utils/src/tracing_span_assert.rs | 2 +- pageserver/ctl/src/draw_timeline_dir.rs | 6 +- pageserver/src/config.rs | 6 +- pageserver/src/context.rs | 3 + pageserver/src/http/routes.rs | 2 +- pageserver/src/metrics.rs | 10 ++-- pageserver/src/pgdatadir_mapping.rs | 2 +- pageserver/src/task_mgr.rs | 13 +++-- pageserver/src/tenant.rs | 18 +++--- pageserver/src/tenant/disk_btree.rs | 2 +- pageserver/src/tenant/layer_map.rs | 4 +- .../layer_map/historic_layer_coverage.rs | 2 +- .../src/tenant/layer_map/layer_coverage.rs | 4 +- pageserver/src/tenant/manifest.rs | 2 +- pageserver/src/tenant/metadata.rs | 4 +- .../src/tenant/remote_timeline_client.rs | 8 ++- pageserver/src/tenant/size.rs | 4 +- pageserver/src/tenant/storage_layer.rs | 18 ++++-- .../src/tenant/storage_layer/delta_layer.rs | 8 ++- .../src/tenant/storage_layer/filename.rs | 5 +- .../src/tenant/storage_layer/image_layer.rs | 6 +- .../src/tenant/storage_layer/remote_layer.rs | 4 +- pageserver/src/tenant/timeline.rs | 32 ++++++---- .../walreceiver/connection_manager.rs | 2 +- proxy/src/cancellation.rs | 2 +- proxy/src/compute.rs | 2 +- proxy/src/config.rs | 2 +- proxy/src/console/provider.rs | 2 +- proxy/src/scram.rs | 2 +- safekeeper/src/control_file.rs | 5 +- safekeeper/src/timelines_global_map.rs | 2 +- safekeeper/src/wal_storage.rs | 6 +- test_runner/regress/test_tenant_detach.py | 2 +- 55 files changed, 200 insertions(+), 138 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index 8fddaa2dd4..cc767a7f68 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -12,6 +12,11 @@ opt-level = 3 # Turn on a small amount of optimization in Development mode. opt-level = 1 +[build] +# This is only present for local builds, as it will be overridden +# by the RUSTDOCFLAGS env var in CI. +rustdocflags = ["-Arustdoc::private_intra_doc_links"] + [alias] build_testing = ["build", "--features", "testing"] neon = ["run", "--bin", "neon_local"] diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 873f8570fc..fb19d54aaa 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -127,6 +127,11 @@ jobs: - name: Run cargo clippy (release) run: cargo hack --feature-powerset clippy --release $CLIPPY_COMMON_ARGS + - name: Check documentation generation + run: cargo doc --workspace --no-deps --document-private-items + env: + RUSTDOCFLAGS: "-Dwarnings -Arustdoc::private_intra_doc_links" + # Use `${{ !cancelled() }}` to run quck tests after the longer clippy run - name: Check formatting if: ${{ !cancelled() }} diff --git a/Cargo.lock b/Cargo.lock index 45f5acce7d..6bbb5dbfa2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2379,9 +2379,9 @@ dependencies = [ [[package]] name = "opentelemetry" -version = "0.18.0" +version = "0.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "69d6c3d7288a106c0a363e4b0e8d308058d56902adefb16f4936f417ffef086e" +checksum = "5f4b8347cc26099d3aeee044065ecc3ae11469796b4d65d065a23a584ed92a6f" dependencies = [ "opentelemetry_api", "opentelemetry_sdk", @@ -2389,9 +2389,9 @@ dependencies = [ [[package]] name = "opentelemetry-http" -version = "0.7.0" +version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1edc79add46364183ece1a4542592ca593e6421c60807232f5b8f7a31703825d" +checksum = "a819b71d6530c4297b49b3cae2939ab3a8cc1b9f382826a1bc29dd0ca3864906" dependencies = [ "async-trait", "bytes", @@ -2402,9 +2402,9 @@ dependencies = [ [[package]] name = "opentelemetry-otlp" -version = "0.11.0" +version = "0.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d1c928609d087790fc936a1067bdc310ae702bdf3b090c3f281b713622c8bbde" +checksum = "8af72d59a4484654ea8eb183fea5ae4eb6a41d7ac3e3bae5f4d2a282a3a7d3ca" dependencies = [ "async-trait", "futures", @@ -2420,48 +2420,47 @@ dependencies = [ [[package]] name = "opentelemetry-proto" -version = "0.1.0" +version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d61a2f56df5574508dd86aaca016c917489e589ece4141df1b5e349af8d66c28" +checksum = "045f8eea8c0fa19f7d48e7bc3128a39c2e5c533d5c61298c548dfefc1064474c" dependencies = [ "futures", "futures-util", "opentelemetry", "prost", "tonic 0.8.3", - "tonic-build 0.8.4", ] [[package]] name = "opentelemetry-semantic-conventions" -version = "0.10.0" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9b02e0230abb0ab6636d18e2ba8fa02903ea63772281340ccac18e0af3ec9eeb" +checksum = "24e33428e6bf08c6f7fcea4ddb8e358fab0fe48ab877a87c70c6ebe20f673ce5" dependencies = [ "opentelemetry", ] [[package]] name = "opentelemetry_api" -version = "0.18.0" +version = "0.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c24f96e21e7acc813c7a8394ee94978929db2bcc46cf6b5014fc612bf7760c22" +checksum = "ed41783a5bf567688eb38372f2b7a8530f5a607a4b49d38dd7573236c23ca7e2" dependencies = [ "fnv", "futures-channel", "futures-util", "indexmap", - "js-sys", "once_cell", "pin-project-lite", "thiserror", + "urlencoding", ] [[package]] name = "opentelemetry_sdk" -version = "0.18.0" +version = "0.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1ca41c4933371b61c2a2f214bf16931499af4ec90543604ec828f7a625c09113" +checksum = "8b3a2a91fdbfdd4d212c0dcc2ab540de2c2bcbbd90be17de7a7daf8822d010c1" dependencies = [ "async-trait", "crossbeam-channel", @@ -2937,9 +2936,9 @@ checksum = "dc375e1527247fe1a97d8b7156678dfe7c1af2fc075c9a4db3690ecd2a148068" [[package]] name = "proc-macro2" -version = "1.0.58" +version = "1.0.64" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fa1fb82fc0c281dd9671101b66b771ebbe1eaf967b96ac8740dcba4b70005ca8" +checksum = "78803b62cbf1f46fde80d7c0e803111524b9877184cfe7c3033659490ac7a7da" dependencies = [ "unicode-ident", ] @@ -3328,9 +3327,9 @@ dependencies = [ [[package]] name = "reqwest-tracing" -version = "0.4.4" +version = "0.4.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "783e8130d2427ddd7897dd3f814d4a3aea31b05deb42a4fdf8c18258fe5aefd1" +checksum = "1b97ad83c2fc18113346b7158d79732242002427c30f620fa817c1f32901e0a8" dependencies = [ "anyhow", "async-trait", @@ -3998,7 +3997,7 @@ dependencies = [ "tokio", "tokio-stream", "tonic 0.9.2", - "tonic-build 0.9.2", + "tonic-build", "tracing", "utils", "workspace_hack", @@ -4516,19 +4515,6 @@ dependencies = [ "tracing", ] -[[package]] -name = "tonic-build" -version = "0.8.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5bf5e9b9c0f7e0a7c027dcfaba7b2c60816c7049171f679d99ee2ff65d0de8c4" -dependencies = [ - "prettyplease 0.1.25", - "proc-macro2", - "prost-build", - "quote", - "syn 1.0.109", -] - [[package]] name = "tonic-build" version = "0.9.2" @@ -4652,9 +4638,9 @@ dependencies = [ [[package]] name = "tracing-opentelemetry" -version = "0.18.0" +version = "0.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "21ebb87a95ea13271332df069020513ab70bdb5637ca42d6e492dc3bbbad48de" +checksum = "00a39dcf9bfc1742fa4d6215253b33a6e474be78275884c216fc2a06267b3600" dependencies = [ "once_cell", "opentelemetry", diff --git a/Cargo.toml b/Cargo.toml index 6d35334deb..6ae31c13ac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -84,9 +84,9 @@ notify = "5.0.0" num_cpus = "1.15" num-traits = "0.2.15" once_cell = "1.13" -opentelemetry = "0.18.0" -opentelemetry-otlp = { version = "0.11.0", default_features=false, features = ["http-proto", "trace", "http", "reqwest-client"] } -opentelemetry-semantic-conventions = "0.10.0" +opentelemetry = "0.19.0" +opentelemetry-otlp = { version = "0.12.0", default_features=false, features = ["http-proto", "trace", "http", "reqwest-client"] } +opentelemetry-semantic-conventions = "0.11.0" parking_lot = "0.12" pbkdf2 = "0.12.1" pin-project-lite = "0.2" @@ -95,7 +95,7 @@ prost = "0.11" rand = "0.8" regex = "1.4" reqwest = { version = "0.11", default-features = false, features = ["rustls-tls"] } -reqwest-tracing = { version = "0.4.0", features = ["opentelemetry_0_18"] } +reqwest-tracing = { version = "0.4.0", features = ["opentelemetry_0_19"] } reqwest-middleware = "0.2.0" reqwest-retry = "0.2.2" routerify = "3" @@ -130,7 +130,7 @@ toml_edit = "0.19" tonic = {version = "0.9", features = ["tls", "tls-roots"]} tracing = "0.1" tracing-error = "0.2.0" -tracing-opentelemetry = "0.18.0" +tracing-opentelemetry = "0.19.0" tracing-subscriber = { version = "0.3", default_features = false, features = ["smallvec", "fmt", "tracing-log", "std", "env-filter"] } url = "2.2" uuid = { version = "1.2", features = ["v4", "serde"] } diff --git a/compute_tools/src/pg_helpers.rs b/compute_tools/src/pg_helpers.rs index 75550978d8..b94a97a126 100644 --- a/compute_tools/src/pg_helpers.rs +++ b/compute_tools/src/pg_helpers.rs @@ -19,7 +19,7 @@ const POSTGRES_WAIT_TIMEOUT: Duration = Duration::from_millis(60 * 1000); // mil /// Escape a string for including it in a SQL literal. Wrapping the result /// with `E'{}'` or `'{}'` is not required, as it returns a ready-to-use /// SQL string literal, e.g. `'db'''` or `E'db\\'`. -/// See https://github.com/postgres/postgres/blob/da98d005cdbcd45af563d0c4ac86d0e9772cd15f/src/backend/utils/adt/quote.c#L47 +/// See /// for the original implementation. pub fn escape_literal(s: &str) -> String { let res = s.replace('\'', "''").replace('\\', "\\\\"); diff --git a/control_plane/src/background_process.rs b/control_plane/src/background_process.rs index 00af1a1d53..64664d65ff 100644 --- a/control_plane/src/background_process.rs +++ b/control_plane/src/background_process.rs @@ -10,7 +10,7 @@ //! (non-Neon binaries don't necessarily follow our pidfile conventions). //! The pid stored in the file is later used to stop the service. //! -//! See [`lock_file`] module for more info. +//! See the [`lock_file`](utils::lock_file) module for more info. use std::ffi::OsStr; use std::io::Write; diff --git a/control_plane/src/broker.rs b/control_plane/src/broker.rs index ad19dfa204..8d40c7afc1 100644 --- a/control_plane/src/broker.rs +++ b/control_plane/src/broker.rs @@ -2,8 +2,9 @@ //! //! In the local test environment, the data for each safekeeper is stored in //! +//! ```text //! .neon/safekeepers/ -//! +//! ``` use anyhow::Context; use std::path::PathBuf; diff --git a/control_plane/src/endpoint.rs b/control_plane/src/endpoint.rs index ab921d096f..ff373d7111 100644 --- a/control_plane/src/endpoint.rs +++ b/control_plane/src/endpoint.rs @@ -2,7 +2,9 @@ //! //! In the local test environment, the data for each endpoint is stored in //! +//! ```text //! .neon/endpoints/ +//! ``` //! //! Some basic information about the endpoint, like the tenant and timeline IDs, //! are stored in the `endpoint.json` file. The `endpoint.json` file is created @@ -22,7 +24,7 @@ //! //! Directory contents: //! -//! ```ignore +//! ```text //! .neon/endpoints/main/ //! compute.log - log output of `compute_ctl` and `postgres` //! endpoint.json - serialized `EndpointConf` struct diff --git a/control_plane/src/safekeeper.rs b/control_plane/src/safekeeper.rs index 9e053ff1f1..d5e0fb112f 100644 --- a/control_plane/src/safekeeper.rs +++ b/control_plane/src/safekeeper.rs @@ -2,8 +2,9 @@ //! //! In the local test environment, the data for each safekeeper is stored in //! +//! ```text //! .neon/safekeepers/ -//! +//! ``` use std::io::Write; use std::path::PathBuf; use std::process::Child; diff --git a/libs/metrics/src/metric_vec_duration.rs b/libs/metrics/src/metric_vec_duration.rs index 840f60f19b..e9a0a65570 100644 --- a/libs/metrics/src/metric_vec_duration.rs +++ b/libs/metrics/src/metric_vec_duration.rs @@ -1,4 +1,4 @@ -//! Helpers for observing duration on HistogramVec / CounterVec / GaugeVec / MetricVec. +//! Helpers for observing duration on `HistogramVec` / `CounterVec` / `GaugeVec` / `MetricVec`. use std::{future::Future, time::Instant}; diff --git a/libs/pageserver_api/src/models.rs b/libs/pageserver_api/src/models.rs index df5f5896a1..4c6529ffab 100644 --- a/libs/pageserver_api/src/models.rs +++ b/libs/pageserver_api/src/models.rs @@ -411,12 +411,16 @@ pub struct LayerResidenceEvent { pub reason: LayerResidenceEventReason, } -/// The reason for recording a given [`ResidenceEvent`]. +/// The reason for recording a given [`LayerResidenceEvent`]. #[derive(Debug, Clone, Copy, Serialize, Deserialize)] pub enum LayerResidenceEventReason { /// The layer map is being populated, e.g. during timeline load or attach. /// This includes [`RemoteLayer`] objects created in [`reconcile_with_remote`]. /// We need to record such events because there is no persistent storage for the events. + /// + // https://github.com/rust-lang/rust/issues/74481 + /// [`RemoteLayer`]: ../../tenant/storage_layer/struct.RemoteLayer.html + /// [`reconcile_with_remote`]: ../../tenant/struct.Timeline.html#method.reconcile_with_remote LayerLoad, /// We just created the layer (e.g., freeze_and_flush or compaction). /// Such layers are always [`LayerResidenceStatus::Resident`]. diff --git a/libs/pageserver_api/src/reltag.rs b/libs/pageserver_api/src/reltag.rs index 12693379f5..c98ad259bf 100644 --- a/libs/pageserver_api/src/reltag.rs +++ b/libs/pageserver_api/src/reltag.rs @@ -60,8 +60,9 @@ impl Ord for RelTag { /// Display RelTag in the same format that's used in most PostgreSQL debug messages: /// +/// ```text /// //[_fsm|_vm|_init] -/// +/// ``` impl fmt::Display for RelTag { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { if let Some(forkname) = forknumber_to_name(self.forknum) { diff --git a/libs/postgres_ffi/src/relfile_utils.rs b/libs/postgres_ffi/src/relfile_utils.rs index 1dc9f367ff..aa0e625b47 100644 --- a/libs/postgres_ffi/src/relfile_utils.rs +++ b/libs/postgres_ffi/src/relfile_utils.rs @@ -49,14 +49,16 @@ pub fn forknumber_to_name(forknum: u8) -> Option<&'static str> { } } -/// /// Parse a filename of a relation file. Returns (relfilenode, forknum, segno) tuple. /// /// Formats: +/// +/// ```text /// /// _ /// . /// _. +/// ``` /// /// See functions relpath() and _mdfd_segpath() in PostgreSQL sources. /// diff --git a/libs/pq_proto/src/framed.rs b/libs/pq_proto/src/framed.rs index 3cdca45009..c12898a05c 100644 --- a/libs/pq_proto/src/framed.rs +++ b/libs/pq_proto/src/framed.rs @@ -5,11 +5,11 @@ //! It is similar to what tokio_util::codec::Framed with appropriate codec //! provides, but `FramedReader` and `FramedWriter` read/write parts can be used //! separately without using split from futures::stream::StreamExt (which -//! allocates box[1] in polling internally). tokio::io::split is used for splitting +//! allocates a [Box] in polling internally). tokio::io::split is used for splitting //! instead. Plus we customize error messages more than a single type for all io //! calls. //! -//! [1] https://docs.rs/futures-util/0.3.26/src/futures_util/lock/bilock.rs.html#107 +//! [Box]: https://docs.rs/futures-util/0.3.26/src/futures_util/lock/bilock.rs.html#107 use bytes::{Buf, BytesMut}; use std::{ future::Future, @@ -117,7 +117,7 @@ impl Framed { impl Framed { /// Split into owned read and write parts. Beware of potential issues with /// using halves in different tasks on TLS stream: - /// https://github.com/tokio-rs/tls/issues/40 + /// pub fn split(self) -> (FramedReader, FramedWriter) { let (read_half, write_half) = tokio::io::split(self.stream); let reader = FramedReader { diff --git a/libs/remote_storage/src/lib.rs b/libs/remote_storage/src/lib.rs index 90f9efa146..92ef793a34 100644 --- a/libs/remote_storage/src/lib.rs +++ b/libs/remote_storage/src/lib.rs @@ -34,12 +34,12 @@ pub const DEFAULT_REMOTE_STORAGE_MAX_CONCURRENT_SYNCS: usize = 50; pub const DEFAULT_REMOTE_STORAGE_MAX_SYNC_ERRORS: u32 = 10; /// Currently, sync happens with AWS S3, that has two limits on requests per second: /// ~200 RPS for IAM services -/// https://docs.aws.amazon.com/AmazonRDS/latest/AuroraUserGuide/UsingWithRDS.IAMDBAuth.html +/// /// ~3500 PUT/COPY/POST/DELETE or 5500 GET/HEAD S3 requests -/// https://aws.amazon.com/premiumsupport/knowledge-center/s3-request-limit-avoid-throttling/ +/// pub const DEFAULT_REMOTE_STORAGE_S3_CONCURRENCY_LIMIT: usize = 100; /// No limits on the client side, which currenltly means 1000 for AWS S3. -/// https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjectsV2.html#API_ListObjectsV2_RequestSyntax +/// pub const DEFAULT_MAX_KEYS_PER_LIST_RESPONSE: Option = None; const REMOTE_STORAGE_PREFIX_SEPARATOR: char = '/'; diff --git a/libs/tenant_size_model/src/calculation.rs b/libs/tenant_size_model/src/calculation.rs index 093b053675..f05997ee65 100644 --- a/libs/tenant_size_model/src/calculation.rs +++ b/libs/tenant_size_model/src/calculation.rs @@ -21,7 +21,7 @@ use crate::{SegmentMethod, SegmentSizeResult, SizeResult, StorageModel}; // 2. D+C+a+b // 3. D+A+B -/// [`Segment`] which has had it's size calculated. +/// `Segment` which has had its size calculated. #[derive(Clone, Debug)] struct SegmentSize { method: SegmentMethod, diff --git a/libs/tracing-utils/src/http.rs b/libs/tracing-utils/src/http.rs index 3f80f49de1..f5ab267ff3 100644 --- a/libs/tracing-utils/src/http.rs +++ b/libs/tracing-utils/src/http.rs @@ -33,7 +33,7 @@ pub enum OtelName<'a> { /// directly into HTTP servers. However, I couldn't find one for Hyper, /// so I had to write our own. OpenTelemetry website has a registry of /// instrumentation libraries at: -/// https://opentelemetry.io/registry/?language=rust&component=instrumentation +/// /// If a Hyper crate appears, consider switching to that. pub async fn tracing_handler( req: Request, diff --git a/libs/utils/src/http/json.rs b/libs/utils/src/http/json.rs index 9c153033cb..70e682cb76 100644 --- a/libs/utils/src/http/json.rs +++ b/libs/utils/src/http/json.rs @@ -14,7 +14,7 @@ pub async fn json_request Deserialize<'de>>( .map_err(ApiError::BadRequest) } -/// Will be removed as part of https://github.com/neondatabase/neon/issues/4282 +/// Will be removed as part of pub async fn json_request_or_empty_body Deserialize<'de>>( request: &mut Request, ) -> Result, ApiError> { diff --git a/libs/utils/src/lib.rs b/libs/utils/src/lib.rs index c3558443c3..3bcb092ba7 100644 --- a/libs/utils/src/lib.rs +++ b/libs/utils/src/lib.rs @@ -130,8 +130,8 @@ pub use failpoint_macro_helpers::failpoint_sleep_helper; /// Note that with git_version prefix is `git:` and in case of git version from env its `git-env:`. /// /// ############################################################################################# -/// TODO this macro is not the way the library is intended to be used, see https://github.com/neondatabase/neon/issues/1565 for details. -/// We use `cachepot` to reduce our current CI build times: https://github.com/neondatabase/cloud/pull/1033#issuecomment-1100935036 +/// TODO this macro is not the way the library is intended to be used, see for details. +/// We use `cachepot` to reduce our current CI build times: /// Yet, it seems to ignore the GIT_VERSION env variable, passed to Docker build, even with build.rs that contains /// `println!("cargo:rerun-if-env-changed=GIT_VERSION");` code for cachepot cache invalidation. /// The problem needs further investigation and regular `const` declaration instead of a macro. diff --git a/libs/utils/src/lock_file.rs b/libs/utils/src/lock_file.rs index adbf47eb7a..ca8295040c 100644 --- a/libs/utils/src/lock_file.rs +++ b/libs/utils/src/lock_file.rs @@ -1,9 +1,10 @@ //! A module to create and read lock files. //! //! File locking is done using [`fcntl::flock`] exclusive locks. -//! The only consumer of this module is currently [`pid_file`]. -//! See the module-level comment there for potential pitfalls -//! with lock files that are used to store PIDs (pidfiles). +//! The only consumer of this module is currently +//! [`pid_file`](crate::pid_file). See the module-level comment +//! there for potential pitfalls with lock files that are used +//! to store PIDs (pidfiles). use std::{ fs, @@ -81,7 +82,7 @@ pub fn create_exclusive(lock_file_path: &Path) -> anyhow::Result TracingPanicHookGuard { diff --git a/libs/utils/src/seqwait.rs b/libs/utils/src/seqwait.rs index 70cf4a1ce9..014887392e 100644 --- a/libs/utils/src/seqwait.rs +++ b/libs/utils/src/seqwait.rs @@ -23,9 +23,9 @@ pub enum SeqWaitError { /// Monotonically increasing value /// -/// It is handy to store some other fields under the same mutex in SeqWait +/// It is handy to store some other fields under the same mutex in `SeqWait` /// (e.g. store prev_record_lsn). So we allow SeqWait to be parametrized with -/// any type that can expose counter. is the type of exposed counter. +/// any type that can expose counter. `V` is the type of exposed counter. pub trait MonotonicCounter { /// Bump counter value and check that it goes forward /// N.B.: new_val is an actual new value, not a difference. @@ -90,7 +90,7 @@ impl Eq for Waiter {} /// [`wait_for`]: SeqWait::wait_for /// [`advance`]: SeqWait::advance /// -/// means Storage, is type of counter that this storage exposes. +/// `S` means Storage, `V` is type of counter that this storage exposes. /// pub struct SeqWait where diff --git a/libs/utils/src/tracing_span_assert.rs b/libs/utils/src/tracing_span_assert.rs index 02c3ea0361..926bfc3188 100644 --- a/libs/utils/src/tracing_span_assert.rs +++ b/libs/utils/src/tracing_span_assert.rs @@ -35,7 +35,7 @@ //! # } //! ``` //! -//! Recommended reading: https://docs.rs/tracing-subscriber/0.3.16/tracing_subscriber/layer/index.html#per-layer-filtering +//! Recommended reading: //! #[derive(Debug)] diff --git a/pageserver/ctl/src/draw_timeline_dir.rs b/pageserver/ctl/src/draw_timeline_dir.rs index 43e0c4422c..568078808f 100644 --- a/pageserver/ctl/src/draw_timeline_dir.rs +++ b/pageserver/ctl/src/draw_timeline_dir.rs @@ -7,10 +7,10 @@ //! - The y axis represents LSN, growing upwards. //! //! Coordinates in both axis are compressed for better readability. -//! (see https://medium.com/algorithms-digest/coordinate-compression-2fff95326fb) +//! (see ) //! //! Example use: -//! ``` +//! ```bash //! $ ls test_output/test_pgbench\[neon-45-684\]/repo/tenants/$TENANT/timelines/$TIMELINE | \ //! $ grep "__" | cargo run --release --bin pagectl draw-timeline-dir > out.svg //! $ firefox out.svg @@ -20,7 +20,7 @@ //! or from pageserver log files. //! //! TODO Consider shipping this as a grafana panel plugin: -//! https://grafana.com/tutorials/build-a-panel-plugin/ +//! use anyhow::Result; use pageserver::repository::Key; use std::cmp::Ordering; diff --git a/pageserver/src/config.rs b/pageserver/src/config.rs index f870c85355..4c6df469aa 100644 --- a/pageserver/src/config.rs +++ b/pageserver/src/config.rs @@ -171,11 +171,13 @@ pub struct PageServerConf { pub log_format: LogFormat, - /// Number of concurrent [`Tenant::gather_size_inputs`] allowed. + /// Number of concurrent [`Tenant::gather_size_inputs`](crate::tenant::Tenant::gather_size_inputs) allowed. pub concurrent_tenant_size_logical_size_queries: ConfigurableSemaphore, /// Limit of concurrent [`Tenant::gather_size_inputs`] issued by module `eviction_task`. /// The number of permits is the same as `concurrent_tenant_size_logical_size_queries`. /// See the comment in `eviction_task` for details. + /// + /// [`Tenant::gather_size_inputs`]: crate::tenant::Tenant::gather_size_inputs pub eviction_task_immitated_concurrent_logical_size_queries: ConfigurableSemaphore, // How often to collect metrics and send them to the metrics endpoint. @@ -993,6 +995,8 @@ impl ConfigurableSemaphore { /// Require a non-zero initial permits, because using permits == 0 is a crude way to disable a /// feature such as [`Tenant::gather_size_inputs`]. Otherwise any semaphore using future will /// behave like [`futures::future::pending`], just waiting until new permits are added. + /// + /// [`Tenant::gather_size_inputs`]: crate::tenant::Tenant::gather_size_inputs pub fn new(initial_permits: NonZeroUsize) -> Self { ConfigurableSemaphore { initial_permits, diff --git a/pageserver/src/context.rs b/pageserver/src/context.rs index f53b7736ab..a1a5c30ae9 100644 --- a/pageserver/src/context.rs +++ b/pageserver/src/context.rs @@ -179,6 +179,9 @@ impl RequestContext { /// a context and you are unwilling to change all callers to provide one. /// /// Before we add cancellation, we should get rid of this method. + /// + /// [`attached_child`]: Self::attached_child + /// [`detached_child`]: Self::detached_child pub fn todo_child(task_kind: TaskKind, download_behavior: DownloadBehavior) -> Self { Self::new(task_kind, download_behavior) } diff --git a/pageserver/src/http/routes.rs b/pageserver/src/http/routes.rs index f39db891e1..08fb917fb6 100644 --- a/pageserver/src/http/routes.rs +++ b/pageserver/src/http/routes.rs @@ -1143,7 +1143,7 @@ async fn disk_usage_eviction_run( let Some(storage) = state.remote_storage.clone() else { return Err(ApiError::InternalServerError(anyhow::anyhow!( "remote storage not configured, cannot run eviction iteration" - ))) + ))); }; let state = state.disk_usage_eviction_state.clone(); diff --git a/pageserver/src/metrics.rs b/pageserver/src/metrics.rs index 96d23e220f..dc0c1c03b7 100644 --- a/pageserver/src/metrics.rs +++ b/pageserver/src/metrics.rs @@ -385,7 +385,7 @@ pub static UNEXPECTED_ONDEMAND_DOWNLOADS: Lazy = Lazy::new(|| { .expect("failed to define a metric") }); -/// Each [`Timeline`]'s [`EVICTIONS_WITH_LOW_RESIDENCE_DURATION`] metric. +/// Each `Timeline`'s [`EVICTIONS_WITH_LOW_RESIDENCE_DURATION`] metric. #[derive(Debug)] pub struct EvictionsWithLowResidenceDuration { data_source: &'static str, @@ -818,7 +818,7 @@ pub static WAL_REDO_RECORD_COUNTER: Lazy = Lazy::new(|| { .unwrap() }); -/// Similar to [`prometheus::HistogramTimer`] but does not record on drop. +/// Similar to `prometheus::HistogramTimer` but does not record on drop. pub struct StorageTimeMetricsTimer { metrics: StorageTimeMetrics, start: Instant, @@ -876,7 +876,7 @@ impl StorageTimeMetrics { /// Starts timing a new operation. /// - /// Note: unlike [`prometheus::HistogramTimer`] the returned timer does not record on drop. + /// Note: unlike `prometheus::HistogramTimer` the returned timer does not record on drop. pub fn start_timer(&self) -> StorageTimeMetricsTimer { StorageTimeMetricsTimer::new(self.clone()) } @@ -1256,7 +1256,7 @@ impl RemoteTimelineClientMetrics { /// Update the metrics that change when a call to the remote timeline client instance starts. /// /// Drop the returned guard object once the operation is finished to updates corresponding metrics that track completions. - /// Or, use [`RemoteTimelineClientCallMetricGuard::will_decrement_manually`] and [`call_end`] if that + /// Or, use [`RemoteTimelineClientCallMetricGuard::will_decrement_manually`] and [`call_end`](Self::call_end) if that /// is more suitable. /// Never do both. pub(crate) fn call_begin( @@ -1289,7 +1289,7 @@ impl RemoteTimelineClientMetrics { /// Manually udpate the metrics that track completions, instead of using the guard object. /// Using the guard object is generally preferable. - /// See [`call_begin`] for more context. + /// See [`call_begin`](Self::call_begin) for more context. pub(crate) fn call_end( &self, file_kind: &RemoteOpFileKind, diff --git a/pageserver/src/pgdatadir_mapping.rs b/pageserver/src/pgdatadir_mapping.rs index a54cf9f91b..54b41f3e9d 100644 --- a/pageserver/src/pgdatadir_mapping.rs +++ b/pageserver/src/pgdatadir_mapping.rs @@ -1131,7 +1131,7 @@ impl<'a> DatadirModification<'a> { /// context, breaking the atomicity is OK. If the import is interrupted, the /// whole import fails and the timeline will be deleted anyway. /// (Or to be precise, it will be left behind for debugging purposes and - /// ignored, see https://github.com/neondatabase/neon/pull/1809) + /// ignored, see ) /// /// Note: A consequence of flushing the pending operations is that they /// won't be visible to subsequent operations until `commit`. The function diff --git a/pageserver/src/task_mgr.rs b/pageserver/src/task_mgr.rs index 13db38d956..9c6851bc71 100644 --- a/pageserver/src/task_mgr.rs +++ b/pageserver/src/task_mgr.rs @@ -205,7 +205,7 @@ pub enum TaskKind { /// /// Walreceiver uses its own abstraction called `TaskHandle` to represent the activity of establishing and handling a connection. /// That abstraction doesn't use `task_mgr`. - /// The [`WalReceiverManager`] task ensures that this `TaskHandle` task does not outlive the [`WalReceiverManager`] task. + /// The `WalReceiverManager` task ensures that this `TaskHandle` task does not outlive the `WalReceiverManager` task. /// For the `RequestContext` that we hand to the TaskHandle, we use the [`WalReceiverConnectionHandler`] task kind. /// /// Once the connection is established, the `TaskHandle` task creates a @@ -213,16 +213,21 @@ pub enum TaskKind { /// the `Connection` object. /// A `CancellationToken` created by the `TaskHandle` task ensures /// that the [`WalReceiverConnectionPoller`] task will cancel soon after as the `TaskHandle` is dropped. + /// + /// [`WalReceiverConnectionHandler`]: Self::WalReceiverConnectionHandler + /// [`WalReceiverConnectionPoller`]: Self::WalReceiverConnectionPoller WalReceiverManager, - /// The `TaskHandle` task that executes [`walreceiver_connection::handle_walreceiver_connection`]. + /// The `TaskHandle` task that executes `handle_walreceiver_connection`. /// Not a `task_mgr` task, but we use this `TaskKind` for its `RequestContext`. /// See the comment on [`WalReceiverManager`]. + /// + /// [`WalReceiverManager`]: Self::WalReceiverManager WalReceiverConnectionHandler, /// The task that polls the `tokio-postgres::Connection` object. - /// Spawned by task [`WalReceiverConnectionHandler`]. - /// See the comment on [`WalReceiverManager`]. + /// Spawned by task [`WalReceiverConnectionHandler`](Self::WalReceiverConnectionHandler). + /// See the comment on [`WalReceiverManager`](Self::WalReceiverManager). WalReceiverConnectionPoller, // Garbage collection worker. One per tenant diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs index be88fd8676..142118bf6e 100644 --- a/pageserver/src/tenant.rs +++ b/pageserver/src/tenant.rs @@ -133,7 +133,7 @@ pub use timeline::{ // re-export this function so that page_cache.rs can use it. pub use crate::tenant::ephemeral_file::writeback as writeback_ephemeral_file; -// re-export for use in storage_sync.rs +// re-export for use in remote_timeline_client.rs pub use crate::tenant::metadata::save_metadata; // re-export for use in walreceiver @@ -1459,7 +1459,7 @@ impl Tenant { let layer_removal_guard = timeline.layer_removal_cs.lock().await; info!("got layer_removal_cs.lock(), deleting layer files"); - // NB: storage_sync upload tasks that reference these layers have been cancelled + // NB: remote_timeline_client upload tasks that reference these layers have been cancelled // by the caller. let local_timeline_directory = self @@ -4335,13 +4335,13 @@ mod tests { // assert freeze_and_flush exercised the initdb optimization { let state = tline.flush_loop_state.lock().unwrap(); - let - timeline::FlushLoopState::Running { - expect_initdb_optimization, - initdb_optimization_count, - } = *state else { - panic!("unexpected state: {:?}", *state); - }; + let timeline::FlushLoopState::Running { + expect_initdb_optimization, + initdb_optimization_count, + } = *state + else { + panic!("unexpected state: {:?}", *state); + }; assert!(expect_initdb_optimization); assert!(initdb_optimization_count > 0); } diff --git a/pageserver/src/tenant/disk_btree.rs b/pageserver/src/tenant/disk_btree.rs index 88dff32b76..734409a619 100644 --- a/pageserver/src/tenant/disk_btree.rs +++ b/pageserver/src/tenant/disk_btree.rs @@ -442,7 +442,7 @@ where writer: W, /// - /// stack[0] is the current root page, stack.last() is the leaf. + /// `stack[0]` is the current root page, `stack.last()` is the leaf. /// /// We maintain the length of the stack to be always greater than zero. /// Two exceptions are: diff --git a/pageserver/src/tenant/layer_map.rs b/pageserver/src/tenant/layer_map.rs index 9dd3212a5b..2908d3a83c 100644 --- a/pageserver/src/tenant/layer_map.rs +++ b/pageserver/src/tenant/layer_map.rs @@ -16,7 +16,7 @@ //! Other read methods are less critical but still impact performance of background tasks. //! //! This data structure relies on a persistent/immutable binary search tree. See the -//! following lecture for an introduction https://www.youtube.com/watch?v=WqCWghETNDc&t=581s +//! following lecture for an introduction //! Summary: A persistent/immutable BST (and persistent data structures in general) allows //! you to modify the tree in such a way that each modification creates a new "version" //! of the tree. When you modify it, you get a new version, but all previous versions are @@ -40,7 +40,7 @@ //! afterwards. We can add layers as long as they have larger LSNs than any previous layer in //! the map, but if we need to remove a layer, or insert anything with an older LSN, we need //! to throw away most of the persistent BST and build a new one, starting from the oldest -//! LSN. See `LayerMap::flush_updates()`. +//! LSN. See [`LayerMap::flush_updates()`]. //! mod historic_layer_coverage; diff --git a/pageserver/src/tenant/layer_map/historic_layer_coverage.rs b/pageserver/src/tenant/layer_map/historic_layer_coverage.rs index ff55fb7374..347490c1ba 100644 --- a/pageserver/src/tenant/layer_map/historic_layer_coverage.rs +++ b/pageserver/src/tenant/layer_map/historic_layer_coverage.rs @@ -411,7 +411,7 @@ fn test_persistent_overlapping() { /// still be more critical. /// /// See this for more on persistent and retroactive techniques: -/// https://www.youtube.com/watch?v=WqCWghETNDc&t=581s +/// pub struct BufferedHistoricLayerCoverage { /// A persistent layer map that we rebuild when we need to retroactively update historic_coverage: HistoricLayerCoverage, diff --git a/pageserver/src/tenant/layer_map/layer_coverage.rs b/pageserver/src/tenant/layer_map/layer_coverage.rs index f940f8e3f7..1d9101d3d1 100644 --- a/pageserver/src/tenant/layer_map/layer_coverage.rs +++ b/pageserver/src/tenant/layer_map/layer_coverage.rs @@ -2,7 +2,7 @@ use std::ops::Range; // NOTE the `im` crate has 20x more downloads and also has // persistent/immutable BTree. But it's bugged so rpds is a -// better choice https://github.com/neondatabase/neon/issues/3395 +// better choice use rpds::RedBlackTreeMapSync; /// Data structure that can efficiently: @@ -11,7 +11,7 @@ use rpds::RedBlackTreeMapSync; /// - insert layers in non-decreasing lsn.start order /// /// For a detailed explanation and justification of this approach, see: -/// https://neon.tech/blog/persistent-structures-in-neons-wal-indexing +/// /// /// NOTE The struct is parameterized over Value for easier /// testing, but in practice it's some sort of layer. diff --git a/pageserver/src/tenant/manifest.rs b/pageserver/src/tenant/manifest.rs index 745437dfbd..1d2835114f 100644 --- a/pageserver/src/tenant/manifest.rs +++ b/pageserver/src/tenant/manifest.rs @@ -24,7 +24,7 @@ //! Currently, this is not used in the system. Future refactors will ensure //! the storage state will be recorded in this file, and the system can be //! recovered from this file. This is tracked in -//! https://github.com/neondatabase/neon/issues/4418 +//! use std::io::{self, Read, Write}; diff --git a/pageserver/src/tenant/metadata.rs b/pageserver/src/tenant/metadata.rs index 0ce10c7bc8..d52bb66e76 100644 --- a/pageserver/src/tenant/metadata.rs +++ b/pageserver/src/tenant/metadata.rs @@ -1,10 +1,12 @@ //! Every image of a certain timeline from [`crate::tenant::Tenant`] //! has a metadata that needs to be stored persistently. //! -//! Later, the file gets is used in [`crate::remote_storage::storage_sync`] as a part of +//! Later, the file gets used in [`remote_timeline_client`] as a part of //! external storage import and export operations. //! //! The module contains all structs and related helper methods related to timeline metadata. +//! +//! [`remote_timeline_client`]: super::remote_timeline_client use std::fs::{File, OpenOptions}; use std::io::Write; diff --git a/pageserver/src/tenant/remote_timeline_client.rs b/pageserver/src/tenant/remote_timeline_client.rs index 0f5a585f3f..1355356712 100644 --- a/pageserver/src/tenant/remote_timeline_client.rs +++ b/pageserver/src/tenant/remote_timeline_client.rs @@ -135,7 +135,7 @@ //! - Initiate upload queue with that [`IndexPart`]. //! - Reschedule all lost operations by comparing the local filesystem state //! and remote state as per [`IndexPart`]. This is done in -//! [`Timeline::timeline_init_and_sync`] and [`Timeline::reconcile_with_remote`]. +//! [`Tenant::timeline_init_and_sync`] and [`Timeline::reconcile_with_remote`]. //! //! Note that if we crash during file deletion between the index update //! that removes the file from the list of files, and deleting the remote file, @@ -163,8 +163,8 @@ //! - download their remote [`IndexPart`]s //! - create `Timeline` struct and a `RemoteTimelineClient` //! - initialize the client's upload queue with its `IndexPart` -//! - create [`RemoteLayer`] instances for layers that are referenced by `IndexPart` -//! but not present locally +//! - create [`RemoteLayer`](super::storage_layer::RemoteLayer) instances +//! for layers that are referenced by `IndexPart` but not present locally //! - schedule uploads for layers that are only present locally. //! - if the remote `IndexPart`'s metadata was newer than the metadata in //! the local filesystem, write the remote metadata to the local filesystem @@ -198,6 +198,8 @@ //! in remote storage. //! But note that we don't test any of this right now. //! +//! [`Tenant::timeline_init_and_sync`]: super::Tenant::timeline_init_and_sync +//! [`Timeline::reconcile_with_remote`]: super::Timeline::reconcile_with_remote mod delete; mod download; diff --git a/pageserver/src/tenant/size.rs b/pageserver/src/tenant/size.rs index ffcbdc1f1d..e737d3f59c 100644 --- a/pageserver/src/tenant/size.rs +++ b/pageserver/src/tenant/size.rs @@ -110,11 +110,11 @@ pub struct TimelineInputs { /// /// Tenant size does not consider the latest state, but only the state until next_gc_cutoff, which /// is updated on-demand, during the start of this calculation and separate from the -/// [`Timeline::latest_gc_cutoff`]. +/// [`TimelineInputs::latest_gc_cutoff`]. /// /// For timelines in general: /// -/// ```ignore +/// ```text /// 0-----|---------|----|------------| · · · · · |·> lsn /// initdb_lsn branchpoints* next_gc_cutoff latest /// ``` diff --git a/pageserver/src/tenant/storage_layer.rs b/pageserver/src/tenant/storage_layer.rs index 7996c00215..e1ebe92c61 100644 --- a/pageserver/src/tenant/storage_layer.rs +++ b/pageserver/src/tenant/storage_layer.rs @@ -162,6 +162,9 @@ impl LayerAccessStats { /// The caller is responsible for recording a residence event /// using [`record_residence_event`] before calling `latest_activity`. /// If they don't, [`latest_activity`] will return `None`. + /// + /// [`record_residence_event`]: Self::record_residence_event + /// [`latest_activity`]: Self::latest_activity pub(crate) fn empty_will_record_residence_event_later() -> Self { LayerAccessStats(Mutex::default()) } @@ -169,6 +172,9 @@ impl LayerAccessStats { /// Create an empty stats object and record a [`LayerLoad`] event with the given residence status. /// /// See [`record_residence_event`] for why you need to do this while holding the layer map lock. + /// + /// [`LayerLoad`]: LayerResidenceEventReason::LayerLoad + /// [`record_residence_event`]: Self::record_residence_event pub(crate) fn for_loading_layer( layer_map_lock_held_witness: &LayerManager, status: LayerResidenceStatus, @@ -187,6 +193,8 @@ impl LayerAccessStats { /// The `new_status` is not recorded in `self`. /// /// See [`record_residence_event`] for why you need to do this while holding the layer map lock. + /// + /// [`record_residence_event`]: Self::record_residence_event pub(crate) fn clone_for_residence_change( &self, layer_map_lock_held_witness: &LayerManager, @@ -294,11 +302,13 @@ impl LayerAccessStats { /// implementation error. This function logs a rate-limited warning in that case. /// /// TODO: use type system to avoid the need for `fallback`. - /// The approach in https://github.com/neondatabase/neon/pull/3775 + /// The approach in /// could be used to enforce that a residence event is recorded /// before a layer is added to the layer map. We could also have /// a layer wrapper type that holds the LayerAccessStats, and ensure /// that that type can only be produced by inserting into the layer map. + /// + /// [`record_residence_event`]: Self::record_residence_event pub(crate) fn latest_activity(&self) -> Option { let locked = self.0.lock().unwrap(); let inner = &locked.for_eviction_policy; @@ -323,7 +333,7 @@ impl LayerAccessStats { } /// Supertrait of the [`Layer`] trait that captures the bare minimum interface -/// required by [`LayerMap`]. +/// required by [`LayerMap`](super::layer_map::LayerMap). /// /// All layers should implement a minimal `std::fmt::Debug` without tenant or /// timeline names, because those are known in the context of which the layers @@ -370,10 +380,10 @@ pub trait Layer: std::fmt::Debug + std::fmt::Display + Send + Sync { fn dump(&self, verbose: bool, ctx: &RequestContext) -> Result<()>; } -/// Returned by [`Layer::iter`] +/// Returned by [`PersistentLayer::iter`] pub type LayerIter<'i> = Box> + 'i + Send>; -/// Returned by [`Layer::key_iter`] +/// Returned by [`PersistentLayer::key_iter`] pub type LayerKeyIter<'i> = Box + 'i + Send>; /// Get a layer descriptor from a layer. diff --git a/pageserver/src/tenant/storage_layer/delta_layer.rs b/pageserver/src/tenant/storage_layer/delta_layer.rs index aafab1dd8e..2eab2106a6 100644 --- a/pageserver/src/tenant/storage_layer/delta_layer.rs +++ b/pageserver/src/tenant/storage_layer/delta_layer.rs @@ -7,14 +7,18 @@ //! must be page images or WAL records with the 'will_init' flag set, so that //! they can be replayed without referring to an older page version. //! -//! The delta files are stored in timelines/ directory. Currently, +//! The delta files are stored in `timelines/` directory. Currently, //! there are no subdirectories, and each delta file is named like this: //! +//! ```text //! -__- +//! ``` //! //! For example: //! +//! ```text //! 000000067F000032BE0000400000000020B6-000000067F000032BE0000400000000030B6__000000578C6B29-0000000057A50051 +//! ``` //! //! Every delta file consists of three parts: "summary", "index", and //! "values". The summary is a fixed size header at the beginning of the file, @@ -800,7 +804,7 @@ impl DeltaLayerWriterInner { /// /// # Note /// -/// As described in https://github.com/neondatabase/neon/issues/2650, it's +/// As described in , it's /// possible for the writer to drop before `finish` is actually called. So this /// could lead to odd temporary files in the directory, exhausting file system. /// This structure wraps `DeltaLayerWriterInner` and also contains `Drop` diff --git a/pageserver/src/tenant/storage_layer/filename.rs b/pageserver/src/tenant/storage_layer/filename.rs index 073a0588e8..843bb1f631 100644 --- a/pageserver/src/tenant/storage_layer/filename.rs +++ b/pageserver/src/tenant/storage_layer/filename.rs @@ -57,8 +57,9 @@ impl Ord for DeltaFileName { /// Represents the filename of a DeltaLayer /// +/// ```text /// -__- -/// +/// ``` impl DeltaFileName { /// /// Parse a string as a delta file name. Returns None if the filename does not @@ -162,7 +163,9 @@ impl ImageFileName { /// /// Represents the filename of an ImageLayer /// +/// ```text /// -__ +/// ``` impl ImageFileName { /// /// Parse a string as an image file name. Returns None if the filename does not diff --git a/pageserver/src/tenant/storage_layer/image_layer.rs b/pageserver/src/tenant/storage_layer/image_layer.rs index e46a6d84f6..b8601af818 100644 --- a/pageserver/src/tenant/storage_layer/image_layer.rs +++ b/pageserver/src/tenant/storage_layer/image_layer.rs @@ -7,11 +7,15 @@ //! timelines/ directory. Currently, there are no //! subdirectories, and each image layer file is named like this: //! +//! ```text //! -__ +//! ``` //! //! For example: //! +//! ```text //! 000000067F000032BE0000400000000070B6-000000067F000032BE0000400000000080B6__00000000346BC568 +//! ``` //! //! Every image layer file consists of three parts: "summary", //! "index", and "values". The summary is a fixed size header at the @@ -660,7 +664,7 @@ impl ImageLayerWriterInner { /// /// # Note /// -/// As described in https://github.com/neondatabase/neon/issues/2650, it's +/// As described in , it's /// possible for the writer to drop before `finish` is actually called. So this /// could lead to odd temporary files in the directory, exhausting file system. /// This structure wraps `ImageLayerWriterInner` and also contains `Drop` diff --git a/pageserver/src/tenant/storage_layer/remote_layer.rs b/pageserver/src/tenant/storage_layer/remote_layer.rs index 4ab11a6c3e..d3c40d93bb 100644 --- a/pageserver/src/tenant/storage_layer/remote_layer.rs +++ b/pageserver/src/tenant/storage_layer/remote_layer.rs @@ -25,7 +25,7 @@ use super::{ }; /// RemoteLayer is a not yet downloaded [`ImageLayer`] or -/// [`crate::storage_layer::DeltaLayer`]. +/// [`DeltaLayer`](super::DeltaLayer). /// /// RemoteLayer might be downloaded on-demand during operations which are /// allowed download remote layers and during which, it gets replaced with a @@ -50,6 +50,8 @@ pub struct RemoteLayer { /// It is very unlikely to accumulate these in the Timeline's LayerMap, but having this avoids /// a possible fast loop between `Timeline::get_reconstruct_data` and /// `Timeline::download_remote_layer`, which also logs. + /// + /// [`ongoing_download`]: Self::ongoing_download pub(crate) download_replacement_failure: std::sync::atomic::AtomicBool, } diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index 3db1347c37..e568f459d7 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -183,7 +183,7 @@ pub struct Timeline { walredo_mgr: Arc, /// Remote storage client. - /// See [`storage_sync`] module comment for details. + /// See [`remote_timeline_client`](super::remote_timeline_client) module comment for details. pub remote_client: Option>, // What page versions do we hold in the repository? If we get a @@ -240,6 +240,8 @@ pub struct Timeline { /// This lock is acquired in [`Timeline::gc`], [`Timeline::compact`], /// and [`Tenant::delete_timeline`]. This is an `Arc` lock because we need an owned /// lock guard in functions that will be spawned to tokio I/O pool (which requires `'static`). + /// + /// [`Tenant::delete_timeline`]: super::Tenant::delete_timeline pub(super) layer_removal_cs: Arc>, // Needed to ensure that we can't create a branch at a point that was already garbage collected @@ -979,8 +981,12 @@ impl Timeline { #[instrument(skip_all, fields(tenant_id = %self.tenant_id, timeline_id = %self.timeline_id))] pub async fn download_layer(&self, layer_file_name: &str) -> anyhow::Result> { - let Some(layer) = self.find_layer(layer_file_name).await else { return Ok(None) }; - let Some(remote_layer) = layer.downcast_remote_layer() else { return Ok(Some(false)) }; + let Some(layer) = self.find_layer(layer_file_name).await else { + return Ok(None); + }; + let Some(remote_layer) = layer.downcast_remote_layer() else { + return Ok(Some(false)); + }; if self.remote_client.is_none() { return Ok(Some(false)); } @@ -989,10 +995,12 @@ impl Timeline { Ok(Some(true)) } - /// Like [`evict_layer_batch`], but for just one layer. + /// Like [`evict_layer_batch`](Self::evict_layer_batch), but for just one layer. /// Additional case `Ok(None)` covers the case where the layer could not be found by its `layer_file_name`. pub async fn evict_layer(&self, layer_file_name: &str) -> anyhow::Result> { - let Some(local_layer) = self.find_layer(layer_file_name).await else { return Ok(None) }; + let Some(local_layer) = self.find_layer(layer_file_name).await else { + return Ok(None); + }; let remote_client = self .remote_client .as_ref() @@ -1013,9 +1021,9 @@ impl Timeline { /// Evict a batch of layers. /// - /// GenericRemoteStorage reference is required as a witness[^witness_article] for "remote storage is configured." + /// GenericRemoteStorage reference is required as a (witness)[witness_article] for "remote storage is configured." /// - /// [^witness_article]: https://willcrichton.net/rust-api-type-patterns/witnesses.html + /// [witness_article]: https://willcrichton.net/rust-api-type-patterns/witnesses.html pub async fn evict_layers( &self, _: &GenericRemoteStorage, @@ -1769,7 +1777,7 @@ impl Timeline { /// 3. Schedule upload of local-only layer files (which will then also update the remote /// IndexPart to include the new layer files). /// - /// Refer to the `storage_sync` module comment for more context. + /// Refer to the [`remote_timeline_client`] module comment for more context. /// /// # TODO /// May be a bit cleaner to do things based on populated remote client, @@ -2602,7 +2610,9 @@ impl Timeline { guard.layer_map().frozen_layers.front().cloned() // drop 'layers' lock to allow concurrent reads and writes }; - let Some(layer_to_flush) = layer_to_flush else { break Ok(()) }; + let Some(layer_to_flush) = layer_to_flush else { + break Ok(()); + }; if let Err(err) = self.flush_frozen_layer(layer_to_flush, ctx).await { error!("could not flush frozen layer: {err:?}"); break Err(err); @@ -3273,6 +3283,8 @@ impl Timeline { /// This method takes the `_layer_removal_cs` guard to highlight it required downloads are /// returned as an error. If the `layer_removal_cs` boundary is changed not to be taken in the /// start of level0 files compaction, the on-demand download should be revisited as well. + /// + /// [`compact_inner`]: Self::compact_inner fn compact_level0_phase1( self: Arc, _layer_removal_cs: Arc>, @@ -3692,7 +3704,7 @@ impl Timeline { } // Before deleting any layers, we need to wait for their upload ops to finish. - // See storage_sync module level comment on consistency. + // See remote_timeline_client module level comment on consistency. // Do it here because we don't want to hold self.layers.write() while waiting. if let Some(remote_client) = &self.remote_client { debug!("waiting for upload ops to complete"); diff --git a/pageserver/src/tenant/timeline/walreceiver/connection_manager.rs b/pageserver/src/tenant/timeline/walreceiver/connection_manager.rs index 5c03c6106f..57c09a4487 100644 --- a/pageserver/src/tenant/timeline/walreceiver/connection_manager.rs +++ b/pageserver/src/tenant/timeline/walreceiver/connection_manager.rs @@ -6,7 +6,7 @@ //! Current connection state is tracked too, to ensure it's not getting stale. //! //! After every connection or storage broker update fetched, the state gets updated correspondingly and rechecked for the new conneciton leader, -//! then a [re]connection happens, if necessary. +//! then a (re)connection happens, if necessary. //! Only WAL streaming task expects to be finished, other loops (storage broker, connection management) never exit unless cancelled explicitly via the dedicated channel. use std::{collections::HashMap, num::NonZeroU64, ops::ControlFlow, sync::Arc, time::Duration}; diff --git a/proxy/src/cancellation.rs b/proxy/src/cancellation.rs index c8c0727471..8d16f202e9 100644 --- a/proxy/src/cancellation.rs +++ b/proxy/src/cancellation.rs @@ -110,7 +110,7 @@ impl<'a> Session<'a> { impl Session<'_> { /// Store the cancel token for the given session. - /// This enables query cancellation in [`crate::proxy::handshake`]. + /// This enables query cancellation in `crate::proxy::prepare_client_connection`. pub fn enable_query_cancellation(self, cancel_closure: CancelClosure) -> CancelKeyData { info!("enabling query cancellation for this session"); self.cancel_map diff --git a/proxy/src/compute.rs b/proxy/src/compute.rs index 70b29679b9..ccf100397b 100644 --- a/proxy/src/compute.rs +++ b/proxy/src/compute.rs @@ -13,7 +13,7 @@ const COULD_NOT_CONNECT: &str = "Couldn't connect to compute node"; #[derive(Debug, Error)] pub enum ConnectionError { /// This error doesn't seem to reveal any secrets; for instance, - /// [`tokio_postgres::error::Kind`] doesn't contain ip addresses and such. + /// `tokio_postgres::error::Kind` doesn't contain ip addresses and such. #[error("{COULD_NOT_CONNECT}: {0}")] Postgres(#[from] tokio_postgres::Error), diff --git a/proxy/src/config.rs b/proxy/src/config.rs index 00f561fcf2..989027f03f 100644 --- a/proxy/src/config.rs +++ b/proxy/src/config.rs @@ -211,7 +211,7 @@ pub struct CacheOptions { } impl CacheOptions { - /// Default options for [`crate::auth::caches::NodeInfoCache`]. + /// Default options for [`crate::console::provider::NodeInfoCache`]. pub const DEFAULT_OPTIONS_NODE_INFO: &str = "size=4000,ttl=4m"; /// Parse cache options passed via cmdline. diff --git a/proxy/src/console/provider.rs b/proxy/src/console/provider.rs index 44e23e0adf..77b4330e44 100644 --- a/proxy/src/console/provider.rs +++ b/proxy/src/console/provider.rs @@ -197,7 +197,7 @@ pub trait Api { ) -> Result; } -/// Various caches for [`console`]. +/// Various caches for [`console`](super). pub struct ApiCaches { /// Cache for the `wake_compute` API method. pub node_info: NodeInfoCache, diff --git a/proxy/src/scram.rs b/proxy/src/scram.rs index 85854427ed..07822e8da5 100644 --- a/proxy/src/scram.rs +++ b/proxy/src/scram.rs @@ -12,7 +12,7 @@ mod messages; mod secret; mod signature; -#[cfg(test)] +#[cfg(any(test, doc))] mod password; pub use exchange::Exchange; diff --git a/safekeeper/src/control_file.rs b/safekeeper/src/control_file.rs index 653e938bb7..504c2d355d 100644 --- a/safekeeper/src/control_file.rs +++ b/safekeeper/src/control_file.rs @@ -163,8 +163,9 @@ impl Deref for FileStorage { #[async_trait::async_trait] impl Storage for FileStorage { - /// persists state durably to underlying storage - /// for description see https://lwn.net/Articles/457667/ + /// Persists state durably to the underlying storage. + /// + /// For a description, see . async fn persist(&mut self, s: &SafeKeeperState) -> Result<()> { let _timer = PERSIST_CONTROL_FILE_SECONDS.start_timer(); diff --git a/safekeeper/src/timelines_global_map.rs b/safekeeper/src/timelines_global_map.rs index f2d5df8744..eda5b9044e 100644 --- a/safekeeper/src/timelines_global_map.rs +++ b/safekeeper/src/timelines_global_map.rs @@ -1,4 +1,4 @@ -//! This module contains global (tenant_id, timeline_id) -> Arc mapping. +//! This module contains global `(tenant_id, timeline_id)` -> `Arc` mapping. //! All timelines should always be present in this map, this is done by loading them //! all from the disk on startup and keeping them in memory. diff --git a/safekeeper/src/wal_storage.rs b/safekeeper/src/wal_storage.rs index 61270a8d0b..d728312de4 100644 --- a/safekeeper/src/wal_storage.rs +++ b/safekeeper/src/wal_storage.rs @@ -106,13 +106,15 @@ pub struct PhysicalStorage { /// Imagine the following: /// - 000000010000000000000001 /// - it was fully written, but the last record is split between 2 segments - /// - after restart, find_end_of_wal() returned 0/1FFFFF0, which is in the end of this segment - /// - write_lsn, write_record_lsn and flush_record_lsn were initialized to 0/1FFFFF0 + /// - after restart, `find_end_of_wal()` returned 0/1FFFFF0, which is in the end of this segment + /// - `write_lsn`, `write_record_lsn` and `flush_record_lsn` were initialized to 0/1FFFFF0 /// - 000000010000000000000002.partial /// - it has only 1 byte written, which is not enough to make a full WAL record /// /// Partial segment 002 has no WAL records, and it will be removed by the next truncate_wal(). /// This flag will be set to true after the first truncate_wal() call. + /// + /// [`write_lsn`]: Self::write_lsn is_truncated_after_restart: bool, } diff --git a/test_runner/regress/test_tenant_detach.py b/test_runner/regress/test_tenant_detach.py index 2ded79954e..7d432def34 100644 --- a/test_runner/regress/test_tenant_detach.py +++ b/test_runner/regress/test_tenant_detach.py @@ -791,7 +791,7 @@ def test_ignore_while_attaching( pageserver_http.tenant_attach(tenant_id) # Run ignore on the task, thereby cancelling the attach. # XXX This should take priority over attach, i.e., it should cancel the attach task. - # But neither the failpoint, nor the proper storage_sync download functions, + # But neither the failpoint, nor the proper remote_timeline_client download functions, # are sensitive to task_mgr::shutdown. # This problem is tracked in https://github.com/neondatabase/neon/issues/2996 . # So, for now, effectively, this ignore here will block until attach task completes.