-
Notifications
You must be signed in to change notification settings - Fork 61
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.
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
feat: introduce persistent cache for providers #1585
feat: introduce persistent cache for providers #1585
Conversation
I need to finish unit tests but the implementation can already get the initial reviews |
@@ -28,3 +28,8 @@ type Impossible<K extends keyof any> = { | |||
[P in K]: never; | |||
}; | |||
export type NoExtraProperties<T, U> = U & Impossible<Exclude<keyof U, keyof T>>; | |||
|
|||
export type Cache<T> = { | |||
get(key: string): Promise<T | undefined>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I parametrised only the value type. The extension storage allows only for string keys.
|
||
const sizeOfChunkToBePurged = 0.1; | ||
|
||
const isGetBytesInUsePresent = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The webextension-polyfill
does not list getBytesInUse
method in the storage API. With this type guard we ensure the getBytesInUse
is present.
for (const key of mostDatedKeysToPurge) { | ||
delete metadata[key]; | ||
} | ||
await storage.local.set({ [metadataKey]: metadata }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In worst-case scenario we will evict an item that was accessed right before the eviction logic started - accessing and evicting happening simultaneously:
- item X has an old
accessTime
- item X gets accessed, but its
accessTime
update is only scheduled - the eviction logic kicks in and accesses the metadata where X has old
accessTime
- X gets clasified to be evicted
- X gets
accessTime
updated in the metadata - X gets removed.
- X's metadata gets removed.
Another scenario: eviction starts first and simultaneously accessing happens
- metadata with old X's
accessTime
queried and X classified for eviction - X accessed and its
accessTime
update scheduled - X removed
- X's
accessTime
updated - X's updated metadata removed
d497913
to
0bc2019
Compare
0bc2019
to
d412ba9
Compare
d412ba9
to
7b53ece
Compare
|
||
void (async () => { | ||
await updateAccessTime(itemKey); | ||
if (await isQuotaExceeded()) await evict(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we check exceeded quota before setting item, so we know we have room to insert it?
Or maybe we can do try ... catch and handle evict
in catch, and after re do loaded.set in finally
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had a conversation with Piotr and agreed we dont want to catch any error here considering we are gonna have unlimited storage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @szymonmaslowski 👏
- define a contract of the cache - enable chain history and utxo providers to use injected cache - create persistent cache storage - utilizing web extension storage - implementing the cache contract - having data envicting capability
7b53ece
to
e39bf3d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏🏻 Great job @szymonmaslowski!
Context
We want to improve caching mechanism by making it persistent and therefore decrease the number of requests made to blockfrost
Proposed Solution
Allow utxo and chain history providers to use persistent cache
Important Changes Introduced
The eviction mechanism
accessTime
stored in metadatagetBytesInUse
is not present - it is not present in the MDN docs neither in the polyfill lib we use (recent version)fallbackMaxCollectionItemsGuard
parameterExample of data managed by the persistent cache implementation. Resouce name:
utxo-provider-cache
.Result of
storage.local.get()
:Reasons behind that structure:
storage.local.getBytesInUse([...allKeys])
)