Add cache option to config to expose ApolloGateway's queryPlanStore

[AneethAnand]

Fixes #2308
Fixes #1034

[netlify]

👷 Deploy request for apollo-federation-docs pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	8734cc9

[changeset-bot]

🦋 Changeset detected

Latest commit: 3fd50cd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages

Name	Type
@apollo/query-planner	Minor
@apollo/gateway	Minor
@apollo/federation-internals	Minor
@apollo/composition	Minor
@apollo/query-graphs	Minor
@apollo/subgraph	Minor
apollo-federation-integration-testsuite	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

[korinne]

Thanks again for opening this PR. We've put in on our review backlog, and will provide feedback shortly.

[trevor-scheer]

Hey @AneethAnand, I think this is a reasonable change in concept. I have one major concern with the PR, and that's the use of LRUCache.

Since we're going to be providing this as a configuration point for users, the interface should be more generic than "an LRUCache from the lru-cache library". We actually have a very similar concern in Apollo Server, where we use the KeyValueCache interface from the @apollo/utils.keyvaluecache package.

My recommendation is this:

• provide the queryPlanConfig.cache interface as a type QueryPlanCache = KeyValueCache<QueryPlan> & { clear: () => void }
• use the InMemoryLRUCache concrete class (also exported by the @apollo/utils.keyvaluecache package) for the default implementation - this cache wraps lru-cache and implements the QueryPlanCache type.

I'm happy to implement these changes if you'd prefer (just let me know you don't mind me pushing to your branch if so).

[AneethAnand]

@trevor-scheer Thanks for the review comment. I had interface in the first place but it dint have the clear method and I dint think of this option

type QueryPlanCache = KeyValueCache<QueryPlan> & { clear: () => void }

.Also I believe the set method also was not compatible. I will give it a try and push my change again.

[AneethAnand]

HI @trevor-scheer I have incorporated your review comments and pushed a commit to my branch.
AneethAnand@5bf44d7 for your perusal. Thanks!

[trevor-scheer]

This is exactly what I had in mind, thanks for implementing those changes. Just a few minor comments and this will be good to go!

• Can you change the target branch to next instead of main?
• Can you run npx changeset and add a minor entry which explains the new configuration option? This will end up in the gateway's changelog so please be descriptive for people who might be interested in using the new feature.

[AneethAnand]

This is exactly what I had in mind, thanks for implementing those changes. Just a few minor comments and this will be good to go!

• Can you change the target branch to next instead of main?
• Can you run npx changeset and add a minor entry which explains the new configuration option? This will end up in the gateway's changelog so please be descriptive for people who might be interested in using the new feature.
  Minor version bump for the following ?
  ◯ @apollo/[email protected]
  ◯ @apollo/[email protected]
  I have updated the changeset and added "new file: .changeset/forty-gifts-burn.md". Not sure what the process.

[clenfest]

lgtm!

[trevor-scheer]

Thanks @AneethAnand!

[AneethAnand]

@trevor-scheer QQ, when will this change get published, so that it could be consumed by us?

[trevor-scheer]

This change will land with v2.4.0 relatively soon, on the order of a couple weeks or so. In the meantime, you're welcome to use the build from your PR (look at the comment posted by the codesandbox bot).