Skip to content

Commit

Permalink
gateway: remove dependency on apollo-server-caching (#2029)
Browse files Browse the repository at this point in the history
The point of apollo-server-caching is to provide an abstraction over
multiple cache backends. Gateway is not using that abstraction; it's
just using one particular implementation, which is a wrapper around an
old version of lru-cache.

As part of apollographql/apollo-server#6057
and apollographql/apollo-server#6719 we want
to remove dependencies on Apollo Server from Apollo Gateway. Technically
we don't really need to remove this dependency (since
apollo-server-caching doesn't depend on anything else in AS) but
apollo-server-caching won't be updated any more (in fact, even in AS3 it
has already been replaced by `@apollo/utils.keyvaluecache`), so let's do
it.

While we're at it, we make a few other improvements:
- Ever since #440, the queryPlanStore field is always set, so we can
  remove some conditionals around it.
- Instead of using the old lru-cache@6 wrapped by the
  apollo-server-caching package, we use the newer lru-cache@7 (which
  improves the algorithm internally and changes the names of methods a
  bit).
- The get and set methods on InMemoryLRUCache were only async because
  they implement the abstract KeyValueCache interface: the
  implementations didn't actually do anything async. So we no longer
  need to await them or include a giant comment about how we're not
  awaiting them.
  • Loading branch information
glasser committed Aug 2, 2022
1 parent 6f6793e commit 173de85
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 56 deletions.
2 changes: 1 addition & 1 deletion gateway-js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@
"@opentelemetry/api": "^1.0.1",
"@types/node-fetch": "2.6.2",
"apollo-reporting-protobuf": "^0.8.0 || ^3.0.0",
"apollo-server-caching": "^0.7.0 || ^3.0.0",
"apollo-server-core": "^2.23.0 || ^3.0.0",
"apollo-server-errors": "^2.5.0 || ^3.0.0",
"apollo-server-types": "^0.9.0 || ^3.0.0",
"async-retry": "^1.3.3",
"loglevel": "^1.6.1",
"lru-cache": "^7.13.1",
"make-fetch-happen": "^10.1.2",
"pretty-format": "^28.0.0"
},
Expand Down
39 changes: 11 additions & 28 deletions gateway-js/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
GraphQLRequestContextExecutionDidStart,
} from 'apollo-server-types';
import type { Logger } from '@apollo/utils.logger';
import { InMemoryLRUCache } from 'apollo-server-caching';
import LRUCache from 'lru-cache';
import {
isObjectType,
isIntrospectionType,
Expand Down Expand Up @@ -128,7 +128,7 @@ export class ApolloGateway implements GraphQLService {
private serviceMap: DataSourceMap = Object.create(null);
private config: GatewayConfig;
private logger: Logger;
private queryPlanStore: InMemoryLRUCache<QueryPlan>;
private queryPlanStore: LRUCache<string, QueryPlan>;
private apolloConfig?: ApolloConfigFromAS3;
private onSchemaChangeListeners = new Set<(schema: GraphQLSchema) => void>();
private onSchemaLoadOrUpdateListeners = new Set<
Expand Down Expand Up @@ -210,14 +210,14 @@ export class ApolloGateway implements GraphQLService {
}

private initQueryPlanStore(approximateQueryPlanStoreMiB?: number) {
return new InMemoryLRUCache<QueryPlan>({
return new LRUCache<string, QueryPlan>({
// Create ~about~ a 30MiB InMemoryLRUCache. This is less than precise
// since the technique to calculate the size of a DocumentNode is
// only using JSON.stringify on the DocumentNode (and thus doesn't account
// for unicode characters, etc.), but it should do a reasonable job at
// providing a caching document store for most operations.
maxSize: Math.pow(2, 20) * (approximateQueryPlanStoreMiB || 30),
sizeCalculator: approximateObjectSize,
sizeCalculation: approximateObjectSize,
});
}

Expand Down Expand Up @@ -607,7 +607,7 @@ export class ApolloGateway implements GraphQLService {
// Once we remove the deprecated onSchemaChange() method, we can remove this.
legacyDontNotifyOnSchemaChangeListeners: boolean = false,
): void {
if (this.queryPlanStore) this.queryPlanStore.flush();
this.queryPlanStore.clear();
this.schema = toAPISchema(coreSchema);
this.queryPlanner = new QueryPlanner(coreSchema);

Expand Down Expand Up @@ -843,10 +843,7 @@ export class ApolloGateway implements GraphQLService {
return { errors: validationErrors };
}

let queryPlan: QueryPlan | undefined;
if (this.queryPlanStore) {
queryPlan = await this.queryPlanStore.get(queryPlanStoreKey);
}
let queryPlan = this.queryPlanStore.get(queryPlanStoreKey);

if (!queryPlan) {
queryPlan = tracer.startActiveSpan(
Expand All @@ -868,25 +865,11 @@ export class ApolloGateway implements GraphQLService {
},
);

if (this.queryPlanStore) {
// The underlying cache store behind the `documentStore` returns a
// `Promise` which is resolved (or rejected), eventually, based on the
// success or failure (respectively) of the cache save attempt. While
// it's certainly possible to `await` this `Promise`, we don't care about
// whether or not it's successful at this point. We'll instead proceed
// to serve the rest of the request and just hope that this works out.
// If it doesn't work, the next request will have another opportunity to
// try again. Errors will surface as warnings, as appropriate.
//
// While it shouldn't normally be necessary to wrap this `Promise` in a
// `Promise.resolve` invocation, it seems that the underlying cache store
// is returning a non-native `Promise` (e.g. Bluebird, etc.).
Promise.resolve(
this.queryPlanStore.set(queryPlanStoreKey, queryPlan),
).catch((err) =>
this.logger.warn(
'Could not store queryPlan' + ((err && err.message) || err),
),
try {
this.queryPlanStore.set(queryPlanStoreKey, queryPlan);
} catch (err) {
this.logger.warn(
'Could not store queryPlan' + ((err && err.message) || err),
);
}
}
Expand Down
35 changes: 8 additions & 27 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 173de85

Please sign in to comment.