Pluggable / disabled documentStore

[owenallenaz]

@glasser This originates from #5629 where there was discussion about making the documentStore pluggable. Before I submit a PR I would like to finalize the design of the feature to make sure we're on the same page.

Current Problem

This is what we're doing in our application now, it's dirty, and reaching into the private variables of ApolloServer and I'd rather not, but it's a neccessity because we have 20 ApolloServers that largely share query signatures running in the same Node process.

The sharedDocumentStore is initialized early in the App, and then we hit code where we loop and build our 20 ApolloServer's.

const sharedDocumentStore = new InMemoryLRUCache({
	maxSize: Math.pow(2, 20) * 30,
	sizeCalculator: function (obj) {
		return Buffer.byteLength(JSON.stringify(obj), 'utf8');
	}
});

for (let schema of schemas) {
	const server = new ApolloServer({
		schema,
		context : ({ req }) => {
			return {
				headers : req.headers
			}
		},
		stopOnTerminationSignals : false
	});

	const schemaDerivedData = await server.schemaDerivedData;
	schemaDerivedData.documentStore = sharedDocumentStore;
}

In this case, I'm swapping out the internal private variable so that we can re-use the store for all of our servers. Again, it's dirty, and I'd rather not.

Proposal

The contract I was looking to change, and seems pretty easy to change in the code is something like:

new ApolloServer({
  ...args,
  documentStore
});

With the config argument documentStore accepting a more pluggable storage definition in TypeScript like:

interface PluggableDocumentStore<T> {
    set(key: string, val: T): void
    get(key: string): T
}

Then we'd do a small alteration in generateSchemaDerivedData where we use the passed documentStore or we initialize the default.

private generateSchemaDerivedData(schema: GraphQLSchema): SchemaDerivedData {
    const schemaHash = generateSchemaHash(schema!);

    // Use a passed in documentStore or the default
    const documentStore = this.config.documentStore || this.initializeDocumentStore();

    return {
      schema,
      schemaHash,
      documentStore,
    };
  }

There is a comment in the code indicates that there was possibly a desire to disable the documentStore This cannot currently be disabled. If that's the case, it's possible that we could do documentStore : PluggableDocumentStore<DocumentNode> | boolean and if it's false make the code updates needed to effectively skip the pathway. The assumption being that undefined or true means use the default, false means no store, or a passed in value means use the passed in value.

I can submit a new PR with the changes once we decide on the design you think is appropriate.

[glasser]

What are the differences between your ApolloServers? Do they share the same schema?

[owenallenaz]

Yup, that's a problem. I've verified with my schema that the validation can result in some weird circumstances. Basically if a field is required in one schema, but not another, then the same query string might work in one case but fail on a diff schema. I'll fix that on my end.

Given that issue, I think this idea still has merit. In this case what I'd want to do is pass my own { get, set } which would handle prefixing per downstream schema. This would still allow me to share the LRU cache size across all of the servers, so that way this Node process wouldn't exceed 30mb total cache, instead of the potentiality for 600mb cache.

So the issues with my hackery aside in my personal app, are you OK with the proposal from my original item in terms of the implementation plan?

[glasser]

Yes, I think it would be fine, though we would just use KeyValueCache rather than a new abstraction — Apollo Server already questionably has one ecosystem of cache implementations, we don't need another.

The docs would have to be very clear that you should not actually share a store across servers.

[owenallenaz]

Ok, will do. I'll begin work on the PR and then we'll go from there. Thanks.

[glasser]

Merged in #5644

[glasser]

Unfortunately, if this feature was used in Apollo Gateway (ie, any new ApolloServer call receiving both non-null/undefined documentStore and gateway options), it could lead to a bug where invalid operations could be executed after a schema update. If you are using this feature in a Gateway, we highly recommend upgrading to v3.6.6 which fixes the issue.

The fix is that we now always prefix keys in the document store with a random string that is refreshed each time a new schema is received. We apply this fix in the non-Gateway case too. The fix to the issue also means that the caveat in the docs about not sharing a documentSource across ApolloServer objects has been removed, as this random prefix helps with that case as well.