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

Adding REDIS as cache with AS4 fails #7416

Closed
pkellner opened this issue Mar 3, 2023 · 13 comments
Closed

Adding REDIS as cache with AS4 fails #7416

pkellner opened this issue Mar 3, 2023 · 13 comments

Comments

@pkellner
Copy link
Contributor

pkellner commented Mar 3, 2023

Issue Description

Taking the basic codesandbox example from the AS4 docs getting started and making very minor changes to support Redis, no redis key/values get inserted when a very simple cache control is added. Below is the updated index.ts.

I created a github repo with the main branch being the unchanged codesandbox demo. I then made very few changes to add Redis cache, assuming there is a localhost:6379 redis server responding. The app works, I verified that the cache hint is set, yet no key is added to the redis db.

Here is the GitHub repo. https://github.com/pkellner/as4-redis-cache-broken
The main branch is exactly what is in AS4 docs with no change.
the branch named bug-add-redis-fail has the changed code that should add key/value to Redis and it does not.
https://github.com/pkellner/as4-redis-cache-broken/tree/bug-add-redis-fail

import { ApolloServer } from "@apollo/server";
import { startStandaloneServer } from "@apollo/server/standalone";
import Keyv from "keyv";
import { KeyvAdapter } from "@apollo/utils.keyvadapter";

// A schema is a collection of type definitions (hence "typeDefs")
// that together define the "shape" of queries that are executed against
// your data.
const typeDefs = `#graphql
  # Comments in GraphQL strings (such as this one) start with the hash (#) symbol.

  # This "Book" type defines the queryable fields for every book in our data source.
  type Book {
    title: String
    author: String
  }

  # The "Query" type is special: it lists all of the available queries that
  # clients can execute, along with the return type for each. In this
  # case, the "books" query returns an array of zero or more Books (defined above).
  type Query {
    books: [Book]
  }
`;

const books = [
  {
    title: "The Awakening",
    author: "Kate Chopin",
  },
  {
    title: "City of Glass",
    author: "Paul Auster",
  },
];

// Resolvers define the technique for fetching the types defined in the
// schema. This resolver retrieves books from the "books" array above.
const resolvers = {
  Query: {
    // @ts-ignore
    books: (parent, args, contextValue, info) => {
      console.log("books");
      info.cacheControl.setCacheHint({ maxAge: 60, scope: "PRIVATE" });
      return books;
    },
  },
};

// The ApolloServer constructor requires two parameters: your schema
// definition and your set of resolvers.
const server = new ApolloServer({
  typeDefs,
  resolvers,
});

// Passing an ApolloServer instance to the `startStandaloneServer` function:
//  1. creates an Express app
//  2. installs your ApolloServer instance as middleware
//  3. prepares your app to handle incoming requests
const keyV = new Keyv(`redis://localhost:6379`);
const { url } = await startStandaloneServer(server, {
  // @ts-ignore
  cache: new KeyvAdapter(keyV),
  cacheControl: {
    defaultMaxAge: 5,
  },
  listen: { port: 4000 },
});

console.log(`🚀 Server listening at: ${url}`);

Link to Reproduction

https://github.com/pkellner/as4-redis-cache-broken/tree/bug-add-redis-fail

Reproduction Steps

execute this query:

{
  books {
     author
     title
  }
}

and no values are added to the Redis store.

@trevor-scheer
Copy link
Member

Hey @pkellner. Taking a look at your updated snippet:

const { url } = await startStandaloneServer(server, {
  // @ts-ignore
  cache: new KeyvAdapter(keyV),
  cacheControl: {
    defaultMaxAge: 5,
  },
  listen: { port: 4000 },
});

It looks like you're crossing some wires. cache and cacheControl aren't options you pass to startStandaloneServer (Why are you @ts-ignoreing that? The error you're getting there is telling you it's wrong. Is there a broken example you're following from somewhere?).

I recommend following the docs for setting up cache control. Please let me know if there's anything lacking there to get you on your way!
https://www.apollographql.com/docs/apollo-server/api/plugin/cache-control
https://www.apollographql.com/docs/apollo-server/performance/caching

@trevor-scheer
Copy link
Member

I see based on your community post that you're coming from AS2. I'm not sure how far along you already are in your migration but I'd recommend migrating to v3 first, then v4 if you're running into difficulties.
https://www.apollographql.com/docs/apollo-server/v3/migration
https://www.apollographql.com/docs/apollo-server/migration

@pkellner
Copy link
Contributor Author

pkellner commented Mar 3, 2023

@trevor-scheer I'll try again with your suggestions later today. No example, I was making it up as I went. Thanks for the suggestions. I'll review the docs I got lost in and let you know if I see something that might make it easy for others to not get lost like I did.

@pkellner
Copy link
Contributor Author

pkellner commented Mar 3, 2023

@trevor-scheer I've removed the ts-ignores and followed the link for configuring an external keyv store and I still get nothing output. If I'm not mistaken, the plugin only would help with what gets written to headers?

I'm now setting the cache: from inside the ApolloServer call. I've updated the some broken branch. I'm also not finding an example (beyond some code snippet suggestions) that has caching enabled. That would be helpful. If caching is actually working as it should, I assume I'm only missing a line or 2,I just can't figure out what that line is.

Here is the updated branch: https://github.com/pkellner/as4-redis-cache-broken/tree/bug-add-redis-fail
and here are the docs I'm following: https://www.apollographql.com/docs/apollo-server/performance/cache-backends#configuring-external-caching

import { ApolloServer } from "@apollo/server";
import { startStandaloneServer } from "@apollo/server/standalone";
import Keyv from "keyv";
import { KeyvAdapter } from "@apollo/utils.keyvadapter";

// A schema is a collection of type definitions (hence "typeDefs")
// that together define the "shape" of queries that are executed against
// your data.
const typeDefs = `#graphql
  # Comments in GraphQL strings (such as this one) start with the hash (#) symbol.
  
  enum CacheControlScope {
    PUBLIC
    PRIVATE
  }

  directive @cacheControl(
    maxAge: Int
    scope: CacheControlScope
    inheritMaxAge: Boolean
  ) on FIELD_DEFINITION | OBJECT | INTERFACE | UNION

  # This "Book" type defines the queryable fields for every book in our data source.
  type Book @cacheControl(maxAge: 240) {
    title: String
    author: String
  }

  # The "Query" type is special: it lists all of the available queries that
  # clients can execute, along with the return type for each. In this
  # case, the "books" query returns an array of zero or more Books (defined above).
  type Query {
    books: [Book]
  }
`;

const books = [
  {
    title: "The Awakening",
    author: "Kate Chopin",
  },
  {
    title: "City of Glass",
    author: "Paul Auster",
  },
];

// Resolvers define the technique for fetching the types defined in the
// schema. This resolver retrieves books from the "books" array above.
const resolvers = {
  Query: {
    books: (parent, args, contextValue, info) => {
      info.cacheControl.setCacheHint({ maxAge: 60, scope: "PRIVATE" });
      return books;
    },
  },
};

// The ApolloServer constructor requires two parameters: your schema
// definition and your set of resolvers.
const keyV = new Keyv(`redis://localhost:6379`);
const server = new ApolloServer({
  typeDefs,
  resolvers,
  cache: new KeyvAdapter(new Keyv()),
});

// Passing an ApolloServer instance to the `startStandaloneServer` function:
//  1. creates an Express app
//  2. installs your ApolloServer instance as middleware
//  3. prepares your app to handle incoming requests

const { url } = await startStandaloneServer(server, {
  listen: { port: 4000 },
});

console.log(`🚀 Server listening at: ${url}`);

@trevor-scheer
Copy link
Member

Looks like you're passing new Keyv() to the KeyvAdapter instead of your redis-configured keyV.

@pkellner
Copy link
Contributor Author

pkellner commented Mar 3, 2023

Good catch, but still no joy :(.

// The ApolloServer constructor requires two parameters: your schema
// definition and your set of resolvers.
const keyV = new Keyv(`redis://localhost:6379`);
const server = new ApolloServer({
  typeDefs,
  resolvers,
  cache: new KeyvAdapter(keyV),
});

@pkellner
Copy link
Contributor Author

pkellner commented Mar 3, 2023

Is a plugin need to make Cache work in my case? I assumed not, but I don't really get what is going on inside the server.

@trevor-scheer
Copy link
Member

I'm sorry I didn't connect these dots sooner, though I guess we still made a lot of progress.

Cache control is for client-side caching. Apollo Server will use the cache control values determined by your static and dynamic cache hints in order to calculate cache-control header information to send back to your cache-enabled client (like Apollo Client).

If you're still interested in server-side caching responses in memory check out these docs:
https://www.apollographql.com/docs/apollo-server/performance/caching#caching-with-responsecacheplugin-advanced

@pkellner
Copy link
Contributor Author

pkellner commented Mar 4, 2023

@trevor-scheer I'm not connecting the dots. I really need more than links. I've read these links over and over and made a very straight forward simple example which I think should work but is obviously wrong somehow or your server is broken. I've only ever been asking about server side cache coming from a resolver like the one I have in my simple example here:

const resolvers = {
  Query: {
    books: (parent, args, contextValue, info) => {
      info.cacheControl.setCacheHint({ maxAge: 60, scope: "PRIVATE" });
      return books;
    },
  },
};

If you have an example that shows a simple AS4 server using an external cache, I'd be happy to look at that, but without that I am very lost on how to make this work.

Please look at the very simple code I posted in the repo https://github.com/pkellner/as4-redis-cache-broken/tree/bug-add-redis-fail and let me know what code I need to add or change to make this trivial example with with AS4. I made sure to provide a very simple example.

@trevor-scheer
Copy link
Member

The link was helpful - what I failed to explain earlier was that the type of caching you were actually using didn't match what you were expecting. Cache control, by itself, will only send browser-style caching headers to your client (which your client may then make use of in order to cache responses). The plugin response cache which I linked to will additionally cache responses in the server's cache when appropriate.

Here's a codesandbox which demonstrates how to use the response cache plugin. I've also implemented a logging cache so you can see what the cache is up to whenever you send a request.

If you're going to leverage response caching like this, I strongly encourage a thorough read of the docs I linked most recently. The response cache has gotchas and a number of configuration options.

https://codesandbox.io/s/jolly-cloud-5qum1i

@pkellner
Copy link
Contributor Author

pkellner commented Mar 4, 2023

Thanks. It seems I was missing the plug-in. I wish that example was in the docs.

Thanks, and thanks for your patience also with me.

@trevor-scheer
Copy link
Member

@pkellner that's good feedback, I've opened an issue. I know I won't be able to get to it any time soon, so if you become an expert and feel compelled to help the next person I'd welcome a PR to improve our docs. Cheers!

pkellner added a commit to pkellner/apollo-server that referenced this issue Mar 4, 2023
I believe that the code in the "Single instance" that calls for redis will not work without the plugin being defined. I've added that along with an appropriate note. Please check me if my assumption is correct. That is, the current code will never work as is.

Also, removed in two places a specific reference to Apollo Server 4 since this should only show up in the Apollo Server 4 doc tree. I assume that when Apollo Server 5 comes out, you do not want to have to go back and change that note (and so on and so forth for every version change)

Note: I assume this change should also be required for the Redis Sential and the Redis Cluster examples, but as this behavior is opaque to me, I really don't know if that is necessary. I would suggest also adding a comment regarding what is really happening and when the responseCachePlugin is needed. I had thoroughly read these doc pages before suggestion this and only know about because of thishttps://github.com/apollographql/issues/7416 discussed with @trevor-scheer.

Another point of confusion for me is that it seems that this section "Cache backends" requires that you have read thoroughly the previous section "Caching" where responseCachePlugin is discussed.  I had read that several times.

It currently says:

You can cache Apollo Server query responses in stores like Redis, Memcached, or Apollo Server's default in-memory cache. For more information, see Configuring cache backends.

And I believe it should say something like

The `responseCachePlugin()` is required to be passed as a plugin along with any KeyvAdapter instantiation. That is, in order to cache Apollo Server query responses in stores like Redis, Memcached, or Apollo Server's default in-memory cache. For more information, see Configuring cache backends.

If this is correct, I'd be happy to update that text, but I feel a little bit like I'm must making stuff up now.
@pkellner
Copy link
Contributor Author

pkellner commented Mar 4, 2023

Hi @trevor-scheer . I'm definitely (and obviously) not an AS4 expert, but have done a lot docs in my career (mostly on docs.microsoft.com). In general, I love Apollo docs. GraphQL in general is very complex and documenting that well must be really hard. As mentioned above, I did do a pull request with lots of comments. My feelings will not be hurt if you don't accept it. I can be dumb as a door sometimes. (and forgot to mention your issue in the PR :( )

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants