Skip to content

Tags: facebook/mcrouter

Tags

v2026.06.29.00

Toggle v2026.06.29.00's commit message
Hoist forEach option names into static const std::string to avoid per…

…-call temporaries

Reviewed By: DenisYaroshevskiy

Differential Revision: D109159609

fbshipit-source-id: 7257538ebda60d7b9ec3abcfb5d29fa2e6fd5e6a

v2026.06.22.00

Toggle v2026.06.22.00's commit message
Capture precomputed backup file name by move instead of copying name/…

…sourcePrefix

Reviewed By: jpporto

Differential Revision: D109159277

fbshipit-source-id: cc28512b422af51ac7dca7cb019c51de3c47367a

v2026.06.15.00

Toggle v2026.06.15.00's commit message
Stop mutating dynamic object keys in config macros

Summary:
## Goal.

Replace UB behavior of `const_cast`-ing elements which may be `const`-storage allocated with a proper drain semantics.

## Context.

The current implementation of mcrouter performs operations of the form:

```
for (auto& item : object.items()) {
  auto& key = const_cast<folly::dynamic&>(item.first);
  auto& value = item.second;
  output.insert(std::move(key), std::move(value));
}
```

As explained in D108334544 this can generally be a UB and cause bad outcomes since the compiler is free to optimize away the const storage such that any mutation of it can lead to an undefined behavior. In production we have found clusters of cores and ASAN failures which show invalid memory being accessed within the keys of objects which implies that this may have production impact. Even if that is not the case, avoiding UB is a strict improvement and hardening.

## This diff

`const_cast` is used as a crutch since there are no extraction semantics supported within `folly::object::dictionary`. The diff below adds such semantics and allows safe, zero-copy move out of one container into another. This diff introduces no behavioral changes at all, but it does improve safety.

## Cores and more context

Direct ASAN core analysis of ASAN-instrumented `thatch_vm` tasks shows crashes while mcrouter is preprocessing config macros on the config thread. Cores `lzunp38fulwr6r2k` and `r93hc7pfo7ex3tcs` abort while destroying a `folly::dynamic::object` from `facebook::memcache::ConfigPreprocessor::expandMacros`; core `279y5ckx34qb9q67` separately reaches a bad/end F14 iterator dereference in `ConfigPreprocessor::Context::expandRawArg` through `BuiltIns::ifMacro`. Production core stack paste: https://www.internalfb.com/intern/paste/P2374616358/

Inline production stack summary:
```
operator delete
folly::f14::detail::F14Table<folly::dynamic, folly::dynamic>::reset
folly::dynamic::destroy
folly::dynamic::ObjectMaker::~ObjectMaker

Reviewed By: snarkmaster

Differential Revision: D108301752

fbshipit-source-id: aad384111cd8c80f01788c6a9692f83fa993627b

v2026.06.08.00

Toggle v2026.06.08.00's commit message
add SetDistributionTargetRoute to stamp DL target from config

Summary:
New route handle SetDistributionTargetRoute stamps the distributionTargetRegion fiber-local (inside runWithLocals) and delegates to its child, so the ucache pool's auto-wrapped DistributionRoute broadcasts the write to DL. C++ half of moving the ads adm:* global-replication prefixes off legacy fabric broadcast onto DL.

route(): (a) target already stamped (e.g. RootRoute's directed stamp) -> delegate; (b) non-empty routing prefix -> delegate (don't convert directed into broadcast); (c) empty prefix -> stamp target_region ("" = broadcast) and route child in-scope. Lives in routes/ (no FOLLY_ATTR_WEAK), registered in buildRouteMap().

Also bumps mcrouter ClientVersion to 182 so the config can gate DL on mc-client-version >= 182. Land before the configerator change.

Reviewed By: stuclar

Differential Revision: D107156224

fbshipit-source-id: 8db84ba22df57827b897c7b6c77c767f7a9b5de6

v2026.06.01.00

Toggle v2026.06.01.00's commit message
closeNow() before socket_.reset() to fix Fizz handshake UAF

Summary:
fizz::client::AsyncFizzClient stores its ConnectCallback as a raw
pointer and arms a handshake-timeout timer when connect() is called.
On the inline-Fizz outbound path in mcrouter, that pointer is the
AsyncMcClientImpl. If the underlying McFizzClient transport outlives
AsyncMcClientImpl with the timer still armed, deliverHandshakeError
dereferences a freed ConnectCallback and crashes. The race window
scales with the configured connect/handshake timeout.

Two paths in AsyncMcClientImpl destroy the transport via
socket_.reset() without first calling socket_->closeNow():

  - connectErr()
  - processShutdown()

closeNow() runs deliverAllErrors -> deliverHandshakeError synchronously,
which cancels the Fizz handshake-timeout timer, clears the raw
ConnectCallback pointer, and delivers a terminal connectErr(). After
closeNow(), even if a DestructorGuard keeps the transport alive past
socket_.reset(), the dangling-pointer hazard is gone.

This matches the existing pattern in ~AsyncMcClientImpl(). Both
reset()-without-closeNow() sites now follow it.

Out of scope: the async SSL handshake-offload path (already anchored
via selfPtr_.lock()), plaintext / non-Fizz, and inbound (server-side)
Fizz handshakes (already fixed separately).

Reviewed By: knekritz

Differential Revision: D105440074

fbshipit-source-id: 4be476feab9041e900f93db690a32d3ac7d8e744

v2026.05.25.00

Toggle v2026.05.25.00's commit message
Make carbon-generated Messages headers self-contained

Summary:
X-link: facebookresearch/DCPerf#631

`Messages-inl.h` is not "self-contained", meaning a .cpp file that only includes it will fail to compile. This blocks some build time optimizations like C++20 header units.

-inl.h are discouraged for that reason (https://www.internalfb.com/wiki/Cpp/CppCodingConventions#avoid-using-inl-h-files), and the proper way is to just list all templates directly in the including file. But we can make a narrow change instead to avoid large scale code movements.

Introduce `Messages-decl.h` (declarations only) so that both `Messages.h` and `Messages-inl.h` are self-contained:

- `Messages-decl.h`: declarations, type traits, free function forward declarations. Identical to the current `Messages.h` without the last `#include "Messages-inl.h"` line.
- `Messages-inl.h`: adds `#pragma once` and `#include "Messages-decl.h"` at top, otherwise unchanged
- `Messages.h`: thin wrapper that includes `-decl.h` and `-inl.h`

Reviewed By: IvanTopolcic

Differential Revision: D106002540

fbshipit-source-id: 3153f0adb2325293e5b50159aca8609c7fa0dd17

v2026.05.18.00

Toggle v2026.05.18.00's commit message
mcrouter: log lease_token to mcrouter_requests Scuba dataset

Summary:
Adds a `lease_token` int column to the `mcrouter_requests` Scuba dataset for `McLeaseGetReply` and `McLeaseSetRequest`. The column is omitted for non-lease ops, so a token value of `0` is distinguishable from absence.

Only `FbAdditionalProxyRequestLogger` reads the token via the new `getLeaseTokenIfExist` SFINAE helper; `UcacheLogger` and `TaoLogger` pass `std::nullopt` since their wrapper types never carry leases. `if constexpr` makes the helper a no-op for non-lease types.

S575723 follow-up: lease tokens were not logged anywhere, making the KCB bug hard to debug from client traces.

Reviewed By: stuclar

Differential Revision: D105033460

fbshipit-source-id: c7d586ce782a64eb54fab0d7ab9bd6b5558a4867

v2026.05.11.00

Toggle v2026.05.11.00's commit message
fbcode/mcrouter/routes/McRouteHandleProvider-inl.h

Reviewed By: MatzeB

Differential Revision: D104066553

fbshipit-source-id: 0a37e97d59a1f7287ef32ab54a90afe0b1569239

v2026.05.04.00

Toggle v2026.05.04.00's commit message
Enable Pyrefly in fbcode/mcrouter

Summary:
Automated migration to enable Pyrefly type checking for `fbcode/mcrouter`.

- Added `python.set_pyrefly(True)` to PACKAGE file
- Suppressed pre-existing type errors

Pyrefly is Meta's next-generation Python type checker, replacing Pyre.

If you encounter issues, you can revert the PACKAGE change by removing
the `python.set_pyrefly(True)` line.
#pyreupgrade

Differential Revision: D103425534

fbshipit-source-id: dbea23f732c5b6d3a9e8e2f0de0c75a8803b6ad4

v2026.04.27.00

Toggle v2026.04.27.00's commit message
Make ExternalCarbonConnectionImpl fiber stack size a gflag (default 6…

…4 KiB)

Summary:
Human Darrien - For the mcrouter folks, let us know if this is a reasonable change to make. We are getting fiber overflows in our tests as of late since D100058297 in our tests.

We're hoping to provide an override in the least invasive way so we can have these tests run again. Let us know if this is reasonable 🙂‍↕️

***

The `fbjava/privacy/zone:zone_library_test` integration tests fail in `mode/dev` with `folly::fibers Fiber stack overflow detected`. Root cause: `mcrouter/lib/carbon/connection/ExternalCarbonConnectionImpl.cpp`'s `ThreadInfo` constructs its `FiberManager` with the default `Options` (16 KiB stack), which becomes 32 KiB in dev after the `stackSizeMultiplier=2` is applied. The `AsyncMcClient::create` path running on that fiber overflows the 32 KiB stack in unoptimized builds where frames are larger.

Added a `cacheclient_external_fiber_stack_size` gflag defaulting to 65536 bytes (64 KiB), and wired it into the `FiberManager::Options.stackSize` used by `ThreadInfo`. The new default gives 64 KiB × 2 = 128 KiB in dev (and 64 KiB in opt), enough headroom for `AsyncMcClient::create` while still being reasonable for a per-fiber allocation. Operators can tune this further at runtime if needed.

Differential Revision: D101644600

fbshipit-source-id: b75d788c6339a76265de634da7e23af83cb70d30