Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions src/replica.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,10 +510,7 @@ impl Tailer {
// behind.
for (lane_name, lane) in &manifest.lanes {
let (context, lane_label) = ship::lane_metric_labels(lane_name);
let stem = lane_name
.strip_suffix(".passages.wal.jsonl")
.or_else(|| lane_name.strip_suffix(".wal.jsonl"))
.unwrap_or(lane_name);
let stem = ship::lane_stem(lane_name);
if failed.contains(stem) {
self.state
.metrics()
Expand Down
2 changes: 1 addition & 1 deletion src/ship.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ use naming::{
pub(crate) use naming::{
HEARTBEAT_MARKER, Manifest, ManifestFile, ManifestLane, REPLICATION_RECORD, RETIRED_MARKER,
ReplicationRecord, TAKEOVER_GRACE, complete_key, fence_key, gen_root, lane_metric_labels,
read_replication_record, segment_name, write_replication_record,
lane_stem, read_replication_record, segment_name, write_replication_record,
};
pub(crate) use progress::{FileSig, ShipProgress};
use progress::{LaneState, ShippedFile};
Expand Down
33 changes: 32 additions & 1 deletion src/ship/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,14 @@ pub(crate) fn open_store(url: &str) -> io::Result<(Arc<dyn ObjectStore>, StorePa
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
format!(
"{url}: unsupported replication scheme — use s3://, gs://, az://, or file://"
"{url}: unsupported replication scheme — use s3://, gs://, az://, or \
file:// (also accepted: s3a://, adl://, azure://, abfs://, abfss://, \
and — per object_store's own host-based detection — https:// URLs \
shaped like a cloud provider's native endpoint, e.g. \
https://{{account}}.blob.core.windows.net/... for Azure or \
https://s3.{{region}}.amazonaws.com/... for S3; object_store also \
recognizes memory://, plain http://, and other https:// hosts, but \
taguru does not support shipping to them)"
),
));
}
Expand All @@ -172,6 +179,30 @@ mod tests {
assert_eq!(error.kind(), io::ErrorKind::InvalidInput, "{error}");
}

/// `object_store` itself recognizes `memory://` and generic
/// `http(s)://` hosts (`ObjectStoreScheme::Memory`/`Http`) — these
/// are not merely unrecognized like `ftp://`, they are schemes
/// `open_store`'s `match` deliberately falls through on. The
/// rejection must still be `InvalidInput`, and the message must
/// name both that they exist and that they are unsupported here,
/// not just recite the three cloud schemes as if these were never
/// considered.
#[test]
fn a_recognized_but_unsupported_scheme_names_itself_in_the_message() {
for url in [
"memory:///",
"http://example.com/path",
"https://example.com/path",
] {
let error = open_store(url).unwrap_err();
assert_eq!(error.kind(), io::ErrorKind::InvalidInput, "{error}");
let message = error.to_string();
assert!(message.contains(url), "{message}");
assert!(message.contains("memory://"), "{message}");
assert!(message.contains("http://"), "{message}");
}
}

/// A syntactically fine `file://` URL naming a directory that does
/// not exist is also a usage mistake, not the store refusing to
/// open: `NotFound`.
Expand Down
42 changes: 31 additions & 11 deletions src/ship/naming.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ pub(crate) const TAKEOVER_GRACE: Duration = Duration::from_secs(300);
/// it hydrated from (the cache-mode marker `crate::hydrate` keys on).
/// Never shipped (see [`classify`]): it describes the local replica of
/// the relationship, not the data.
///
/// Both `crate::hydrate`'s boot-time restore and the replica tailer
/// (`replica::Tailer::poll_once`, after a successful generation
/// switch) write this. The `taguru restore` CLI (`restore::run`)
/// never does — it materializes
/// an independent directory with no promise of ever booting against
/// this bucket again, so there is no cache relationship to record
/// (see `restore::run`'s own doc for the full reasoning).
Comment thread
coderabbitai[bot] marked this conversation as resolved.
pub(crate) const REPLICATION_RECORD: &str = ".taguru.replication";

/// What [`REPLICATION_RECORD`] holds.
Expand Down Expand Up @@ -218,21 +226,33 @@ pub(super) fn parent_snapshot_of(lane_name: &str) -> Option<String> {
.map(|stem| format!("{stem}.ctx"))
}

/// The stem a lane's file name derives from — the passage suffix
/// checked first because both lanes end in `.wal.jsonl`, same as
/// [`parent_snapshot_of`]. Shared by [`lane_metric_labels`] (which
/// decodes the stem further into a display name) and the replica
/// tailer (which needs the raw stem itself, to key its per-poll
/// `failed` set against `ensure_context`'s own stem-keyed errors), so
/// the two can never drift on which suffixes count (issue #619).
pub(crate) fn lane_stem(lane_name: &str) -> &str {
lane_name
.strip_suffix(".passages.wal.jsonl")
.or_else(|| lane_name.strip_suffix(".wal.jsonl"))
.unwrap_or(lane_name)
}

/// The per-lane label pair the lag metric carries: the context's
/// decoded name where the stem decodes (it always should — these files
/// were written by the server), plus which lane. The replica's lag
/// rows reuse it so the two vocabularies cannot drift.
pub(crate) fn lane_metric_labels(lane_name: &str) -> (String, &'static str) {
if let Some(stem) = lane_name.strip_suffix(".passages.wal.jsonl") {
(
crate::registry::name_from_stem(stem).unwrap_or_else(|| stem.to_string()),
"passages",
)
let kind = if lane_name.ends_with(".passages.wal.jsonl") {
"passages"
} else {
let stem = lane_name.strip_suffix(".wal.jsonl").unwrap_or(lane_name);
(
crate::registry::name_from_stem(stem).unwrap_or_else(|| stem.to_string()),
"graph",
)
}
"graph"
};
let stem = lane_stem(lane_name);
(
crate::registry::name_from_stem(stem).unwrap_or_else(|| stem.to_string()),
kind,
)
}
13 changes: 13 additions & 0 deletions src/ship/restore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,19 @@ mix two histories. Verify the result with: taguru inspect DIR
/// — bad/missing credentials, a rejected cloud config, an
/// inaccessible local path) · 2 usage error (a malformed URL, an
/// unrecognized scheme, or a bad flag).
///
/// Unlike `crate::hydrate`'s boot-time restore, this never writes
/// [`ReplicationRecord`] into `out`: `taguru restore` hands back an
/// independent directory the caller owns, with no promise the writer
/// will ever point at this bucket again, so there is no lineage for a
/// record to describe. `hydrate` writes one because it materializes
/// the SAME data directory `serve` is about to run against — the
/// record is how a later boot tells "this disk is a cache of the
/// bucket" from "this disk is independent truth" (see
/// [`ReplicationRecord`]'s own doc). A directory this CLI restores
/// stays plain local truth from the moment it lands; if the caller
/// wants it to behave as a cache instead, that is a `serve --replica`
/// boot against it, not this command.
pub(crate) fn run(args: &[String]) -> i32 {
let usage = |message: &str| crate::config::subcommand_usage_error("restore", message);
let mut out: Option<PathBuf> = None;
Expand Down