Skip to content

feat(idempotency): add local cache to BasePersistenceLayer #1396

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

Merged
merged 3 commits into from
Apr 7, 2023

Conversation

dreamorosi
Copy link
Contributor

@dreamorosi dreamorosi commented Apr 4, 2023

Description of your changes

This PR introduces a Least Recently Used (LRU) cache to the BasePersistenceLayer of the upcoming Idempotency utility. In this type of cache the most recently-used items are kept in the cache while older ones, that have been accessed less recently are evicted as new items come in.

Users can configure any persistence layer that extends the BasePersistenceLayer to use this cache, and in doing so they can also specify the maximum amount of items that are kept in the cache:

const persistenceLayer = new MyPersistenceLayer(); // i.e. DynamoDB
persistenceLayer.configure({
  config: new IdempotencyConfig({
    useLocalCache: true, // defaults to `false`
    maxLocalCacheSize: 50, // defaults to `256`
  }),
});

When caching is enabled, the persistence layer stores, retrieves, and deletes idempotency records from the cache.

Notes on the LRUCache implementation

As discussed in the linked issue, contrary to Python, Node.js doesn't have any LRU Cache implementation in its standard library. Prior to working the PR I have researched popular modules and found two: lru-cache (123M weekly downloads) and lru_map (3.7M weekly downloads).

While both libraries are popular, the lru-cache one offers a wide range of features like TTL, retrieval functions from remote, and a host of other features that we weren't going to be using in this utility. This, in addition to the module size (638 kB unpacked - vs ~150 kB on average on our side), made me opt for the less popular one.

When looking at the lru_map module, I realized that the recommended usage stated this:

Recommended: Copy the code in lru.js or copy the lru.js and lru.d.ts files into your source directory. For minimal functionality, you only need the lines up until the comment that says "Following code is optional".

For this reason I have opted for reimplementing the module directly in our repo. The resulting implementation follows loosely the one found in the lru_map module in terms of flow, but adds type annotations, comments, unit tests and retains only the bare minimum methods/features that are needed for Idempotency to work.

The result of this work can be found at packages/idempotency/src/persistence/LRUCache.ts and the license as well as a direct mention to the original implementation and recommended usage are included in the docstring of the class.

For now I have opted for putting the class in the Idempotency utility, however in the future we might consider extracting it in the commons package so that it can be used by other modules, as well as customers in their own code.

Once merged this PR closes #1299

Related issues, RFCs

Issue number: #1299

Checklist

  • My changes meet the tenets criteria
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in areas that should be flagged with a TODO, or hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding changes to the examples
  • My changes generate no new warnings
  • The code coverage hasn't decreased
  • I have added tests that prove my change is effective and works
  • New and existing unit tests pass locally and in Github Actions
  • Any dependent changes have been merged and published
  • The PR title follows the conventional commit semantics

Breaking change checklist

Is it a breaking change?: NO

  • I have documented the migration process
  • I have added, implemented necessary warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@pull-request-size pull-request-size bot added the size/XL PRs between 500-999 LOC, often PRs that grown with feedback label Apr 4, 2023
@dreamorosi dreamorosi self-assigned this Apr 4, 2023
@github-actions github-actions bot added the feature PRs that introduce new features or minor changes label Apr 4, 2023
@dreamorosi dreamorosi marked this pull request as ready for review April 5, 2023 08:32
@dreamorosi dreamorosi merged commit 2013ff2 into main Apr 7, 2023
@dreamorosi dreamorosi deleted the dreamorosi/issue1299 branch April 7, 2023 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature PRs that introduce new features or minor changes size/XL PRs between 500-999 LOC, often PRs that grown with feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: add local cache to Idempotency persistence layer
1 participant