From 7ed72e60edbb2f9eff3c8d7c4935459265ee74b8 Mon Sep 17 00:00:00 2001 From: Maxwell Elliott Date: Tue, 3 Nov 2026 18:28:19 -0500 Subject: [PATCH] Add `serve-multi`: one query service for several repositories `bazel-diff serve-multi --repo NAME=PATH ...` serves several git clones from one process. Each repository gets its own workspace lock, git/Bazel binding, readiness flag and counters, so queries on different repositories run concurrently; the memory, disk and remote cache tiers are shared, since cache keys are tree + configuration fingerprint and identical trees in two clones share an entry on purpose. Requests select a repository with `repo=` (query string on GET/POST; a POST body `repo` wins). With one repository the parameter is optional; with several, a missing `repo` is a 400 and an unknown one a 404, both listing the served names. `/health` reports all repositories ready; `/metrics` keeps its top-level fields as aggregates and adds a `repositories` map. Startup fetch and warmup run per repository in parallel. `serve` is unchanged (its one repository is named `default`). `serve-multi` flattens `ServeArgs` so the two commands share every flag; `--workspacePath` is hidden and rejected there, which is why `HashingArgs.workspace_path` became `Option` with `required = true`. Adds CLI parsing tests, a routing unit test over two real git repos, the e2e case `core::serve_multiple_repositories`, and README docs (including the `serve-multi` CLI help block). Co-Authored-By: Claude Fable 5.1 --- README.md | 152 +++++++++- src/main.rs | 219 ++++++++++++- src/server.rs | 573 ++++++++++++++++++++++++++++++++--- tests/e2e/core.rs | 66 ++++ tests/e2e/support/mod.rs | 41 ++- tools/BUILD | 2 + tools/e2e/rust_e2e_cases.bzl | 4 + tools/generate_readme.py | 16 +- tools/readme_template.md | 46 ++- 9 files changed, 1045 insertions(+), 74 deletions(-) diff --git a/README.md b/README.md index fd63da89..5731e1f6 100644 --- a/README.md +++ b/README.md @@ -282,7 +282,9 @@ Endpoints: balancer should use this to route only to ready instances. If a fatal git error occurs at startup the instance "lame-ducks" itself by continuing to report `503` so the load balancer removes it. * `GET /impacted_targets?from=&to=` — returns the impacted targets as JSON. The optional - `targetType` parameter (e.g. `&targetType=Rule,SourceFile`) filters by target type. + `targetType` parameter (e.g. `&targetType=Rule,SourceFile`) filters by target type. On a server + that serves several repositories, `repo=` names the one to query (see [Serving several + repositories](#serving-several-repositories)). ```bash curl 'http://localhost:8080/impacted_targets?from=main&to=my-feature-branch' @@ -362,7 +364,9 @@ curl 'http://localhost:8080/dependency_edges?from=main&to=my-feature-branch' the queries accepted and not yet finished (queued, running, or abandoned by a client that stopped waiting), and `workspaceBusy` is `true` while any operation holds `--workspacePath` (a query's checkout and `bazel query`, or a startup warmup). A tool that shares the checkout with the server - should wait for `workspaceBusy` to be `false` before touching it. + should wait for `workspaceBusy` to be `false` before touching it. `repositories` breaks the + per-workspace figures down by repository (see [Serving several + repositories](#serving-several-repositories)). ```bash curl 'http://localhost:8080/metrics' @@ -377,11 +381,49 @@ curl 'http://localhost:8080/metrics' "workspaceBusy": true, "gitEngine": "subprocess", "trackDeps": false, + "repositories": {"default": {"workspace": "/path/to/workspace-clone", "ready": true, "activeQueries": 1, "workspaceBusy": true, "partialGraph": {"derived": 0, "fullGenerations": 0}, "untrackedPackages": {"rejections": 0}}}, "cache": {"directory": "/var/cache/bazel-diff", "remote": "s3://my-bucket/bazel-diff/", "entries": 128, "sizeBytes": 4823913, "sizeHuman": "4.6 MB", "memory": {"entries": 2, "sizeBytes": 75374, "maxBytes": 536870912}}, "jvm": {"usedBytes": 123456789, "maxBytes": 2147483648} } ``` +### Serving several repositories + +One instance can answer for several git clones. `bazel-diff serve-multi` takes every `serve` +flag except `--workspacePath`, which `--repo NAME=PATH` (repeatable) replaces, and a request +addresses one repository with `repo=NAME`: a query-string parameter on `GET` and `POST`, or a +`repo` field in the `POST` body, which wins over the query string. + +```bash +bazel-diff serve-multi \ + --repo app=/clones/app \ + --repo lib=/clones/lib \ + --cacheDir /var/cache/bazel-diff \ + --port 8080 + +curl 'http://localhost:8080/impacted_targets?repo=lib&from=main&to=my-feature-branch' +``` + +* Every repository shares the hashing flags (`--bazelPath`, `--bazelStartupOptions`, + `--trackDeps`, `--partialGraph`, ...), the cache (local disk, memory and the shared tier) and + the port. A cache entry is keyed by the git tree and the hashing configuration, never by the + clone it came from, so two clones that hold the same tree share one entry, and the cache a + single-repository instance built stays valid. +* Each repository has its own working tree, git, Bazel server, workspace lock and readiness, so + queries against different repositories run concurrently, while queries against the same + repository are serialised as before. Size the instance for that many Bazel servers at once. +* A request that names no `repo` is answered when the server serves exactly one repository and + refused with `400` otherwise; an unknown name is `404`. Either error lists the repositories + served. `serve` serves its one `--workspacePath` repository under the name `default`. +* `--warmupRevision` warms every repository; a revision one clone lacks logs a warning for that + repository and does not stop the others. +* `/health` reports `200` only once every repository's initial fetch succeeded, so a load + balancer never routes to an instance that can answer for some of its repositories but not + all. `/metrics` reports each repository under `repositories` with its own `ready`, + `activeQueries`, `workspaceBusy` and partial-graph counts, while the top-level fields + aggregate over all of them (`activeQueries` summed, `workspaceBusy` when any workspace is + held). + ### Shared S3 cache for multi-instance deployments A single instance caches hashes on local disk only. When you run several replicas behind a load @@ -896,6 +938,112 @@ Options: -V, --version Print version ``` + +### `serve-multi` command + +```terminal +Run the HTTP impacted-target query service for several repositories at once + +Usage: bazel-diff serve-multi [OPTIONS] --repo --cacheDir + +Options: + --repo + A repository to serve, as NAME=PATH (repeatable). Queries address one with `repo=NAME`. Every repository shares the hashing flags, the cache and the port + -v, --verbose + + -b, --bazelPath + Path to the Bazel or Bazelisk executable [default: bazel] + -s, --seed-filepaths + File containing workspace-relative paths whose contents seed every target hash + --bazelStartupOptions + Additional space-separated Bazel startup options + --bazelCommandOptions + Additional space-separated `bazel query` options + --cqueryCommandOptions + Additional space-separated `bazel cquery` options + --fineGrainedHashExternalRepos + + --fineGrainedHashExternalReposFile + + --labelOnlyModuleExtensions + Module extensions (`@rules_rs//rs:extensions.bzl%crate`) whose repositories are used only through labels, never by resolving a toolchain from one of them. The module that owns such an extension then does not stand in for its repositories, so a lockfile bump impacts only the targets that depend on the bumped repository + --isolatedDependencyRoot + Assert that consumers outside this root-isolated usage reach it through ordinary target inputs or BUILD loads. The named root import must cover every generated repository. Keep the whole usage on each such consumer + --useCquery[=] + [default: false] [possible values: true, false] + --cqueryExpression + + -k, --keep_going[=] + [default: false] [possible values: true, false] + --ignoredRuleHashingAttributes + + --excludeExternalTargets[=] + [default: false] [possible values: true, false] + --excludeTargetsQuery + + --alwaysAffectedTags + + --gitPath + [default: git] + --port + [default: 8080] + --requestTimeout + [default: 0] + --cacheDir + + --trackDeps[=] + [default: false] [possible values: true, false] + --dependencyFingerprint[=] + Opt-in: only serve cached entries whose external-dependency fingerprint still matches [default: false] [possible values: true, false] + --partialGraph[=] + Opt-in: derive a revision's hashes from the cached `from` entry without a `bazel query` when the diff between them only edits regular source files; every entry then also records the hashing recipes that makes possible. Incompatible with --useCquery and --alwaysAffectedTags [default: false] [possible values: true, false] + --partialGraphRebuildPattern + Glob (`*` within a path segment, `**` across, a pattern without `/` matches the file name) of workspace paths whose change always forces a full generation under --partialGraph + --allowUntrackedPackages + Cache a revision whose bazel query loaded packages its git tree does not contain (untracked or ignored BUILD files in the working tree) instead of failing the request + --no-initial-fetch + + --warmupRevision + + --cacheMaxAge + + --cacheMaxEntries + + --cacheMaxSize + + --cachePruneInterval + [default: 1h] + --cacheMemoryMax + [default: 512MB] + --s3Bucket + + --s3Prefix + [default: ""] + --s3Region + + --s3Endpoint + + --s3ForcePathStyle + + --gcsBucket + Google Cloud Storage bucket used as the shared cache tier (Application Default Credentials) + --gcsPrefix + Object-name prefix for cache entries in --gcsBucket [default: ""] + --nexusUrl + Base URL of a Sonatype Nexus Repository instance used as the shared cache tier + --nexusRepository + Raw hosted repository on --nexusUrl that holds cache entries + --nexusPrefix + [default: ""] + --nexusUsername + Basic-auth user; the password is read from the BAZEL_DIFF_NEXUS_PASSWORD environment variable [env: BAZEL_DIFF_NEXUS_USERNAME=] + --nexusCaCert + PEM bundle of extra CA certificates to trust for --nexusUrl + -h, --help + Print help + -V, --version + Print version +``` ### What does the SHA256 value of `generate-hashes` represent? diff --git a/src/main.rs b/src/main.rs index 103d0476..959ede9a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -62,17 +62,25 @@ enum Commands { Fingerprint(FingerprintArgs), #[command(about = "Run the HTTP impacted-target query service")] Serve(Box), + #[command( + name = "serve-multi", + about = "Run the HTTP impacted-target query service for several repositories at once" + )] + ServeMulti(Box), } #[derive(Clone, Debug, Args)] struct HashingArgs { + /// Required by every command but `serve`, which lifts the requirement (see [`ServeArgs`]) + /// so `--repo` can name the workspaces instead. #[arg( short = 'w', long = "workspacePath", value_parser = parse_normalized_path, + required = true, help = "Path to the Bazel workspace" )] - workspace_path: PathBuf, + workspace_path: Option, #[arg( short = 'b', @@ -373,6 +381,27 @@ struct WarmupArgs { fingerprint_output: PathBuf, } +/// `serve` for several repositories: every `serve` flag but `--workspacePath`, which `--repo` +/// replaces. Clap still knows `-w` through the flattened [`HashingArgs`], so it is unrequired +/// and hidden here rather than removed. +#[derive(Debug, Args)] +#[command(mut_arg("workspace_path", |arg| arg.required(false).hide(true)))] +struct ServeMultiArgs { + /// A repository to serve, as NAME=PATH (repeatable). Queries address one with `repo=NAME`. + /// Every repository shares the hashing flags, the cache and the port + #[arg( + long = "repo", + value_name = "NAME=PATH", + value_parser = parse_repository, + required = true, + conflicts_with = "workspace_path" + )] + repositories: Vec, + + #[command(flatten)] + serve: ServeArgs, +} + #[derive(Debug, Args)] struct ServeArgs { #[command(flatten)] @@ -549,6 +578,22 @@ fn parse_normalized_path(value: &str) -> Result { Ok(normalize_path(absolute)) } +/// `--repo NAME=PATH`, with the path anchored and normalized like `--workspacePath`. +fn parse_repository(value: &str) -> Result { + let mut repository = + bazel_diff::server::Repository::parse(value).map_err(|error| error.to_string())?; + repository.workspace = parse_normalized_path(&repository.workspace.to_string_lossy())?; + Ok(repository) +} + +impl HashingArgs { + /// The workspace clap required, or the empty path for a `serve` started with `--repo`, + /// whose config replaces it per repository before anything spawns in it. + fn workspace(&self) -> PathBuf { + self.workspace_path.clone().unwrap_or_default() + } +} + fn read_lines(path: Option<&Path>) -> Result> { let Some(path) = path else { return Ok(BTreeSet::new()); @@ -589,7 +634,7 @@ fn bazel_options(args: &HashingArgs, verbose: bool) -> Result { repos.extend(read_string_lines(path)?); } Ok(BazelOptions { - workspace: args.workspace_path.clone(), + workspace: args.workspace(), bazel: args.bazel_path.clone(), startup_options: flatten_options(&args.bazel_startup_options), command_options: flatten_options(&args.bazel_command_options), @@ -804,7 +849,7 @@ fn partial_graph_hashes( .iter() .map(|pattern| Glob::new(pattern)) .collect::>>()?; - let git = Git::new(&args.git_path, &args.hashing.workspace_path); + let git = Git::new(&args.git_path, args.hashing.workspace()); // bazel-diff's own files are not part of the revision, wherever the user keeps them. let own_files = [ args.output_path.as_deref(), @@ -1000,7 +1045,7 @@ fn run_explain(args: &ExplainArgs, verbose: bool) -> Result<()> { fn run_fingerprint(args: &FingerprintArgs) -> Result<()> { let flags = fingerprint_flags(&args.hashing, args.include_target_type, &args.target_type); let inputs = fingerprint::gather( - &args.hashing.workspace_path, + &args.hashing.workspace(), &args.hashing.bazel_path, flags.clone(), ); @@ -1056,13 +1101,27 @@ fn run_warmup_with( impl ServeArgs { fn to_config(&self, verbose: bool) -> Result { - self.to_config_with_nexus_password(verbose, std::env::var(NEXUS_PASSWORD_ENV).ok()) + self.to_config_with_repositories(verbose, Vec::new()) + } + + /// `serve-multi`: the same config, serving `repositories` instead of `--workspacePath`. + fn to_config_with_repositories( + &self, + verbose: bool, + repositories: Vec, + ) -> Result { + self.to_config_with_nexus_password( + verbose, + std::env::var(NEXUS_PASSWORD_ENV).ok(), + repositories, + ) } fn to_config_with_nexus_password( &self, verbose: bool, nexus_password: Option, + repositories: Vec, ) -> Result { if self.s3_bucket.is_some() && self.s3_endpoint.as_deref().is_some_and(str::is_empty) { bail!("--s3Endpoint must not be empty"); @@ -1106,8 +1165,15 @@ impl ServeArgs { } else { None }; + let mut hash_options = hash_options(&self.hashing, None, None, self.track_deps, verbose)?; + if let Some(first) = repositories.first() { + // `serve` replaces the workspace per repository; the first stands in for the + // validation below, which only reads the shared flags. + hash_options.bazel.workspace = first.workspace.clone(); + } let config = bazel_diff::server::ServerConfig { - hash_options: hash_options(&self.hashing, None, None, self.track_deps, verbose)?, + hash_options, + repositories, git_path: self.git_path.clone(), port: self.port, request_timeout: Duration::from_secs(self.request_timeout), @@ -1151,6 +1217,7 @@ impl ServeArgs { nexus_ca_cert: self.nexus_ca_cert.clone(), }; bazel_diff::server::validate_partial_graph(&config)?; + bazel_diff::server::repositories(&config)?; Ok(config) } } @@ -1197,6 +1264,12 @@ fn run() -> Result<()> { Commands::Serve(args) => { bazel_diff::server::serve(args.to_config(cli.verbose)?)?; } + Commands::ServeMulti(args) => { + bazel_diff::server::serve( + args.serve + .to_config_with_repositories(cli.verbose, args.repositories.clone())?, + )?; + } } io::stdout().flush()?; Ok(()) @@ -1218,7 +1291,7 @@ mod tests { fn hashing_args(workspace: &Path) -> HashingArgs { HashingArgs { - workspace_path: workspace.to_path_buf(), + workspace_path: Some(workspace.to_path_buf()), bazel_path: PathBuf::from("bazel"), seed_filepaths: None, bazel_startup_options: Vec::new(), @@ -1613,6 +1686,124 @@ mod tests { } } + #[test] + fn serve_multi_names_the_repositories_it_serves() { + let serve_multi = |extra: &[&str]| { + let mut argv = vec!["bazel-diff", "serve-multi", "--cacheDir", "/tmp/cache"]; + argv.extend(extra); + Cli::try_parse_from(&argv) + }; + let multi = |cli: Cli| match cli.command { + Commands::ServeMulti(args) => args, + _ => panic!("expected serve-multi"), + }; + let args = multi( + serve_multi(&[ + "--repo", + "app=./app", + "--repo", + "lib=/clones/lib", + "--trackDeps", + ]) + .unwrap(), + ); + assert!(args.serve.hashing.workspace_path.is_none()); + let names = args + .repositories + .iter() + .map(|repository| repository.name.as_str()) + .collect::>(); + assert_eq!(names, ["app", "lib"]); + // Anchored to the working directory like --workspacePath. + assert_eq!( + args.repositories[0].workspace, + std::path::absolute("app").unwrap() + ); + assert_eq!(args.repositories[1].workspace, PathBuf::from("/clones/lib")); + let config = args + .serve + .to_config_with_repositories(false, args.repositories.clone()) + .unwrap(); + assert_eq!(config.repositories, args.repositories); + assert_eq!( + config.hash_options.bazel.workspace, + args.repositories[0].workspace + ); + assert!(config.track_deps); + + // `serve` is untouched: --workspacePath is required and serves the default repository. + let cli = parse(&[ + "bazel-diff", + "serve", + "--workspacePath", + "/tmp/ws", + "--cacheDir", + "/tmp/cache", + ]); + let Commands::Serve(args) = cli.command else { + panic!("expected serve"); + }; + let config = args.to_config(false).unwrap(); + assert!(config.repositories.is_empty()); + assert_eq!( + bazel_diff::server::repositories(&config).unwrap(), + vec![bazel_diff::server::Repository { + name: bazel_diff::server::DEFAULT_REPOSITORY.to_owned(), + workspace: PathBuf::from("/tmp/ws"), + }] + ); + assert_eq!( + Cli::try_parse_from(["bazel-diff", "serve", "--cacheDir", "/tmp/cache"]) + .unwrap_err() + .kind(), + clap::error::ErrorKind::MissingRequiredArgument + ); + assert_eq!( + Cli::try_parse_from([ + "bazel-diff", + "serve", + "-w", + "/tmp/ws", + "--cacheDir", + "/tmp/cache", + "--repo", + "app=/a" + ]) + .unwrap_err() + .kind(), + clap::error::ErrorKind::UnknownArgument + ); + + // serve-multi takes --repo, not --workspacePath; names are validated as they are + // parsed and for uniqueness once they are all known. + assert_eq!( + serve_multi(&[]).unwrap_err().kind(), + clap::error::ErrorKind::MissingRequiredArgument + ); + assert_eq!( + serve_multi(&["--workspacePath", "/tmp/ws", "--repo", "app=/a"]) + .unwrap_err() + .kind(), + clap::error::ErrorKind::ArgumentConflict + ); + assert_eq!( + serve_multi(&["--repo", "/a"]).unwrap_err().kind(), + clap::error::ErrorKind::ValueValidation + ); + assert_eq!( + serve_multi(&["--repo", "my repo=/a"]).unwrap_err().kind(), + clap::error::ErrorKind::ValueValidation + ); + let args = multi(serve_multi(&["--repo", "app=/a", "--repo", "app=/b"]).unwrap()); + assert_eq!( + args.serve + .to_config_with_repositories(false, args.repositories.clone()) + .unwrap_err() + .to_string(), + "repository \"app\" is given more than once" + ); + } + #[test] fn serve_gcs_bucket_must_not_be_empty() { let cli = parse(&[ @@ -1707,7 +1898,7 @@ mod tests { ]; let config = serve_args(&nexus) - .to_config_with_nexus_password(false, None) + .to_config_with_nexus_password(false, None, Vec::new()) .unwrap(); assert_eq!( config.remote_cache.as_deref(), @@ -1725,7 +1916,7 @@ mod tests { let mut authenticated = nexus.to_vec(); authenticated.extend(["--nexusUsername", "svc"]); let config = serve_args(&authenticated) - .to_config_with_nexus_password(false, Some("s3cret".to_owned())) + .to_config_with_nexus_password(false, Some("s3cret".to_owned()), Vec::new()) .unwrap(); let credentials = config.nexus_credentials.unwrap(); assert_eq!(credentials.username, "svc"); @@ -1733,26 +1924,26 @@ mod tests { // A username without a password, or a password without a username, is a misconfiguration. assert!(serve_args(&authenticated) - .to_config_with_nexus_password(false, None) + .to_config_with_nexus_password(false, None, Vec::new()) .is_err()); assert!(serve_args(&nexus) - .to_config_with_nexus_password(false, Some("s3cret".to_owned())) + .to_config_with_nexus_password(false, Some("s3cret".to_owned()), Vec::new()) .is_err()); // The password environment variable is ignored when Nexus is not configured at all. assert!(serve_args(&[]) - .to_config_with_nexus_password(false, Some("s3cret".to_owned())) + .to_config_with_nexus_password(false, Some("s3cret".to_owned()), Vec::new()) .unwrap() .remote_cache .is_none()); // URL and repository problems surface at startup, not on the first cache lookup. assert!( serve_args(&["--nexusUrl", "ftp://host", "--nexusRepository", "cache"]) - .to_config_with_nexus_password(false, None) + .to_config_with_nexus_password(false, None, Vec::new()) .is_err() ); assert!( serve_args(&["--nexusUrl", "https://host", "--nexusRepository", "a/b"]) - .to_config_with_nexus_password(false, None) + .to_config_with_nexus_password(false, None, Vec::new()) .is_err() ); } diff --git a/src/server.rs b/src/server.rs index f0c9cc10..14c813fc 100644 --- a/src/server.rs +++ b/src/server.rs @@ -30,9 +30,58 @@ use std::time::{Duration, Instant, SystemTime}; use tiny_http::{Header, Method, Request, Response, Server, StatusCode}; use url::{form_urlencoded, Url}; +/// The name a server started with `--workspacePath` registers its one repository under. +pub const DEFAULT_REPOSITORY: &str = "default"; + +/// One git clone the server answers queries for. Every repository shares the hashing +/// configuration, the cache tiers and the port; each has its own working tree, git, Bazel +/// server, readiness and workspace lock, so queries against different repositories run +/// concurrently. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Repository { + pub name: String, + pub workspace: PathBuf, +} + +impl Repository { + /// Parses the `NAME=PATH` form of `--repo`. The path is not normalised here; the CLI does + /// that with the same rule it applies to `--workspacePath`. + pub fn parse(value: &str) -> Result { + let Some((name, workspace)) = value.split_once('=') else { + bail!("expected NAME=PATH, got {value:?}"); + }; + validate_repository_name(name)?; + if workspace.is_empty() { + bail!("repository {name:?} has an empty path"); + } + Ok(Self { + name: name.to_owned(), + workspace: PathBuf::from(workspace), + }) + } +} + +/// A repository name is addressed in URLs and JSON and names a log line, so it is kept to +/// letters, digits, `.`, `_` and `-`. +pub fn validate_repository_name(name: &str) -> Result<()> { + if name.is_empty() { + bail!("repository name must not be empty"); + } + if !name + .chars() + .all(|character| character.is_ascii_alphanumeric() || matches!(character, '.' | '_' | '-')) + { + bail!("repository name {name:?} may only contain letters, digits, '.', '_' and '-'"); + } + Ok(()) +} + #[derive(Clone, Debug)] pub struct ServerConfig { pub hash_options: HashOptions, + /// The repositories served, in the order given. Empty means the one repository at + /// `hash_options.bazel.workspace`, named [`DEFAULT_REPOSITORY`]. + pub repositories: Vec, pub git_path: PathBuf, pub port: u16, pub request_timeout: Duration, @@ -83,7 +132,12 @@ const CANCEL_GRACE: Duration = Duration::from_secs(60); /// How often a query waiting for the workspace re-checks whether it has been cancelled. const WORKSPACE_WAIT_POLL: Duration = Duration::from_millis(50); +/// One repository's share of the server (see [`Repository`]). `config` is the server's with +/// `hash_options.bazel.workspace` set to this repository's clone; the cache tiers behind +/// `remote` and `memory` are the [`Shared`] ones every repository uses, since a cache key is +/// the git tree plus the hashing configuration and says nothing about which clone it came from. struct State { + name: String, config: ServerConfig, ready: AtomicBool, workspace: Workspace, @@ -97,12 +151,28 @@ struct State { /// Queries accepted and not yet finished: waiting for the workspace, running, or abandoned by /// a timed-out client but still running. `/metrics` reports it as `activeQueries`. active_queries: AtomicUsize, - started: Instant, fingerprint: String, - remote: Option, + remote: Option>, /// Parsed entries, in front of the local-disk tier. Guarded by its own lock rather than the /// workspace's so `/metrics` can read its size while a query holds the workspace. - memory: Mutex, + memory: Arc>, +} + +/// The cache tiers every repository of one server shares: `--cacheDir` is the same path in +/// each repository's config, and these are the handles to the other two. +#[derive(Clone)] +struct Shared { + remote: Option>, + memory: Arc>, +} + +impl Shared { + fn new(config: &ServerConfig, remote: Option) -> Self { + Self { + remote: remote.map(Arc::new), + memory: Arc::new(Mutex::new(MemoryCache::new(config.cache_memory_max))), + } + } } /// The in-memory tier: entries already parsed, most recently used last. @@ -181,10 +251,19 @@ fn memory_cache(state: &State) -> std::sync::MutexGuard<'_, MemoryCache> { } impl State { + /// A single-repository state named [`DEFAULT_REPOSITORY`] with cache tiers of its own. + #[cfg(test)] fn new(config: ServerConfig, remote: Option) -> Self { + let shared = Shared::new(&config, remote); + Self::with_shared(DEFAULT_REPOSITORY, config, &shared) + } + + fn with_shared(name: &str, config: ServerConfig, shared: &Shared) -> Self { let fingerprint = configuration_fingerprint(&config); Self { - memory: Mutex::new(MemoryCache::new(config.cache_memory_max)), + name: name.to_owned(), + memory: Arc::clone(&shared.memory), + remote: shared.remote.clone(), config, ready: AtomicBool::new(false), partial_derived: AtomicUsize::new(0), @@ -192,12 +271,14 @@ impl State { untracked_package_rejections: AtomicUsize::new(0), workspace: Workspace::default(), active_queries: AtomicUsize::new(0), - started: Instant::now(), fingerprint, - remote, } } + fn workspace(&self) -> &Path { + &self.config.hash_options.bazel.workspace + } + /// Git bound to the query currently holding the workspace, so the subprocesses it spawns die /// with that query; outside the lock (startup fetch) they run plainly. fn git(&self) -> Git { @@ -298,6 +379,69 @@ impl Drop for ActiveQuery { } } +/// The repositories one server answers for, by name, and how a request picks one. +struct Service { + /// In the order the repositories were configured, which `/metrics` keeps. + repositories: Vec>, + started: Instant, +} + +/// Why a request's `repo` could not be resolved: the HTTP status and the message to answer with. +struct UnknownRepository { + status: u16, + message: String, +} + +impl Service { + fn new(repositories: Vec>) -> Self { + Self { + repositories, + started: Instant::now(), + } + } + + /// The repository a request addressed. A server with one repository answers for it whether + /// or not the request names it; with several, a request must name one. + fn select(&self, name: Option<&str>) -> Result<&Arc, UnknownRepository> { + let names = || { + self.repositories + .iter() + .map(|state| state.name.as_str()) + .collect::>() + .join(", ") + }; + match name { + Some(name) => self + .repositories + .iter() + .find(|state| state.name == name) + .ok_or_else(|| UnknownRepository { + status: 404, + message: format!( + "unknown repository {name:?}; this server serves: {}", + names() + ), + }), + None if self.repositories.len() == 1 => Ok(&self.repositories[0]), + None => Err(UnknownRepository { + status: 400, + message: format!( + "missing required parameter 'repo'; this server serves: {}", + names() + ), + }), + } + } + + /// The instance is ready once every repository's startup fetch succeeded, so a load + /// balancer never routes to an instance that can answer for only some of them. + fn ready(&self) -> bool { + self.repositories + .iter() + .all(|state| state.ready.load(Ordering::Acquire)) + } +} + /// The shared cache tier behind local disk. Every backend degrades the same way: a failed read is /// a miss (the revision is regenerated) and a failed write leaves the entry local-only, so an /// outage of the shared store costs throughput, never availability. @@ -834,6 +978,7 @@ fn read_pem_certificates(path: &Path) -> Result>> { #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] struct QueryBody { + repo: Option, from: Option, to: Option, target_type: Option>, @@ -843,6 +988,8 @@ struct QueryBody { #[derive(Debug)] struct QueryInputs { + /// The `repo` the request named, if any (see [`Service::select`]). + repository: Option, from: String, to: String, target_types: Option>, @@ -933,30 +1080,76 @@ pub fn serve(config: ServerConfig) -> Result<()> { bail!("serve does not support --isolatedDependencyRoot={root}"); } validate_partial_graph(&config)?; + let repositories = repositories(&config)?; fs::create_dir_all(&config.cache_dir)?; let remote = RemoteCache::new(&config)?; - let state = Arc::new(State::new(config, remote)); - prune_cache(&state)?; - start_cache_pruner(&state)?; - perform_initial_fetch(&state); - let server = Server::http(("0.0.0.0", state.config.port)) + let shared = Shared::new(&config, remote); + let port = config.port; + let states = repositories + .iter() + .map(|repository| { + let mut config = config.clone(); + config.hash_options.bazel.workspace = repository.workspace.clone(); + Arc::new(State::with_shared(&repository.name, config, &shared)) + }) + .collect::>(); + // Every repository prunes the same `--cacheDir`, so one pruner is enough. + prune_cache(&states[0])?; + start_cache_pruner(&states[0])?; + perform_initial_fetches(&states); + let service = Arc::new(Service::new(states)); + let server = Server::http(("0.0.0.0", port)) .map_err(|error| anyhow!("failed to bind HTTP server: {error}"))?; eprintln!( - "[Info] bazel-diff query service listening on port {}", - state.config.port + "[Info] bazel-diff query service listening on port {port} for {}", + repositories + .iter() + .map(|repository| format!("{}={}", repository.name, repository.workspace.display())) + .collect::>() + .join(", ") ); for request in server.incoming_requests() { - let state = Arc::clone(&state); - std::thread::spawn(move || handle_request(request, state)); + let service = Arc::clone(&service); + std::thread::spawn(move || handle_request(request, service)); } Ok(()) } +/// The repositories `config` serves: `config.repositories`, or the one at +/// `hash_options.bazel.workspace` named [`DEFAULT_REPOSITORY`]. Names must be valid and unique. +pub fn repositories(config: &ServerConfig) -> Result> { + if config.repositories.is_empty() { + return Ok(vec![Repository { + name: DEFAULT_REPOSITORY.to_owned(), + workspace: config.hash_options.bazel.workspace.clone(), + }]); + } + let mut seen = HashSet::new(); + for repository in &config.repositories { + validate_repository_name(&repository.name)?; + if !seen.insert(repository.name.as_str()) { + bail!("repository {:?} is given more than once", repository.name); + } + } + Ok(config.repositories.clone()) +} + +/// Fetches and warms every repository at once: each has its own clone and Bazel server, so +/// the startup cost of an instance is its slowest repository's, not their sum. +fn perform_initial_fetches(states: &[Arc]) { + std::thread::scope(|scope| { + for state in states { + scope.spawn(move || perform_initial_fetch(state)); + } + }); +} + /// Performs the startup git fetch (unless `--noInitialFetch`) and warmup, then flips `ready` /// once the clone is known good. On a fetch failure the server is left running but un-ready /// ("lame duck") so `/health` reports 503 and a load balancer removes the instance rather than /// us attempting a risky in-place repair; `/metrics` keeps answering with `ready: false`. fn perform_initial_fetch(state: &Arc) { + let name = &state.name; if !state.config.no_initial_fetch { let fetch = [ String::from("fetch"), @@ -965,14 +1158,14 @@ fn perform_initial_fetch(state: &Arc) { ]; if let Err(error) = git(state, &fetch) { eprintln!( - "[Error] initial git fetch failed; server is lame-ducked (health will report NOT_READY): {error:#}" + "[Error] initial git fetch of repository {name} failed; server is lame-ducked (health will report NOT_READY): {error:#}" ); return; } } for revision in &state.config.warmup_revisions { if let Err(error) = warm_revision(state, revision) { - eprintln!("[Warn] failed to warm revision {revision}: {error:#}"); + eprintln!("[Warn] failed to warm revision {revision} of repository {name}: {error:#}"); } } state.ready.store(true, Ordering::Release); @@ -1002,18 +1195,18 @@ fn start_cache_pruner(state: &Arc) -> Result<()> { Ok(()) } -fn handle_request(request: Request, state: Arc) { - if let Err(error) = route(request, &state) { +fn handle_request(request: Request, service: Arc) { + if let Err(error) = route(request, &service) { eprintln!("[Error] failed to handle HTTP request: {error:#}"); } } -fn route(mut request: Request, state: &Arc) -> Result<()> { +fn route(mut request: Request, service: &Service) -> Result<()> { let raw_url = request.url().to_owned(); let (path, query) = raw_url.split_once('?').unwrap_or((&raw_url, "")); match path { "/health" => { - let ready = state.ready.load(Ordering::Acquire); + let ready = service.ready(); respond_text( request, if ready { 200 } else { 503 }, @@ -1028,12 +1221,9 @@ fn route(mut request: Request, state: &Arc) -> Result<()> { &json!({"error": "method not allowed, use GET"}), ); } - respond_json(request, 200, &metrics(state)) + respond_json(request, 200, &metrics(service)) } "/impacted_targets" | "/impacted_targets_with_distances" | "/dependency_edges" => { - if !state.ready.load(Ordering::Acquire) { - return respond_json(request, 503, &json!({"error": "service not ready"})); - } if request.method() != &Method::Get && request.method() != &Method::Post { return respond_json( request, @@ -1047,6 +1237,19 @@ fn route(mut request: Request, state: &Arc) -> Result<()> { return respond_json(request, 400, &json!({"error": error.to_string()})) } }; + let state = match service.select(inputs.repository.as_deref()) { + Ok(state) => state, + Err(unknown) => { + return respond_json( + request, + unknown.status, + &json!({"error": unknown.message}), + ) + } + }; + if !state.ready.load(Ordering::Acquire) { + return respond_json(request, 503, &json!({"error": "service not ready"})); + } let kind = match path { "/dependency_edges" => QueryKind::DepEdges, "/impacted_targets_with_distances" => QueryKind::Distances, @@ -1143,6 +1346,15 @@ fn cancel_query( } fn parse_query_inputs(request: &mut Request, query: &str) -> Result { + let params = form_urlencoded::parse(query.as_bytes()) + .into_owned() + .collect::>(); + // `repo` is read from the query string for both methods, so a client can keep one URL per + // repository; a POST body's `repo` wins over it. + let repository = params + .get("repo") + .filter(|value| !value.is_empty()) + .cloned(); if request.method() == &Method::Post { let mut raw = String::new(); request.as_reader().read_to_string(&mut raw)?; @@ -1159,6 +1371,7 @@ fn parse_query_inputs(request: &mut Request, query: &str) -> Result .filter(|value| !value.is_empty()) .ok_or_else(|| anyhow!("missing required fields 'from' and 'to'"))?; return Ok(QueryInputs { + repository: body.repo.filter(|value| !value.is_empty()).or(repository), from, to, target_types: normalized_types(body.target_type), @@ -1173,9 +1386,6 @@ fn parse_query_inputs(request: &mut Request, query: &str) -> Result profile: body.profile.unwrap_or(false), }); } - let params = form_urlencoded::parse(query.as_bytes()) - .into_owned() - .collect::>(); let from = params .get("from") .filter(|value| !value.is_empty()) @@ -1187,6 +1397,7 @@ fn parse_query_inputs(request: &mut Request, query: &str) -> Result .cloned() .ok_or_else(|| anyhow!("missing required query parameters 'from' and 'to'"))?; Ok(QueryInputs { + repository, from, to, target_types: normalized_types( @@ -1945,7 +2156,11 @@ fn git_output(state: &State, args: &[String]) -> Result { state.git().output(args) } -fn metrics(state: &State) -> Value { +/// The instance snapshot. The top-level counters sum over every repository, and `workspaceBusy` +/// is true while any repository's workspace is held, so a single-repository deployment reads +/// exactly as it always has; `repositories` breaks the same figures down by repository. +fn metrics(service: &Service) -> Value { + let state = &service.repositories[0]; let entries = cache_entries(&state.config.cache_dir); let total = entries.iter().map(|entry| entry.size).sum::(); let (used, max) = memory_usage(); @@ -1953,24 +2168,37 @@ fn metrics(state: &State) -> Value { let memory = memory_cache(state); (memory.len(), memory.used, memory.budget) }; + let sum = |counter: fn(&State) -> &AtomicUsize| { + service + .repositories + .iter() + .map(|state| counter(state).load(Ordering::SeqCst)) + .sum::() + }; + let repositories = service + .repositories + .iter() + .map(|state| (state.name.clone(), repository_metrics(state))) + .collect::>(); json!({ "version": env!("CARGO_PKG_VERSION"), - "uptimeSeconds": state.started.elapsed().as_secs(), - "ready": state.ready.load(Ordering::Acquire), - "activeQueries": state.active_queries.load(Ordering::SeqCst), - "workspaceBusy": state.workspace.is_busy(), + "uptimeSeconds": service.started.elapsed().as_secs(), + "ready": service.ready(), + "activeQueries": sum(|state| &state.active_queries), + "workspaceBusy": service.repositories.iter().any(|state| state.workspace.is_busy()), "gitEngine": "subprocess", "trackDeps": state.config.track_deps, "dependencyFingerprint": state.config.dependency_fingerprint, "partialGraph": { "enabled": state.config.partial_graph, - "derived": state.partial_derived.load(Ordering::SeqCst), - "fullGenerations": state.partial_rebuilds.load(Ordering::SeqCst), + "derived": sum(|state| &state.partial_derived), + "fullGenerations": sum(|state| &state.partial_rebuilds), }, "untrackedPackages": { "allowed": state.config.allow_untracked_packages, - "rejections": state.untracked_package_rejections.load(Ordering::SeqCst), + "rejections": sum(|state| &state.untracked_package_rejections), }, + "repositories": repositories, "cache": { "directory": state.config.cache_dir.to_string_lossy(), "remote": state.config.remote_cache, @@ -1987,6 +2215,23 @@ fn metrics(state: &State) -> Value { }) } +/// One repository's share of `/metrics`: its clone, and the figures that are per workspace. +fn repository_metrics(state: &State) -> Value { + json!({ + "workspace": state.workspace().to_string_lossy(), + "ready": state.ready.load(Ordering::Acquire), + "activeQueries": state.active_queries.load(Ordering::SeqCst), + "workspaceBusy": state.workspace.is_busy(), + "partialGraph": { + "derived": state.partial_derived.load(Ordering::SeqCst), + "fullGenerations": state.partial_rebuilds.load(Ordering::SeqCst), + }, + "untrackedPackages": { + "rejections": state.untracked_package_rejections.load(Ordering::SeqCst), + }, + }) +} + #[derive(Clone)] struct CacheEntry { path: PathBuf, @@ -2142,6 +2387,7 @@ mod tests { always_affected_tags: HashSet::new(), partial_tree: None, }, + repositories: Vec::new(), git_path: PathBuf::from("git"), port: 0, request_timeout: Duration::ZERO, @@ -2238,6 +2484,11 @@ mod tests { Arc::new(state) } + /// A server answering for `state` alone, as `--workspacePath` starts one. + fn service_of(state: &Arc) -> Service { + Service::new(vec![Arc::clone(state)]) + } + /// The full-content cache key `get_hashes_locked` uses for a commit: its tree, not its SHA. fn cache_key_for_commit(state: &State, sha: &str) -> String { cache_key(state, &resolve_tree(state, sha).unwrap(), &BTreeSet::new()) @@ -2300,6 +2551,10 @@ mod tests { } fn request(state: &Arc, request: &str) -> String { + request_service(&service_of(state), request) + } + + fn request_service(service: &Service, request: &str) -> String { let server = Server::http("127.0.0.1:0").unwrap(); let address = server.server_addr().to_ip().unwrap(); let request = request.to_owned(); @@ -2312,10 +2567,217 @@ mod tests { response }); let incoming = server.recv().unwrap(); - route(incoming, state).unwrap(); + route(incoming, service).unwrap(); client.join().unwrap() } + /// `(status, body)` of a request routed to `service`. + fn query_service(service: &Service, request: &str) -> (u16, Value) { + let response = request_service(service, request); + let status = response + .split_whitespace() + .nth(1) + .and_then(|status| status.parse().ok()) + .unwrap(); + let body = response + .split_once("\r\n\r\n") + .map(|(_, body)| body) + .unwrap(); + (status, serde_json::from_str(body).unwrap()) + } + + /// A repository state whose cache tiers are `shared`, without the dependency-fingerprint + /// guard, so a pre-written entry is served without a checkout. + fn shared_state(name: &str, repo: &Path, cache: &Path, shared: &Shared) -> Arc { + let mut config = test_config(); + config.hash_options.bazel.workspace = repo.to_path_buf(); + config.hash_options.bazel.bazel = PathBuf::from("/bin/false"); + config.cache_dir = cache.to_path_buf(); + let state = State::with_shared(name, config, shared); + state.ready.store(true, Ordering::Release); + Arc::new(state) + } + + /// Writes cache entries for `from` and `to` in which `label` changed, so a query between + /// them answers `[label]` from the cache alone. + fn write_changed_entry(state: &State, from: &str, to: &str, label: &str) { + for (sha, value) in [(from, "before"), (to, "after")] { + let data = HashFileData { + hashes: std::collections::BTreeMap::from([( + label.to_owned(), + hash("Rule", value, value), + )]), + ..Default::default() + }; + let key = cache_key_for_commit(state, sha); + write_hashes( + &state.config.cache_dir.join(format!("{key}.json")), + &data, + false, + ); + } + } + + #[test] + fn repository_names_and_specs_are_validated() { + let parsed = Repository::parse("app=/clones/app").unwrap(); + assert_eq!(parsed.name, "app"); + assert_eq!(parsed.workspace, PathBuf::from("/clones/app")); + // The first `=` splits, so a path may contain one. + assert_eq!( + Repository::parse("a.b_c-1=/x=y").unwrap().workspace, + PathBuf::from("/x=y") + ); + for invalid in ["app", "=/x", "app=", "a b=/x", "a/b=/x"] { + assert!(Repository::parse(invalid).is_err(), "{invalid}"); + } + let mut config = test_config(); + config.hash_options.bazel.workspace = PathBuf::from("/single"); + let only = repositories(&config).unwrap(); + assert_eq!( + only, + vec![Repository { + name: DEFAULT_REPOSITORY.to_owned(), + workspace: PathBuf::from("/single") + }] + ); + config.repositories = vec![ + Repository::parse("app=/a").unwrap(), + Repository::parse("lib=/b").unwrap(), + ]; + assert_eq!(repositories(&config).unwrap(), config.repositories); + config + .repositories + .push(Repository::parse("app=/c").unwrap()); + assert_eq!( + repositories(&config).unwrap_err().to_string(), + "repository \"app\" is given more than once" + ); + } + + #[test] + fn queries_are_routed_to_the_named_repository() { + let (app_repo, app_from, app_to) = initialize_git_repo(); + let (lib_repo, lib_first, _) = initialize_git_repo(); + let cache = tempfile::tempdir().unwrap(); + let shared = Shared::new(&test_config(), None); + let app = shared_state("app", app_repo.path(), cache.path(), &shared); + let lib = shared_state("lib", lib_repo.path(), cache.path(), &shared); + // The two clones start identical, and a cache entry is keyed by the tree alone, so + // they would share one: give `lib` trees of its own to answer for. + assert_eq!( + cache_key_for_commit(&app, &app_from), + cache_key_for_commit(&lib, &lib_first) + ); + let lib_commit = |content: &str| { + fs::write(lib_repo.path().join("lib.txt"), content).unwrap(); + git_command(lib_repo.path(), &["add", "lib.txt"]); + git_command(lib_repo.path(), &["commit", "-q", "-m", content]); + git_command(lib_repo.path(), &["rev-parse", "HEAD"]) + }; + let lib_from = lib_commit("lib one"); + let lib_to = lib_commit("lib two"); + write_changed_entry(&app, &app_from, &app_to, "//app:one"); + write_changed_entry(&lib, &lib_from, &lib_to, "//lib:one"); + let service = Service::new(vec![Arc::clone(&app), Arc::clone(&lib)]); + let get = |query: &str| { + query_service( + &service, + &format!("GET /impacted_targets?{query} HTTP/1.1\r\nHost: x\r\n\r\n"), + ) + }; + + // With several repositories a request must name one, and only a configured one. + let (status, body) = get(&format!("from={app_from}&to={app_to}")); + assert_eq!(status, 400); + assert_eq!( + body["error"], + "missing required parameter 'repo'; this server serves: app, lib" + ); + let (status, body) = get(&format!("repo=web&from={app_from}&to={app_to}")); + assert_eq!(status, 404); + assert_eq!( + body["error"], + "unknown repository \"web\"; this server serves: app, lib" + ); + let (status, body) = get(&format!("repo=app&from={app_from}&to={app_to}")); + assert_eq!(status, 200, "{body}"); + assert_eq!(body["impactedTargets"], json!(["//app:one"])); + assert_eq!(body["from"], app_from); + let (status, body) = get(&format!("repo=lib&from={lib_from}&to={lib_to}")); + assert_eq!(status, 200, "{body}"); + assert_eq!(body["impactedTargets"], json!(["//lib:one"])); + // A revision of one repository is not resolvable in the other. + let (status, body) = get(&format!("repo=app&from={lib_from}&to={lib_to}")); + assert_eq!(status, 400); + assert!( + body["error"].as_str().unwrap().contains("missing"), + "{body}" + ); + + // POST: the body's `repo` wins over the query string's. + let body = json!({"repo": "lib", "from": lib_from, "to": lib_to}).to_string(); + let (status, answer) = query_service( + &service, + &format!( + "POST /impacted_targets?repo=app HTTP/1.1\r\nHost: x\r\nContent-Length: {}\r\n\r\n{body}", + body.len() + ), + ); + assert_eq!(status, 200, "{answer}"); + assert_eq!(answer["impactedTargets"], json!(["//lib:one"])); + + // Both repositories share the cache tiers, and `/metrics` breaks the figures down. + let (status, metrics) = query_service(&service, "GET /metrics HTTP/1.1\r\nHost: x\r\n\r\n"); + assert_eq!(status, 200); + assert_eq!(metrics["ready"], true); + assert_eq!(metrics["cache"]["entries"], 4); + assert_eq!(metrics["cache"]["memory"]["entries"], 4); + assert_eq!( + metrics["repositories"]["app"]["workspace"], + app_repo.path().to_string_lossy().as_ref() + ); + assert_eq!(metrics["repositories"]["lib"]["ready"], true); + assert_eq!(metrics["repositories"]["lib"]["activeQueries"], 0); + + // Readiness is per repository for queries and all-or-nothing for the instance. + lib.ready.store(false, Ordering::Release); + assert!( + request_service(&service, "GET /health HTTP/1.1\r\nHost: x\r\n\r\n") + .starts_with("HTTP/1.1 503") + ); + let (status, _) = get(&format!("repo=lib&from={lib_from}&to={lib_to}")); + assert_eq!(status, 503); + let (status, _) = get(&format!("repo=app&from={app_from}&to={app_to}")); + assert_eq!(status, 200); + let (_, metrics) = query_service(&service, "GET /metrics HTTP/1.1\r\nHost: x\r\n\r\n"); + assert_eq!(metrics["ready"], false); + assert_eq!(metrics["repositories"]["app"]["ready"], true); + assert_eq!(metrics["repositories"]["lib"]["ready"], false); + + // A single-repository server answers whether or not the request names it. + let single = service_of(&app); + for query in [ + format!("from={app_from}&to={app_to}"), + format!("repo=app&from={app_from}&to={app_to}"), + ] { + let (status, body) = query_service( + &single, + &format!("GET /impacted_targets?{query} HTTP/1.1\r\nHost: x\r\n\r\n"), + ); + assert_eq!(status, 200, "{body}"); + } + let (status, body) = query_service( + &single, + &format!("GET /impacted_targets?repo=lib&from={app_from}&to={app_to} HTTP/1.1\r\nHost: x\r\n\r\n"), + ); + assert_eq!(status, 404); + assert_eq!( + body["error"], + "unknown repository \"lib\"; this server serves: app" + ); + } + struct CapturedS3Request { url: String, } @@ -2636,7 +3098,10 @@ esac fs::read_dir(&cache).unwrap().next().is_none(), "a rejected generation must not be cached" ); - assert_eq!(metrics(&strict)["untrackedPackages"]["rejections"], 1); + assert_eq!( + metrics(&service_of(&strict))["untrackedPackages"]["rejections"], + 1 + ); let lenient = state_with(true); let retrieval = get_hashes_locked(&lenient, &sha, &BTreeSet::new(), None).unwrap(); @@ -2645,7 +3110,10 @@ esac lenient.untracked_package_rejections.load(Ordering::SeqCst), 0 ); - assert_eq!(metrics(&lenient)["untrackedPackages"]["allowed"], true); + assert_eq!( + metrics(&service_of(&lenient))["untrackedPackages"]["allowed"], + true + ); } /// A repository with the fixed graph above and four commits: `base`; a source and a README @@ -2843,11 +3311,14 @@ esac .unwrap() .contains("carries no partial-graph recipes")); - let snapshot = metrics(&state); + let snapshot = metrics(&service_of(&state)); assert_eq!(snapshot["partialGraph"]["enabled"], true); assert_eq!(snapshot["partialGraph"]["derived"], 2); assert_eq!(snapshot["partialGraph"]["fullGenerations"], 2); - assert_eq!(metrics(&plain)["partialGraph"]["enabled"], false); + assert_eq!( + metrics(&service_of(&plain))["partialGraph"]["enabled"], + false + ); } #[test] @@ -3690,11 +4161,14 @@ nQBeQ4AIl47nDOrFeVnvse8R9IHJ let key = cache_key_for_commit(&state, &sha); let path = cache.path().join(format!("{key}.json")); write_hashes(&path, &data, false); - assert_eq!(metrics(&state)["cache"]["memory"]["entries"], 0); + assert_eq!( + metrics(&service_of(&state))["cache"]["memory"]["entries"], + 0 + ); let first = get_hashes_locked(&state, &sha, &BTreeSet::new(), None).unwrap(); assert!(first.cache_hit); - let memory = metrics(&state)["cache"]["memory"].clone(); + let memory = metrics(&service_of(&state))["cache"]["memory"].clone(); assert_eq!(memory["entries"], 1); assert_eq!(memory["sizeBytes"], fs::metadata(&path).unwrap().len()); assert_eq!(memory["maxBytes"], state.config.cache_memory_max); @@ -3785,6 +4259,7 @@ nQBeQ4AIl47nDOrFeVnvse8R9IHJ ); } let inputs = QueryInputs { + repository: None, from: first, to: second.clone(), target_types: None, @@ -3991,7 +4466,7 @@ nQBeQ4AIl47nDOrFeVnvse8R9IHJ let cache = tempfile::tempdir().unwrap(); fs::write(cache.path().join("one.json"), vec![0; 1536]).unwrap(); let state = state_for_repo(repo.path(), cache.path(), true); - let value = metrics(&state); + let value = metrics(&service_of(&state)); assert_eq!(value["ready"], true); assert_eq!(value["trackDeps"], true); assert_eq!(value["dependencyFingerprint"], true); @@ -4194,7 +4669,7 @@ nQBeQ4AIl47nDOrFeVnvse8R9IHJ let repo = tempfile::tempdir().unwrap(); let cache = tempfile::tempdir().unwrap(); let state = state_for_repo(repo.path(), cache.path(), false); - let idle = metrics(&state); + let idle = metrics(&service_of(&state)); assert_eq!(idle["activeQueries"], 0); assert_eq!(idle["workspaceBusy"], false); assert!(state.workspace.holder().is_none()); @@ -4203,7 +4678,7 @@ nQBeQ4AIl47nDOrFeVnvse8R9IHJ let token = Arc::new(Cancellation::new()); let guard = state.workspace.acquire(&token).unwrap(); - assert_eq!(metrics(&state)["workspaceBusy"], true); + assert_eq!(metrics(&service_of(&state))["workspaceBusy"], true); assert!(Arc::ptr_eq(&state.workspace.holder().unwrap(), &token)); // Bazel options handed out while the query holds the workspace carry its token, so the // subprocesses they spawn are killed with it. @@ -4217,13 +4692,13 @@ nQBeQ4AIl47nDOrFeVnvse8R9IHJ ); assert_eq!(bazel.check_cancelled().unwrap_err().to_string(), CANCELLED); drop(guard); - assert_eq!(metrics(&state)["workspaceBusy"], false); + assert_eq!(metrics(&service_of(&state))["workspaceBusy"], false); assert!(state.workspace.holder().is_none()); let counted = ActiveQuery::start(&state); - assert_eq!(metrics(&state)["activeQueries"], 1); + assert_eq!(metrics(&service_of(&state))["activeQueries"], 1); drop(counted); - assert_eq!(metrics(&state)["activeQueries"], 0); + assert_eq!(metrics(&service_of(&state))["activeQueries"], 0); } #[test] diff --git a/tests/e2e/core.rs b/tests/e2e/core.rs index a5b621b8..a50821fe 100644 --- a/tests/e2e/core.rs +++ b/tests/e2e/core.rs @@ -239,6 +239,72 @@ fn serve_end_to_end() { ); } +/// One server, two clones: each query is answered from the repository it names, with the +/// repository's own revisions and hashes, and a query that names none is refused. +#[test] +fn serve_multiple_repositories() { + let app = copy_workspace("distance_metrics"); + let app_from = init_git(app.path()); + fs::write(app.path().join("lib.sh"), "echo app\n").unwrap(); + let app_to = commit(app.path(), "change app"); + let lib = copy_workspace("distance_metrics"); + let lib_from = init_git(lib.path()); + fs::write(lib.path().join("BUILD.bazel"), "").unwrap(); + let lib_to = commit(lib.path(), "empty lib"); + let cache = tempfile::tempdir().unwrap(); + let server = ServerProcess::start_repositories( + &[("app", app.path()), ("lib", lib.path())], + cache.path(), + &["--trackDeps"], + ); + let query = |path: &str| -> (u16, Value) { + let (status, body) = server.get(path).unwrap(); + (status, serde_json::from_str(&body).unwrap()) + }; + + let (status, body) = query(&format!("/impacted_targets?from={app_from}&to={app_to}")); + assert_eq!(status, 400, "{body}"); + assert!( + body["error"] + .as_str() + .unwrap() + .contains("missing required parameter 'repo'"), + "{body}" + ); + let (status, body) = query(&format!( + "/impacted_targets?repo=app&from={app_from}&to={app_to}" + )); + assert_eq!(status, 200, "{body}"); + let impacted = body["impactedTargets"].as_array().unwrap(); + assert!(impacted.iter().any(|label| label == "//:lib"), "{body}"); + // The library clone emptied its BUILD file: its targets are gone rather than changed, so + // only the BUILD file itself is impacted, unlike the app clone's answer above. + let (status, body) = query(&format!( + "/impacted_targets?repo=lib&from={lib_from}&to={lib_to}" + )); + assert_eq!(status, 200, "{body}"); + assert_eq!( + body["impactedTargets"], + serde_json::json!(["//:BUILD.bazel"]) + ); + let (status, body) = query(&format!( + "/impacted_targets?repo=lib&from={app_from}&to={app_to}" + )); + assert_eq!(status, 400, "{body}"); + let (status, body) = query(&format!( + "/impacted_targets_with_distances?repo=app&from={app_from}&to={app_to}" + )); + assert_eq!(status, 200, "{body}"); + let (status, metrics) = query("/metrics"); + assert_eq!(status, 200); + assert_eq!(metrics["ready"], true); + assert_eq!( + metrics["repositories"]["app"]["workspace"], + app.path().to_string_lossy().as_ref() + ); + assert_eq!(metrics["repositories"]["lib"]["ready"], true); +} + /// `--partialGraph`: a source edit is answered without a `bazel query`, with the same impacted /// set and distances as the default mode, and the derived cache entry is byte-identical to the /// one a full generation in the same mode writes; a `BUILD` edit falls back to a full generation. diff --git a/tests/e2e/support/mod.rs b/tests/e2e/support/mod.rs index d3db97f6..3e8d7782 100644 --- a/tests/e2e/support/mod.rs +++ b/tests/e2e/support/mod.rs @@ -430,11 +430,42 @@ pub struct ServerProcess { impl ServerProcess { pub fn start(workspace: &Path, cache: &Path, extra: &[&str]) -> Self { + Self::start_with( + "serve", + vec!["-w".to_owned(), workspace.display().to_string()], + cache, + extra, + ) + } + + /// `serve-multi` for several named repositories (`--repo NAME=PATH` each). + pub fn start_repositories( + repositories: &[(&str, &Path)], + cache: &Path, + extra: &[&str], + ) -> Self { + let args = repositories + .iter() + .flat_map(|(name, workspace)| { + [ + "--repo".to_owned(), + format!("{name}={}", workspace.display()), + ] + }) + .collect(); + Self::start_with("serve-multi", args, cache, extra) + } + + fn start_with( + command: &str, + workspace_args: Vec, + cache: &Path, + extra: &[&str], + ) -> Self { let port = free_port(); - let mut args = vec![ - "serve".to_owned(), - "-w".to_owned(), - workspace.display().to_string(), + let mut args = vec![command.to_owned()]; + args.extend(workspace_args); + args.extend([ "-b".to_owned(), bazel().display().to_string(), "--cacheDir".to_owned(), @@ -442,7 +473,7 @@ impl ServerProcess { "--port".to_owned(), port.to_string(), "--no-initial-fetch".to_owned(), - ]; + ]); args.extend(extra.iter().map(|value| (*value).to_owned())); let mut command = Command::new(binary()); command.args(args); diff --git a/tools/BUILD b/tools/BUILD index 781ac38a..e0c844c0 100644 --- a/tools/BUILD +++ b/tools/BUILD @@ -8,6 +8,7 @@ genrule( "help_get_impacted_targets.txt", "help_explain.txt", "help_serve.txt", + "help_serve_multi.txt", ], cmd = """ BINARY=$(location //src:bazel-diff) @@ -16,6 +17,7 @@ genrule( $$BINARY get-impacted-targets --help > $(location help_get_impacted_targets.txt) 2>&1 || true $$BINARY explain --help > $(location help_explain.txt) 2>&1 || true $$BINARY serve --help > $(location help_serve.txt) 2>&1 || true + $$BINARY serve-multi --help > $(location help_serve_multi.txt) 2>&1 || true """, tools = ["//src:bazel-diff"], ) diff --git a/tools/e2e/rust_e2e_cases.bzl b/tools/e2e/rust_e2e_cases.bzl index 14ac6d94..081b1366 100644 --- a/tools/e2e/rust_e2e_cases.bzl +++ b/tools/e2e/rust_e2e_cases.bzl @@ -51,6 +51,10 @@ RUST_E2E_SUITES = [ "name": "core::serve_macro_digest_is_revision_scoped", "timeout": "moderate", }, + { + "name": "core::serve_multiple_repositories", + "timeout": "moderate", + }, { "name": "core::serve_partial_graph_derives_source_edits", "timeout": "moderate", diff --git a/tools/generate_readme.py b/tools/generate_readme.py index 445ecf8a..d0de35e7 100644 --- a/tools/generate_readme.py +++ b/tools/generate_readme.py @@ -64,7 +64,12 @@ def inject_section(text: str, marker: str, content: str) -> str: # --------------------------------------------------------------------------- def build_cli_help_section( - help_root: str, help_gen: str, help_get: str, help_explain: str, help_serve: str + help_root: str, + help_gen: str, + help_get: str, + help_explain: str, + help_serve: str, + help_serve_multi: str, ) -> str: lines = [ "## CLI Interface", @@ -98,6 +103,12 @@ def build_cli_help_section( "```terminal", help_serve.rstrip(), "```", + "", + "### `serve-multi` command", + "", + "```terminal", + help_serve_multi.rstrip(), + "```", ] return "\n".join(lines) @@ -297,6 +308,7 @@ def main() -> None: help_get = runfile("tools/help_get_impacted_targets.txt").read_text() help_explain = runfile("tools/help_explain.txt").read_text() help_serve = runfile("tools/help_serve.txt").read_text() + help_serve_multi = runfile("tools/help_serve_multi.txt").read_text() version = read_module_version(workspace_dir) template = template.replace("{{BAZEL_DIFF_VERSION}}", version) @@ -309,7 +321,7 @@ def main() -> None: contributors_section = build_contributors_section(workspace_dir, email_map) cli_help_section = build_cli_help_section( - help_root, help_gen, help_get, help_explain, help_serve + help_root, help_gen, help_get, help_explain, help_serve, help_serve_multi ) readme = inject_section(template, "cli-help", cli_help_section) diff --git a/tools/readme_template.md b/tools/readme_template.md index ecec1d7f..eaa2d7d5 100644 --- a/tools/readme_template.md +++ b/tools/readme_template.md @@ -282,7 +282,9 @@ Endpoints: balancer should use this to route only to ready instances. If a fatal git error occurs at startup the instance "lame-ducks" itself by continuing to report `503` so the load balancer removes it. * `GET /impacted_targets?from=&to=` — returns the impacted targets as JSON. The optional - `targetType` parameter (e.g. `&targetType=Rule,SourceFile`) filters by target type. + `targetType` parameter (e.g. `&targetType=Rule,SourceFile`) filters by target type. On a server + that serves several repositories, `repo=` names the one to query (see [Serving several + repositories](#serving-several-repositories)). ```bash curl 'http://localhost:8080/impacted_targets?from=main&to=my-feature-branch' @@ -362,7 +364,9 @@ curl 'http://localhost:8080/dependency_edges?from=main&to=my-feature-branch' the queries accepted and not yet finished (queued, running, or abandoned by a client that stopped waiting), and `workspaceBusy` is `true` while any operation holds `--workspacePath` (a query's checkout and `bazel query`, or a startup warmup). A tool that shares the checkout with the server - should wait for `workspaceBusy` to be `false` before touching it. + should wait for `workspaceBusy` to be `false` before touching it. `repositories` breaks the + per-workspace figures down by repository (see [Serving several + repositories](#serving-several-repositories)). ```bash curl 'http://localhost:8080/metrics' @@ -377,11 +381,49 @@ curl 'http://localhost:8080/metrics' "workspaceBusy": true, "gitEngine": "subprocess", "trackDeps": false, + "repositories": {"default": {"workspace": "/path/to/workspace-clone", "ready": true, "activeQueries": 1, "workspaceBusy": true, "partialGraph": {"derived": 0, "fullGenerations": 0}, "untrackedPackages": {"rejections": 0}}}, "cache": {"directory": "/var/cache/bazel-diff", "remote": "s3://my-bucket/bazel-diff/", "entries": 128, "sizeBytes": 4823913, "sizeHuman": "4.6 MB", "memory": {"entries": 2, "sizeBytes": 75374, "maxBytes": 536870912}}, "jvm": {"usedBytes": 123456789, "maxBytes": 2147483648} } ``` +### Serving several repositories + +One instance can answer for several git clones. `bazel-diff serve-multi` takes every `serve` +flag except `--workspacePath`, which `--repo NAME=PATH` (repeatable) replaces, and a request +addresses one repository with `repo=NAME`: a query-string parameter on `GET` and `POST`, or a +`repo` field in the `POST` body, which wins over the query string. + +```bash +bazel-diff serve-multi \ + --repo app=/clones/app \ + --repo lib=/clones/lib \ + --cacheDir /var/cache/bazel-diff \ + --port 8080 + +curl 'http://localhost:8080/impacted_targets?repo=lib&from=main&to=my-feature-branch' +``` + +* Every repository shares the hashing flags (`--bazelPath`, `--bazelStartupOptions`, + `--trackDeps`, `--partialGraph`, ...), the cache (local disk, memory and the shared tier) and + the port. A cache entry is keyed by the git tree and the hashing configuration, never by the + clone it came from, so two clones that hold the same tree share one entry, and the cache a + single-repository instance built stays valid. +* Each repository has its own working tree, git, Bazel server, workspace lock and readiness, so + queries against different repositories run concurrently, while queries against the same + repository are serialised as before. Size the instance for that many Bazel servers at once. +* A request that names no `repo` is answered when the server serves exactly one repository and + refused with `400` otherwise; an unknown name is `404`. Either error lists the repositories + served. `serve` serves its one `--workspacePath` repository under the name `default`. +* `--warmupRevision` warms every repository; a revision one clone lacks logs a warning for that + repository and does not stop the others. +* `/health` reports `200` only once every repository's initial fetch succeeded, so a load + balancer never routes to an instance that can answer for some of its repositories but not + all. `/metrics` reports each repository under `repositories` with its own `ready`, + `activeQueries`, `workspaceBusy` and partial-graph counts, while the top-level fields + aggregate over all of them (`activeQueries` summed, `workspaceBusy` when any workspace is + held). + ### Shared S3 cache for multi-instance deployments A single instance caches hashes on local disk only. When you run several replicas behind a load