Skip to content

Commit 6a0bf98

Browse files
mzrmeta-codesync[bot]
authored andcommitted
lfs_server: gate upstream batch_download fetch behind a JustKnob
Summary: Adds `scm/mononoke:lfs_server_skip_upstream_for_downloads`, a per-repo JustKnob that, when enabled, makes `batch_download` skip the upstream LFS server entirely. Only the internal Filestore is consulted; objects that exist solely upstream are reported as 404 to the client. Upload paths (`batch_upload`, `upload_from_client`, `sync_internal_and_upstream`) are unaffected and still propagate uploads to upstream. The motivation is to support repos where new content has stopped being mirrored upstream: clients should still write through to upstream so the mirror stays in sync, but downloads should not depend on or wait for upstream. The existing knob `repo_config().lfs.use_upstream_lfs_server` is all-or-nothing — disabling it removes upstream from both directions. The new JK is download-only and switchable per-repo (via `switchval = repo_name`) so the change can be canaried instead of flipped globally. Default in `just_knobs.json` is `false`, preserving existing behavior. Reviewed By: YousefSalama Differential Revision: D104026975 fbshipit-source-id: d82f01df50da3a5adb4e618668f029a16fb49a5f
1 parent 21a6228 commit 6a0bf98

2 files changed

Lines changed: 73 additions & 0 deletions

File tree

eden/mononoke/common/mononoke_macros/just_knobs_defaults/just_knobs.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@
6969
"scm/mononoke:use_acl_manifest_for_restricted_paths": true,
7070
"scm/metagit:mononoke_git_lfs": false,
7171
"scm/mononoke:lfs_server_compression_sniff_enabled": false,
72+
"scm/mononoke:lfs_server_skip_upstream_for_downloads": false,
7273
"scm/mononoke:use_split_config_loading": false,
7374
"scm/mononoke:cross_repo_pause_backsyncer": false,
7475
"scm/mononoke:allow_bare_author_unixname": true,

eden/mononoke/servers/lfs/lfs_server/src/batch.rs

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ use mononoke_types::hash::Sha256;
4949
use mononoke_types::typed_hash::ContentId;
5050
use redactedblobstore::has_redaction_root_cause;
5151
use repo_blobstore::RepoBlobstoreRef;
52+
use repo_identity::RepoIdentityRef;
5253
use serde::Deserialize;
5354
use stats::prelude::*;
5455
use time_ext::DurationExt;
@@ -73,6 +74,13 @@ define_stats! {
7374
upload_rejected: timeseries(Rate, Sum),
7475
}
7576

77+
/// JustKnob that, when enabled for a repo, makes batch download responses
78+
/// ignore the upstream LFS server entirely. Internal-only objects are returned
79+
/// as usual; objects that exist only upstream become 404s. Upload paths are
80+
/// unaffected. Per `eden/.llms/rules/rust_unwrap_safety.md`, a bare `?` is the
81+
/// correct pattern here; defaults belong in `just_knobs.json`.
82+
const SKIP_UPSTREAM_FOR_DOWNLOADS_JK: &str = "scm/mononoke:lfs_server_skip_upstream_for_downloads";
83+
7684
enum Source {
7785
Internal,
7886
Upstream,
@@ -566,6 +574,25 @@ async fn batch_download(
566574
batch: RequestBatch,
567575
scuba: &mut Option<&mut ScubaMiddlewareState>,
568576
) -> Result<ResponseBatch, ErrorKind> {
577+
if justknobs::eval(
578+
SKIP_UPSTREAM_FOR_DOWNLOADS_JK,
579+
None,
580+
Some(ctx.repo.repo_identity().name()),
581+
)
582+
.map_err(ErrorKind::Error)?
583+
{
584+
let internal_objects = internal_objects(ctx, &batch.objects).await?;
585+
ScubaMiddlewareState::maybe_add(scuba, LfsScubaKey::BatchOrder, "skip_upstream");
586+
let upstream = Ok(UpstreamObjects::NoUpstream);
587+
let objects =
588+
batch_download_response_objects(&batch.objects, &upstream, &internal_objects, scuba)
589+
.map_err(ErrorKind::Error)?;
590+
return Ok(ResponseBatch {
591+
transfer: Transfer::Basic,
592+
objects,
593+
});
594+
}
595+
569596
let upstream = upstream_objects(ctx, &batch.objects).fuse();
570597
let internal = internal_objects(ctx, &batch.objects).fuse();
571598
pin_mut!(upstream, internal);
@@ -1207,4 +1234,49 @@ mod test {
12071234

12081235
Ok(())
12091236
}
1237+
1238+
#[mononoke::fbinit_test]
1239+
async fn test_batch_download_skips_upstream_when_jk_enabled(
1240+
fb: FacebookInit,
1241+
) -> Result<(), Error> {
1242+
use justknobs::test_helpers::JustKnobsInMemory;
1243+
use justknobs::test_helpers::KnobVal;
1244+
use justknobs::test_helpers::with_just_knobs_async;
1245+
1246+
// Build a repo whose LFS config enables upstream and point the context
1247+
// at an upstream URI. The test HTTP client is Disabled — so any actual
1248+
// dispatch to upstream would panic. The test passes only if the JK gate
1249+
// prevents that call.
1250+
let repo: Repo = TestRepoFactory::new(fb)?
1251+
.with_config_override(|c| c.lfs.use_upstream_lfs_server = true)
1252+
.build()
1253+
.await?;
1254+
1255+
let ctx = RepositoryRequestContext::test_builder_with_repo(fb, repo)?
1256+
.upstream_uri(Some("http://upstream.example/".to_string()))
1257+
.build()?;
1258+
1259+
let request = RequestBatch {
1260+
operation: Operation::Download,
1261+
r#ref: None,
1262+
transfers: vec![Transfer::Basic],
1263+
objects: vec![obj(ONES_SHA256, 111)],
1264+
};
1265+
1266+
let res = with_just_knobs_async(
1267+
JustKnobsInMemory::new(hashmap! {
1268+
SKIP_UPSTREAM_FOR_DOWNLOADS_JK.to_string() => KnobVal::Bool(true),
1269+
}),
1270+
Box::pin(batch_download(&ctx, request, &mut None)),
1271+
)
1272+
.await?;
1273+
1274+
assert_eq!(res.objects.len(), 1);
1275+
match &res.objects[0].status {
1276+
ObjectStatus::Err { error } => assert_eq!(error.code, 404),
1277+
ObjectStatus::Ok { .. } => panic!("expected 404, got Ok"),
1278+
}
1279+
1280+
Ok(())
1281+
}
12101282
}

0 commit comments

Comments
 (0)