Skip to content
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

Remove homepage projects and tags build-time caching #1200

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

brawaru
Copy link
Contributor

@brawaru brawaru commented Jun 13, 2023

Re-building and re-deploying the site every thirty minutes just to update data is very expensive and hurts user experience by constantly blowing out the client's cache without meaningful changes to the bundle. Even ignoring that, hardcoding changing data, even though it would be rebuilt eventually is considered a very bad practice. Overall, this all feels like a terrible hack.

This pull request should address this issue by removing client data generation logic and instead creating the supply logic using Nitro server cached routes that will refresh on the server every thirty minutes unless there's a re-deploy due to actual changes.

@brawaru
Copy link
Contributor Author

brawaru commented Jul 10, 2023

Redrafting to add support for persistent storage (KV) for workers and resolve conflicts.

@brawaru brawaru marked this pull request as draft July 10, 2023 18:22
@brawaru
Copy link
Contributor Author

brawaru commented Jul 10, 2023

It seems that this approach is not compatible with short-living workers or serverless functions:

  • You can, of course, use Workers KV, but reads on it will probably be quite expensive.
  • Also, when using serverless functions insead of workers, you cannot use KV bindings directly and have to rely on KV over HTTP, which doesn't have low latency benefit. There also seems to be a bug in Unstorage with KV over HTTP driver where it simply doesn't read nor store anything.
  • Using Redis could've been possible, but establishing connections may also get expensive and suboptimal.

So it seems there is no good solution to this problem, expect for directly contacting API to fetch tags and homepage projects, but this may be taxing on the API if no proper caching is implemented for these endpoints, there is also no /homepage API endpoint at this time.

Closing this pull request. Hopefully some better solution comes up in the future to replace the re-deployment hack.

@brawaru brawaru closed this Jul 10, 2023
@brawaru
Copy link
Contributor Author

brawaru commented Jul 13, 2023

Reopening as according to Geo KV won't cost much. I have implemented KV support for Cloudflare and Vercel, below are steps to set it up properly. If cache is not set up then it falls back to in-memory storage that is not persistent in workers. Additional persistent storage drivers should be relatively easy to add, check out Unstorage documentation for the list of all storage options and custom storage driver implementation steps.

Additionally also implemented a cached overlay driver that will store KV response in memory for the lifespan of a worker to prevent excessive and stale KV reads. Writes aren't immediately flushed with Cloudflare KV for whatever reason and may return stale data making Nitro re-compute the response and write it back to KV (effectively making caching ineffective), so having a cached overlay somewhat protects against this. #WondersOfModernWebDevelopment

Setting up Cloudflare KV (Pages)
  1. Open Workers & Pages Cloudflare Dashboard.
  2. Switch to KV page.
  3. Create a namespace, call it anyhow you like.
  4. Go back to Overview, and open Knossos application.
  5. Go to Settings → Functions, scroll until ‘KV namespace bindings’.
  6. Click Edit bindings, Add variable, call it whatever (e.g. CACHE), and set it to KV namespace you've created. Save.
  7. Now go to Environment variables. Edit variables for production, add below and save.
  • CACHE_STORAGE_OPTION = cloudflare-kv
  • CF_KV_BINDING_CACHE = <name of the binding variable>
  1. Retry latest deploy (if this PR was merged before setting up KV).

Note
To check that cache is functioning you can go to KV page again, click on the namespace, and check that keys like cache:nitro:handlers:homepage.<hash>.json, cache:nitro:handlers:tags.<hash>.json are present.


Setting up Cloudflare KV (Workers)
  1. Open Workers & Pages Cloudflare Dashboard.
  2. Switch to KV page.
  3. Create a namespace, call it anyhow you like.
  4. Copy ID of your namespace.
  5. Edit wrangler.toml to include:
     [[kv_namespaces]]
     binding = "<name of binding variable>"
     id = "<ID of the KV namespace>"
  6. Edit deployment workflow to build Nuxt with environment variables:
    • CACHE_STORAGE_OPTION = cloudflare-kv
    • CF_KV_BINDING_CACHE = <name of the binding variable>
  7. Push your changes and await for re-deployment.

Note
To check that cache is functioning you can go to KV page again, click on the namespace, and check that keys like cache:nitro:handlers:homepage.<hash>.json, cache:nitro:handlers:tags.<hash>.json are present.


Setting up Vercel KV
  1. Open Knossos project page.
  2. Switch to Storage tab.
  3. Press Connect Store / Create New Database.
  4. Choose KV (Durable Redis).
  5. Configure it how you want (e.g. by adding read replicas in other regions).
  6. On the next screen (when asked about environments and environment variables prefix) leave everything as is. Press Connect.
  7. Switch to Settings tab, go to Environment variables.
    1. Ensure that the following variables are now present:
      • KV_URL
      • KV_REST_API_URL
      • KV_REST_API_TOKEN
      • KV_REST_API_READ_ONLY_TOKEN
    2. Add variable CACHE_STORAGE_OPTION with value vercel-kv.
  8. Redeploy latest deployment (if this PR was merged before setting up KV).

Note
To check that cache is functioning you can go to Storage tab again, click on the KV database, and type KEYS cache* in the CLI. It should print something like cache:nitro:handlers:homepage.<hash>.json, cache:nitro:handlers:tags.<hash>.json.


Here's an example app running a similar configuration of KV:
https://github.com/brawaru/nuxt-cf-test-app

Live proof of work:

@brawaru brawaru reopened this Jul 13, 2023
@brawaru brawaru marked this pull request as ready for review July 13, 2023 23:22
@brawaru
Copy link
Contributor Author

brawaru commented Aug 22, 2023

Saw merge conflicts, tried to resolve them, but figured why not take the opportunity and decided to re-create this almost from scratch to make it much cleaner and maintainable 👍🏻

Pretty much ready for review

composables/shared-state.ts Outdated Show resolved Hide resolved
plugins/shared-state.ts Outdated Show resolved Hide resolved
@triphora
Copy link
Contributor

triphora commented Sep 2, 2023

We talked about this internally, and it's probably just better to convert it back to the old way of requesting everything on the client - ever since redis caching was added to the backend, it wouldn't affect performance any more or less than this would, from our understanding.

@brawaru
Copy link
Contributor Author

brawaru commented Sep 21, 2023

Addressed the concerns both of you had, @falseresync and @triphora. Reapplied simple strategy and just removed all of the build-time caching as you asked. There's no local caching or anything like that. Now the server will make a request and serve it to client as a payload, the client will refetch it every time homepage or Modrinth app page are open.

Since then I learnt a bit about ISR (Incremental Static Regeneration) which we could've used for the API routes (so they just regenerate every x minutes and in a meanwhile serve the same data to everyone), but Cloudflare Pages and Workers don't support this technology, so it essentially would end up what I made in the first iteration — caching with KV.

@brawaru brawaru changed the title Use server routes to supply client data Remove homepage projects and tags build-time caching Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants