Skip to content

Commit 8152658

Browse files
jfmendez11claude
jfmendez11
and
claude
authored
Use session cache in broadcast-signal to skip DB lookups (microsoft#26588)
## Description Add Redis session cache support to the broadcast-signal endpoint. The cache is populated by the session discovery endpoint (GET /documents/:tenantId/session/:id) which clients call before connecting, so it will be warm for any active session. - Read from getSessionFromCache before falling back to DB - Export getSessionFromCache from sessionHelper for reuse - Add cacheHit telemetry to success and 404 logs - Add tests for cache hit, miss, redirect, and deleted sentinel --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b771a28 commit 8152658

File tree

5 files changed

+206
-11
lines changed

5 files changed

+206
-11
lines changed

‎server/routerlicious/packages/routerlicious-base/src/alfred/routes/api/api.ts‎

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import sillyname from "sillyname";
3636
import { v4 as uuid } from "uuid";
3737
import winston from "winston";
3838

39-
import { Constants } from "../../../utils";
39+
import { Constants, getSessionFromCache } from "../../../utils";
4040

4141
import {
4242
craftClientJoinMessage,
@@ -57,6 +57,7 @@ export function create(
5757
revokedTokenChecker?: core.IRevokedTokenChecker,
5858
collaborationSessionEventEmitter?: RedisEmitter,
5959
fluidAccessTokenGenerator?: core.IFluidAccessTokenGenerator,
60+
redisCacheForGetSession?: core.ICache,
6061
denyList?: core.IDenyList,
6162
): Router {
6263
const router: Router = Router();
@@ -199,6 +200,7 @@ export function create(
199200
config,
200201
storage,
201202
collaborationSessionEventEmitter,
203+
redisCacheForGetSession,
202204
);
203205
handleResponse(
204206
handleBroadcastSignalP,
@@ -429,6 +431,7 @@ async function handleBroadcastSignal(
429431
config: Provider,
430432
storage: core.IDocumentStorage,
431433
collaborationSessionEventEmitter?: RedisEmitter,
434+
redisCacheForGetSession?: core.ICache,
432435
): Promise<void> {
433436
const tenantId = request.params.tenantId;
434437
const documentId = request.params.id;
@@ -452,25 +455,53 @@ async function handleBroadcastSignal(
452455
}
453456

454457
const serverUrl: string = config.get("worker:serverUrl");
455-
const document = await storage.getDocument(tenantId, documentId);
458+
let sessionOrdererUrl: string | undefined;
459+
let isSessionAlive: boolean | undefined;
460+
let cacheHit = false;
461+
462+
// Attempt a cache lookup first to avoid a DB round-trip on every call.
463+
// The cache is populated by the session discovery endpoint (GET /documents/:tenantId/session/:id)
464+
// which clients call before connecting, so it will be warm for any active session.
465+
if (redisCacheForGetSession) {
466+
const cachedSession = await getSessionFromCache(
467+
tenantId,
468+
documentId,
469+
redisCacheForGetSession,
470+
);
471+
if (cachedSession) {
472+
sessionOrdererUrl = cachedSession.ordererUrl;
473+
isSessionAlive = cachedSession.isSessionAlive;
474+
cacheHit = true;
475+
}
476+
}
477+
478+
// Cache miss: fall back to DB lookup.
479+
if (!cacheHit) {
480+
const document = await storage.getDocument(tenantId, documentId);
481+
sessionOrdererUrl = document?.session?.ordererUrl;
482+
// A document with a scheduled deletion time should be treated as not found.
483+
isSessionAlive = document?.session?.isSessionAlive && !document?.scheduledDeletionTime;
484+
}
456485

457-
if (!document?.session?.isSessionAlive || document?.scheduledDeletionTime) {
458-
Lumberjack.error("Document not found", { tenantId, documentId });
486+
if (!isSessionAlive || !sessionOrdererUrl) {
487+
Lumberjack.error("Document not found", { tenantId, documentId, cacheHit });
459488
throw new NetworkError(404, "Document not found");
460489
}
461-
if (document.session.ordererUrl !== serverUrl) {
490+
491+
if (sessionOrdererUrl !== serverUrl) {
462492
Lumberjack.info("Redirecting broadcast-signal to correct cluster", {
463-
documentUrl: document.session.ordererUrl,
493+
documentUrl: sessionOrdererUrl,
464494
currentUrl: serverUrl,
465-
targetUrlAndPath: `${document.session.ordererUrl}${request.originalUrl}`,
495+
targetUrlAndPath: `${sessionOrdererUrl}${request.originalUrl}`,
496+
cacheHit,
466497
});
467-
response.redirect(`${document.session.ordererUrl}${request.originalUrl}`);
498+
response.redirect(`${sessionOrdererUrl}${request.originalUrl}`);
468499
return;
469500
}
470501

471502
const signalMessage = createRuntimeMessage(signalContent);
472503
const signalRoom: IRoom = { tenantId, documentId };
473-
Lumberjack.info("Broadcasting signal to room", { tenantId, documentId });
504+
Lumberjack.info("Broadcasting signal to room", { tenantId, documentId, cacheHit });
474505
collaborationSessionEventEmitter.to(getRoomId(signalRoom)).emit("signal", signalMessage);
475506
}
476507

‎server/routerlicious/packages/routerlicious-base/src/alfred/routes/api/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ export function create(
9191
revokedTokenChecker,
9292
collaborationSessionEventEmitter,
9393
fluidAccessTokenGenerator,
94+
redisCacheForGetSession,
9495
denyList,
9596
);
9697

‎server/routerlicious/packages/routerlicious-base/src/test/alfred/api.spec.ts‎

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,164 @@ describe("Routerlicious", () => {
522522
});
523523
});
524524

525+
describe("broadcast-signal cache behavior", () => {
526+
let defaultSessionCache: TestCache;
527+
528+
beforeEach(() => {
529+
const restTenantThrottler = new TestThrottler(10);
530+
const restTenantThrottlers = new Map<string, TestThrottler>();
531+
restTenantThrottlers.set(
532+
Constants.generalRestCallThrottleIdPrefix,
533+
restTenantThrottler,
534+
);
535+
const restClusterThrottlers = new Map<string, TestThrottler>();
536+
const startupCheck = new StartupCheck();
537+
testFluidAccessTokenGenerator = new TestFluidAccessTokenGenerator();
538+
defaultSessionCache = new TestCache();
539+
app = alfredApp.create(
540+
defaultProvider,
541+
defaultTenantManager,
542+
restTenantThrottlers,
543+
restClusterThrottlers,
544+
defaultSingleUseTokenCache,
545+
defaultStorage,
546+
defaultAppTenants,
547+
defaultDeltaService,
548+
defaultProducer,
549+
defaultDocumentRepository,
550+
defaultDocumentDeleteService,
551+
startupCheck,
552+
undefined,
553+
undefined,
554+
defaultCollaborationSessionEventEmitter,
555+
undefined,
556+
undefined,
557+
undefined,
558+
testFluidAccessTokenGenerator,
559+
defaultSessionCache,
560+
);
561+
supertest = request(app);
562+
});
563+
564+
afterEach(() => {
565+
Sinon.restore();
566+
});
567+
568+
const signalBody = {
569+
signalContent: {
570+
contents: {
571+
type: "ExternalDataChanged_V1.0.0",
572+
content: { taskListId: "task-list-1" },
573+
},
574+
},
575+
};
576+
577+
it("Cache hit on same cluster returns 200 without calling DB", async () => {
578+
const cacheKey = SessionHelper.generateCacheKey(
579+
appTenant1.id,
580+
document1._id,
581+
);
582+
const cachedSession = {
583+
ordererUrl: defaultProvider.get("worker:serverUrl"),
584+
deltaStreamUrl: defaultProvider.get("worker:deltaStreamUrl"),
585+
historianUrl: defaultProvider.get("worker:blobStorageUrl"),
586+
isSessionAlive: true,
587+
isSessionActive: true,
588+
};
589+
await defaultSessionCache.set(cacheKey, JSON.stringify(cachedSession));
590+
591+
const getDocumentStub = Sinon.stub(defaultStorage, "getDocument");
592+
593+
await supertest
594+
.post(`/api/v1/${appTenant1.id}/${document1._id}/broadcast-signal`)
595+
.send(signalBody)
596+
.set("Authorization", tenantToken1)
597+
.set("Content-Type", "application/json")
598+
.expect(200);
599+
600+
assert(
601+
getDocumentStub.notCalled,
602+
"getDocument should not be called on a cache hit",
603+
);
604+
});
605+
606+
it("Cache hit for wrong cluster returns 302 without calling DB", async () => {
607+
const otherClusterUrl = "http://other-cluster:3006";
608+
const cacheKey = SessionHelper.generateCacheKey(
609+
appTenant1.id,
610+
document1._id,
611+
);
612+
const cachedSession = {
613+
ordererUrl: otherClusterUrl,
614+
deltaStreamUrl: defaultProvider.get("worker:deltaStreamUrl"),
615+
historianUrl: defaultProvider.get("worker:blobStorageUrl"),
616+
isSessionAlive: true,
617+
isSessionActive: true,
618+
};
619+
await defaultSessionCache.set(cacheKey, JSON.stringify(cachedSession));
620+
621+
const getDocumentStub = Sinon.stub(defaultStorage, "getDocument");
622+
623+
await supertest
624+
.post(`/api/v1/${appTenant1.id}/${document1._id}/broadcast-signal`)
625+
.send(signalBody)
626+
.set("Authorization", tenantToken1)
627+
.set("Content-Type", "application/json")
628+
.expect(302);
629+
630+
assert(
631+
getDocumentStub.notCalled,
632+
"getDocument should not be called on a cache hit",
633+
);
634+
});
635+
636+
it("Cache hit with deleted session sentinel returns 404 without calling DB", async () => {
637+
const cacheKey = SessionHelper.generateCacheKey(
638+
appTenant1.id,
639+
document1._id,
640+
);
641+
const deletedSentinel = {
642+
ordererUrl: "",
643+
deltaStreamUrl: "",
644+
historianUrl: "",
645+
isSessionAlive: false,
646+
isSessionActive: false,
647+
};
648+
await defaultSessionCache.set(cacheKey, JSON.stringify(deletedSentinel));
649+
650+
const getDocumentStub = Sinon.stub(defaultStorage, "getDocument");
651+
652+
await supertest
653+
.post(`/api/v1/${appTenant1.id}/${document1._id}/broadcast-signal`)
654+
.send(signalBody)
655+
.set("Authorization", tenantToken1)
656+
.set("Content-Type", "application/json")
657+
.expect(404);
658+
659+
assert(
660+
getDocumentStub.notCalled,
661+
"getDocument should not be called on a cache hit",
662+
);
663+
});
664+
665+
it("Cache miss falls back to DB and returns 200", async () => {
666+
// Cache is empty — no set() call
667+
const getDocumentSpy = Sinon.spy(defaultStorage, "getDocument");
668+
669+
await supertest
670+
.post(`/api/v1/${appTenant1.id}/${document1._id}/broadcast-signal`)
671+
.send(signalBody)
672+
.set("Authorization", tenantToken1)
673+
.set("Content-Type", "application/json")
674+
.expect(200);
675+
676+
assert(
677+
getDocumentSpy.calledOnce,
678+
"getDocument should be called once on a cache miss",
679+
);
680+
});
681+
});
682+
525683
describe("/documents", () => {
526684
it("/:tenantId/:id", async () => {
527685
await supertest

‎server/routerlicious/packages/routerlicious-base/src/utils/index.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,10 @@ export { Constants } from "./constants";
77
export { createDocumentRouter, type IPlugin } from "./documentRouter";
88
export { catch404, handleError } from "./middleware";
99
export { getIdFromRequest, getTenantIdFromRequest } from "./params";
10-
export { getSession, generateCacheKey, setGetSessionResultInCache } from "./sessionHelper";
10+
export {
11+
getSession,
12+
generateCacheKey,
13+
getSessionFromCache,
14+
setGetSessionResultInCache,
15+
} from "./sessionHelper";
1116
export { configureThrottler } from "./throttling";

‎server/routerlicious/packages/routerlicious-base/src/utils/sessionHelper.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ export function generateCacheKey(tenantId: string, documentId: string): string {
284284
* If the document is deleted, we store a session object with empty URLs and isSessionAlive=false.
285285
* @internal
286286
*/
287-
async function getSessionFromCache(
287+
export async function getSessionFromCache(
288288
tenantId: string,
289289
documentId: string,
290290
redisCacheForGetSession: ICache,

0 commit comments

Comments
 (0)