Skip to content

Update driver cache entries to contain the file version #25337

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
+126 −6
Open

Update driver cache entries to contain the file version #25337

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
97e3aeb
Update cache entry syntax and fix typos
jzaffiro Aug 28, 2025
7972883
Use snapshotKey instead of string
jzaffiro Aug 28, 2025
a5f5ac5
Format snapshots with loading group ids the same as plain snapshots
jzaffiro Aug 28, 2025
dc402b0
Use loading group id and update fluid cache entry function
jzaffiro Aug 28, 2025
72ba0bc
Update suffix to give the same value as the old function
jzaffiro Aug 28, 2025
ae735ef
Use switch statement to clarify entry format
jzaffiro Aug 28, 2025
04c8817
Apply pr feedback
jzaffiro Sep 2, 2025
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
9 changes: 7 additions & 2 deletions packages/drivers/odsp-driver-definitions/src/odspCache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export interface IPersistedCache {
put(entry: ICacheEntry, value: any): Promise<void>;

/**
* Removes the entries from the cache for given parametres.
* Removes the entries from the cache for given parameters.
* @param file - file entry to be deleted.
*/
removeEntries(file: IFileEntry): Promise<void>;
Expand All @@ -127,5 +127,10 @@ export interface IPersistedCache {
* @internal
*/
export function getKeyForCacheEntry(entry: ICacheEntry): string {
return `${entry.file.docId}_${entry.type}_${entry.key}`;
const version =
"fileVersion" in entry.file.resolvedUrl && entry.file.resolvedUrl.fileVersion !== undefined
? `_${entry.file.resolvedUrl.fileVersion}`
: "";
const suffix = entry.type === "snapshot" ? "" : `_${entry.key}`;
return `${entry.file.docId}${version}_${entry.type}${suffix}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

This will cause a regression in load performance as we are changing the key corresponding to the stored snapshot. So the previous entries won't be found and instead of loading from cache, we will load from network for each document.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, the version will "" for snapshot key.

Copy link
Contributor

@jatgarg jatgarg Aug 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Can you before and after example of how would the key look for snapshot and ops?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the goal is to only change the cache entries for versioned snapshots/ops and leave the entries for the main document untouched.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok got it. So, when they will start supplying the version, then they will face some regression. You can let them know before hand, when they will start the rollout.

Copy link
Contributor Author

@jzaffiro jzaffiro Aug 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Examples with versions:
before:
docId_snapshot_4.0
docId_ops_100_2
after:
docId_4.0_snapshot
docId_4.0_ops_100_2

If there is a version value in entry.key for a snapshot entry, it will also be accessible from the resolved url, and will end up in the version variable in the updated function.

}
2 changes: 1 addition & 1 deletion packages/drivers/odsp-driver/src/odspDocumentService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ export class OdspDocumentService
return this._opsCache;
}

// Called whenever re receive ops through any channel for this document (snapshot, delta connection, delta storage)
// Called whenever we receive ops through any channel for this document (snapshot, delta connection, delta storage)
// We use it to notify caching layer of how stale is snapshot stored in cache.
protected opsReceived(ops: ISequencedDocumentMessage[]): void {
// No need for two clients to save same ops
Expand Down
Loading