Skip to content

Commit

Permalink
Extract safekeeper http client to separate crate. (#10140)
Browse files Browse the repository at this point in the history
## Problem

We want to use safekeeper http client in storage controller and
neon_local.

## Summary of changes

Extract it to separate crate. No functional changes.
  • Loading branch information
arssher authored Dec 16, 2024
1 parent 24d6587 commit 1ed0e52
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 7 deletions.
13 changes: 13 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ members = [
"pageserver/pagebench",
"proxy",
"safekeeper",
"safekeeper/client",
"storage_broker",
"storage_controller",
"storage_controller/client",
Expand Down Expand Up @@ -233,6 +234,7 @@ postgres_initdb = { path = "./libs/postgres_initdb" }
pq_proto = { version = "0.1", path = "./libs/pq_proto/" }
remote_storage = { version = "0.1", path = "./libs/remote_storage/" }
safekeeper_api = { version = "0.1", path = "./libs/safekeeper_api" }
safekeeper_client = { path = "./safekeeper/client" }
desim = { version = "0.1", path = "./libs/desim" }
storage_broker = { version = "0.1", path = "./storage_broker/" } # Note: main broker code is inside the binary crate, so linking with the library shouldn't be heavy.
storage_controller_client = { path = "./storage_controller/client" }
Expand Down
1 change: 1 addition & 0 deletions safekeeper/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ postgres_ffi.workspace = true
pq_proto.workspace = true
remote_storage.workspace = true
safekeeper_api.workspace = true
safekeeper_client.workspace = true
sha2.workspace = true
sd-notify.workspace = true
storage_broker.workspace = true
Expand Down
13 changes: 13 additions & 0 deletions safekeeper/client/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[package]
name = "safekeeper_client"
version = "0.1.0"
edition.workspace = true
license.workspace = true

[dependencies]
safekeeper_api.workspace = true
thiserror.workspace = true
reqwest = { workspace = true, features = [ "stream" ] }
serde.workspace = true
utils.workspace = true
workspace_hack = { version = "0.1", path = "../../workspace_hack" }
1 change: 1 addition & 0 deletions safekeeper/client/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub mod mgmt_api;
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,6 @@
//!
//! Partially copied from pageserver client; some parts might be better to be
//! united.
//!
//! It would be also good to move it out to separate crate, but this needs
//! duplication of internal-but-reported structs like WalSenderState, ServerInfo
//! etc.
use reqwest::{IntoUrl, Method, StatusCode};
use safekeeper_api::models::TimelineStatus;
Expand Down
1 change: 0 additions & 1 deletion safekeeper/src/http/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
pub mod client;
pub mod routes;
pub use routes::make_router;

Expand Down
5 changes: 3 additions & 2 deletions safekeeper/src/pull_timeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use chrono::{DateTime, Utc};
use futures::{SinkExt, StreamExt, TryStreamExt};
use postgres_ffi::{XLogFileName, XLogSegNo, PG_TLI};
use safekeeper_api::{models::TimelineStatus, Term};
use safekeeper_client::mgmt_api;
use safekeeper_client::mgmt_api::Client;
use serde::{Deserialize, Serialize};
use std::{
cmp::min,
Expand All @@ -22,7 +24,6 @@ use tracing::{error, info, instrument};
use crate::{
control_file::CONTROL_FILE_NAME,
debug_dump,
http::client::{self, Client},
state::{EvictionState, TimelinePersistentState},
timeline::{Timeline, WalResidentTimeline},
timelines_global_map::{create_temp_timeline_dir, validate_temp_timeline},
Expand Down Expand Up @@ -419,7 +420,7 @@ pub async fn handle_request(
let http_hosts = request.http_hosts.clone();

// Figure out statuses of potential donors.
let responses: Vec<Result<TimelineStatus, client::Error>> =
let responses: Vec<Result<TimelineStatus, mgmt_api::Error>> =
futures::future::join_all(http_hosts.iter().map(|url| async {
let cclient = Client::new(url.clone(), sk_auth_token.clone());
let info = cclient
Expand Down

1 comment on commit 1ed0e52

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7084 tests run: 6786 passed, 1 failed, 297 skipped (full report)


Failures on Postgres 17

# Run all failed tests locally:
scripts/pytest -vv -n $(nproc) -k "test_timeline_archival_chaos[release-pg17]"
Flaky tests (3)

Postgres 17

Test coverage report is not available

The comment gets automatically updated with the latest test results
1ed0e52 at 2024-12-16T13:14:33.405Z :recycle:

Please sign in to comment.