Skip to content

cacheable - fix: size each key value in deleteMany stats #1742

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jaredwray merged 2 commits into from
Oct 18, 2026
+25 −10
Merged

cacheable - fix: size each key value in deleteMany stats #1742

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
33af2ab
cacheable - fix: size each key value in deleteMany stats (#1738)
claude Oct 17, 2026
eed4a05
cacheable - fix: count cached undefined values in deleteMany stats
claude Oct 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions packages/cacheable/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1155,10 +1155,17 @@ export class Cacheable extends Hookified {
const primaryBackfillBarrier = this.cancelPrimaryBackfills(keys);
await primaryBackfillBarrier;
if (this.stats.enabled) {
const statResult = (await this._primary.get(keys)) as unknown;
for (const key of keys) {
// Use the raw entries, because a cached value can be undefined.
const statResults = await this._primary.getManyRaw<unknown>(keys);
for (const [index, key] of keys.entries()) {
// Skip a key that is not in the cache. Size only the value of this key.
const statResult = statResults[index];
if (statResult === undefined) {
continue;
Comment thread
jaredwray marked this conversation as resolved.
Comment on lines +1163 to +1164

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Account for keys that remain in the secondary store

When the primary copy expires before the secondary copy, has(key) still reports that the key exists. This check skips the key, although deleteMany removes the secondary copy, so all four storage statistics remain stale. Check the secondary store before the code classifies the key as absent.

Useful? React with 👍 / 👎.

@jaredwray jaredwray Oct 17, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not change this in this PR. These are the reasons:

  • deleteInternal (packages/cacheable/src/index.ts:1098) also reads only the primary store. This PR keeps delete and deleteMany the same.
  • The stats do not decrease when a primary entry expires or when the LRU removes it. So the stats do not follow the secondary store in those paths either.
  • A secondary read adds a remote call to each deleteMany when stats are on.
  • A shared secondary store can hold keys that a different instance set. This instance did not count those keys, but the check would decrease the stats for them.

A stats model for two layers is a different issue from #1738. I leave this thread open for the maintainer.

Generated by Claude Code

}

this.stats.decreaseKSize(key);
this.stats.decreaseVSize(statResult);
this.stats.decreaseVSize(statResult.value);
this.stats.decreaseCount();
this.stats.incrementDeletes();
}
Expand Down
15 changes: 11 additions & 4 deletions packages/cacheable/test/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1095,12 +1095,19 @@ describe("cacheable stats enabled", async () => {
test("should get stats on delete many", async () => {
const cacheable = new Cacheable({ stats: true });
await cacheable.setMany([
{ key: "key1", value: "value1" },
{ key: "key2", value: "value2" },
{ key: "key1", value: { id: "value1" } },
{ key: "key2", value: undefined },
{ key: "key3", value: { id: "value3" } },
]);
await cacheable.deleteMany(["key1", "key2"]);
// The stats of a cache that holds only key3 are the expected result.
const expected = new Cacheable({ stats: true });
await expected.set("key3", { id: "value3" });

await cacheable.deleteMany(["key1", "key2", "missing"]);
expect(cacheable.stats.deletes).toBe(2);
expect(cacheable.stats.count).toBe(0);
expect(cacheable.stats.count).toBe(1);
expect(cacheable.stats.ksize).toBe(expected.stats.ksize);
expect(cacheable.stats.vsize).toBe(expected.stats.vsize);
});
test("should get stats on clear", async () => {
const cacheable = new Cacheable({ stats: true });
Expand Down
7 changes: 4 additions & 3 deletions packages/utils/src/stats.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,8 @@ export class Stats {
}

public roughSizeOfObject(object: any): number {
const objectList: any[] = [];
// Use a Set so that the visited check is O(1) and the full walk stays linear.
const objectList = new Set<any>();
const stack: any[] = [object];
let bytes = 0;

Expand All @@ -504,12 +505,12 @@ export class Stats {
}

// Check for circular references - if this object was already processed, skip it
if (objectList.includes(value)) {
if (objectList.has(value)) {
continue;
}

// object as default
objectList.push(value);
objectList.add(value);

// Estimate object overhead, and then recursively estimate the size of properties
for (const key in value) {
Expand Down
Loading