Skip to content

Commit 3e86008

Browse files
authored
read-only timelines (neondatabase#12015)
Support timeline creations on the storage controller to opt out from their creation on the safekeepers, introducing the read-only timelines concept. Read only timelines: * will never receive WAL of their own, so it's fine to not create them on the safekeepers * the property is non-transitive. children of read-only timelines aren't neccessarily read-only themselves. This feature can be used for snapshots, to prevent the safekeepers from being overloaded by empty timelines that won't ever get written to. In the current world, this is not a problem, because timelines are created implicitly by the compute connecting to a safekeeper that doesn't have the timeline yet. In the future however, where the storage controller creates timelines eagerly, we should watch out for that. We represent read-only timelines in the storage controller database so that we ensure that they never touch the safekeepers at all. Especially we don't want them to cause a mess during the importing process of the timelines from the cplane to the storcon database. In a hypothetical future where we have a feature to detach timelines from safekeepers, we'll either need to find a way to distinguish the two, or if not, asking safekeepers to list the (empty) timeline prefix and delete everything from it isn't a big issue either. This patch will unconditionally hit the new safekeeper timeline creation path for read-only timelines, without them needing the `--timelines-onto-safekeepers` flag enabled. This is done because it's lower risk (no safekeepers or computes involved at all) and gives us some initial way to verify at least some parts of that code in prod. neondatabase/neon-archive-cloud#29435 neondatabase#11670
1 parent 23fc611 commit 3e86008

File tree

7 files changed

+67
-12
lines changed

7 files changed

+67
-12
lines changed

control_plane/src/bin/neon_local.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1279,6 +1279,7 @@ async fn handle_timeline(cmd: &TimelineCmd, env: &mut local_env::LocalEnv) -> Re
12791279
mode: pageserver_api::models::TimelineCreateRequestMode::Branch {
12801280
ancestor_timeline_id,
12811281
ancestor_start_lsn: start_lsn,
1282+
read_only: false,
12821283
pg_version: None,
12831284
},
12841285
};

libs/pageserver_api/src/models.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,8 @@ pub enum TimelineCreateRequestMode {
402402
// using a flattened enum, so, it was an accepted field, and
403403
// we continue to accept it by having it here.
404404
pg_version: Option<u32>,
405+
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
406+
read_only: bool,
405407
},
406408
ImportPgdata {
407409
import_pgdata: TimelineCreateRequestModeImportPgdata,

pageserver/src/http/openapi_spec.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,8 @@ paths:
626626
format: hex
627627
pg_version:
628628
type: integer
629+
read_only:
630+
type: boolean
629631
existing_initdb_timeline_id:
630632
type: string
631633
format: hex

pageserver/src/http/routes.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -572,6 +572,7 @@ async fn timeline_create_handler(
572572
TimelineCreateRequestMode::Branch {
573573
ancestor_timeline_id,
574574
ancestor_start_lsn,
575+
read_only: _,
575576
pg_version: _,
576577
} => tenant::CreateTimelineParams::Branch(tenant::CreateTimelineParamsBranch {
577578
new_timeline_id,

storage_controller/src/service.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3823,6 +3823,13 @@ impl Service {
38233823
.await;
38243824
failpoint_support::sleep_millis_async!("tenant-create-timeline-shared-lock");
38253825
let is_import = create_req.is_import();
3826+
let read_only = matches!(
3827+
create_req.mode,
3828+
models::TimelineCreateRequestMode::Branch {
3829+
read_only: true,
3830+
..
3831+
}
3832+
);
38263833

38273834
if is_import {
38283835
// Ensure that there is no split on-going.
@@ -3895,13 +3902,13 @@ impl Service {
38953902
}
38963903

38973904
None
3898-
} else if safekeepers {
3905+
} else if safekeepers || read_only {
38993906
// Note that for imported timelines, we do not create the timeline on the safekeepers
39003907
// straight away. Instead, we do it once the import finalized such that we know what
39013908
// start LSN to provide for the safekeepers. This is done in
39023909
// [`Self::finalize_timeline_import`].
39033910
let res = self
3904-
.tenant_timeline_create_safekeepers(tenant_id, &timeline_info)
3911+
.tenant_timeline_create_safekeepers(tenant_id, &timeline_info, read_only)
39053912
.instrument(tracing::info_span!("timeline_create_safekeepers", %tenant_id, timeline_id=%timeline_info.timeline_id))
39063913
.await?;
39073914
Some(res)

storage_controller/src/service/safekeeper_service.rs

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,7 @@ impl Service {
208208
self: &Arc<Self>,
209209
tenant_id: TenantId,
210210
timeline_info: &TimelineInfo,
211+
read_only: bool,
211212
) -> Result<SafekeepersInfo, ApiError> {
212213
let timeline_id = timeline_info.timeline_id;
213214
let pg_version = timeline_info.pg_version * 10000;
@@ -220,7 +221,11 @@ impl Service {
220221
let start_lsn = timeline_info.last_record_lsn;
221222

222223
// Choose initial set of safekeepers respecting affinity
223-
let sks = self.safekeepers_for_new_timeline().await?;
224+
let sks = if !read_only {
225+
self.safekeepers_for_new_timeline().await?
226+
} else {
227+
Vec::new()
228+
};
224229
let sks_persistence = sks.iter().map(|sk| sk.id.0 as i64).collect::<Vec<_>>();
225230
// Add timeline to db
226231
let mut timeline_persist = TimelinePersistence {
@@ -253,6 +258,16 @@ impl Service {
253258
)));
254259
}
255260
}
261+
let ret = SafekeepersInfo {
262+
generation: timeline_persist.generation as u32,
263+
safekeepers: sks.clone(),
264+
tenant_id,
265+
timeline_id,
266+
};
267+
if read_only {
268+
return Ok(ret);
269+
}
270+
256271
// Create the timeline on a quorum of safekeepers
257272
let remaining = self
258273
.tenant_timeline_create_safekeepers_quorum(
@@ -316,12 +331,7 @@ impl Service {
316331
}
317332
}
318333

319-
Ok(SafekeepersInfo {
320-
generation: timeline_persist.generation as u32,
321-
safekeepers: sks,
322-
tenant_id,
323-
timeline_id,
324-
})
334+
Ok(ret)
325335
}
326336

327337
pub(crate) async fn tenant_timeline_create_safekeepers_until_success(
@@ -336,8 +346,10 @@ impl Service {
336346
return Err(TimelineImportFinalizeError::ShuttingDown);
337347
}
338348

349+
// This function is only used in non-read-only scenarios
350+
let read_only = false;
339351
let res = self
340-
.tenant_timeline_create_safekeepers(tenant_id, &timeline_info)
352+
.tenant_timeline_create_safekeepers(tenant_id, &timeline_info, read_only)
341353
.await;
342354

343355
match res {
@@ -410,6 +422,18 @@ impl Service {
410422
.chain(tl.sk_set.iter())
411423
.collect::<HashSet<_>>();
412424

425+
// The timeline has no safekeepers: we need to delete it from the db manually,
426+
// as no safekeeper reconciler will get to it
427+
if all_sks.is_empty() {
428+
if let Err(err) = self
429+
.persistence
430+
.delete_timeline(tenant_id, timeline_id)
431+
.await
432+
{
433+
tracing::warn!(%tenant_id, %timeline_id, "couldn't delete timeline from db: {err}");
434+
}
435+
}
436+
413437
// Schedule reconciliations
414438
for &sk_id in all_sks.iter() {
415439
let pending_op = TimelinePendingOpPersistence {

test_runner/regress/test_timeline_detach_ancestor.py

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from threading import Barrier
1111

1212
import pytest
13+
import requests
1314
from fixtures.common_types import Lsn, TimelineArchivalState, TimelineId
1415
from fixtures.log_helper import log
1516
from fixtures.neon_fixtures import (
@@ -401,8 +402,25 @@ def test_ancestor_detach_behavior_v2(neon_env_builder: NeonEnvBuilder, snapshots
401402
"earlier", ancestor_branch_name="main", ancestor_start_lsn=branchpoint_pipe
402403
)
403404

404-
snapshot_branchpoint_old = env.create_branch(
405-
"snapshot_branchpoint_old", ancestor_branch_name="main", ancestor_start_lsn=branchpoint_y
405+
snapshot_branchpoint_old = TimelineId.generate()
406+
407+
env.storage_controller.timeline_create(
408+
env.initial_tenant,
409+
{
410+
"new_timeline_id": str(snapshot_branchpoint_old),
411+
"ancestor_start_lsn": str(branchpoint_y),
412+
"ancestor_timeline_id": str(env.initial_timeline),
413+
"read_only": True,
414+
},
415+
)
416+
sk = env.safekeepers[0]
417+
assert sk
418+
with pytest.raises(requests.exceptions.HTTPError, match="Not Found"):
419+
sk.http_client().timeline_status(
420+
tenant_id=env.initial_tenant, timeline_id=snapshot_branchpoint_old
421+
)
422+
env.neon_cli.mappings_map_branch(
423+
"snapshot_branchpoint_old", env.initial_tenant, snapshot_branchpoint_old
406424
)
407425

408426
snapshot_branchpoint = env.create_branch(

0 commit comments

Comments
 (0)