Skip to content

Commit 578ce59

Browse files
rahuls-dbIsaac
andcommitted
Make the Reyden cache presence-based, dropping the dead tri-state
markReyden was the only writer and only ever stored isReyden: true, so the CacheEntry.isReyden field and isKnownReyden's boolean | undefined return type carried an unreachable "known not Reyden" state. Drop the field and return a plain boolean: an unexpired entry means Reyden, its absence means not known. Callers already used the result in a boolean context. Update the tests that asserted the old undefined return to expect false. Co-authored-by: Isaac <no-reply@databricks.com> Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com>
1 parent d98b69f commit 578ce59

3 files changed

Lines changed: 16 additions & 16 deletions

File tree

lib/ReydenWarehouseCache.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ const TTL_MS = 6 * 60 * 60 * 1000; // 6 hours
1818

1919
interface CacheEntry {
2020
timestamp: number;
21-
isReyden: boolean;
2221
}
2322

2423
class ReydenWarehouseCache {
@@ -54,23 +53,27 @@ class ReydenWarehouseCache {
5453

5554
/**
5655
* Checks if a warehouse is known to be Reyden (requiring SEA fallback).
57-
* Returns undefined if the warehouse is not in the cache or the entry has expired.
56+
*
57+
* Membership is presence-based: the cache only ever records known-Reyden
58+
* warehouses (via markReyden), so an unexpired entry means Reyden and the
59+
* absence of one means "not known" — there is no negative-cache state.
60+
* Returns false when the warehouse is not in the cache or the entry expired.
5861
*/
59-
public isKnownReyden(host: string, warehouseId: string): boolean | undefined {
62+
public isKnownReyden(host: string, warehouseId: string): boolean {
6063
const key = this.getKey(host, warehouseId);
6164
const entry = this.cache.get(key);
6265

6366
if (!entry) {
64-
return undefined;
67+
return false;
6568
}
6669

6770
// Opportunistically evict expired entries on access
6871
if (this.isExpired(entry)) {
6972
this.cache.delete(key);
70-
return undefined;
73+
return false;
7174
}
7275

73-
return entry.isReyden;
76+
return true;
7477
}
7578

7679
/**
@@ -91,10 +94,7 @@ class ReydenWarehouseCache {
9194
}
9295
}
9396

94-
this.cache.set(this.getKey(host, warehouseId), {
95-
timestamp: now,
96-
isReyden: true,
97-
});
97+
this.cache.set(this.getKey(host, warehouseId), { timestamp: now });
9898
}
9999

100100
/**

tests/unit/thrift-backend/ReydenThriftRecovery.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ describe('Reyden Warehouse Cache', () => {
4646
const host = 'example.com';
4747
const warehouseId = 'warehouse-123';
4848

49-
expect(reydenCache.isKnownReyden(host, warehouseId)).to.be.undefined;
49+
expect(reydenCache.isKnownReyden(host, warehouseId)).to.be.false;
5050
reydenCache.markReyden(host, warehouseId);
5151
expect(reydenCache.isKnownReyden(host, warehouseId)).to.be.true;
5252
});
@@ -66,7 +66,7 @@ describe('Reyden Warehouse Cache', () => {
6666
reydenCache.markReyden(host, 'warehouse-1');
6767

6868
expect(reydenCache.isKnownReyden(host, 'warehouse-1')).to.be.true;
69-
expect(reydenCache.isKnownReyden(host, 'warehouse-2')).to.be.undefined;
69+
expect(reydenCache.isKnownReyden(host, 'warehouse-2')).to.be.false;
7070
});
7171

7272
it('should isolate entries by host', () => {
@@ -75,7 +75,7 @@ describe('Reyden Warehouse Cache', () => {
7575
reydenCache.markReyden('host1.com', warehouseId);
7676

7777
expect(reydenCache.isKnownReyden('host1.com', warehouseId)).to.be.true;
78-
expect(reydenCache.isKnownReyden('host2.com', warehouseId)).to.be.undefined;
78+
expect(reydenCache.isKnownReyden('host2.com', warehouseId)).to.be.false;
7979
});
8080

8181
it('should have cache size method', () => {
@@ -95,7 +95,7 @@ describe('Reyden Warehouse Cache', () => {
9595

9696
reydenCache.clear();
9797
expect(reydenCache.size()).to.equal(0);
98-
expect(reydenCache.isKnownReyden('host1.com', 'warehouse-1')).to.be.undefined;
98+
expect(reydenCache.isKnownReyden('host1.com', 'warehouse-1')).to.be.false;
9999
});
100100
});
101101

@@ -124,7 +124,7 @@ describe('Reyden Warehouse Cache', () => {
124124

125125
// One tick past the TTL: expired, evicted on access.
126126
clock.tick(1);
127-
expect(reydenCache.isKnownReyden(host, warehouseId)).to.be.undefined;
127+
expect(reydenCache.isKnownReyden(host, warehouseId)).to.be.false;
128128
expect(reydenCache.size()).to.equal(0);
129129
});
130130

tests/unit/thrift-backend/ReydenThriftRecoveryOrchestration.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ describe('Reyden Thrift Auto-Recovery — Orchestration', () => {
104104

105105
expect(thrown).to.equal(genericError);
106106
expect(kernelStub.called).to.be.false; // No kernel fallback for a non-Reyden error.
107-
expect(reydenCache.isKnownReyden(HOST, WAREHOUSE_ID)).to.be.undefined; // Not marked.
107+
expect(reydenCache.isKnownReyden(HOST, WAREHOUSE_ID)).to.be.false; // Not marked.
108108
});
109109

110110
it('preserves the Thrift rejection as cause when the kernel fallback also fails', async () => {

0 commit comments

Comments
 (0)