dataloader and graphql subscriptions

[mattkrick]

dataloader is usually used on a per-execution basis (ie 1 call to a graphql execute). that's pretty useless on subscriptions. instead, subscriptions should span across users on a per-channel basis.

so, the only way i think we can do this is by mutating the context passed into a mutation.

• assuming a mutation on calls 1 graphql subscription channel, we can do something like context.loader[channel] = context.loader[channel] || new Dataloader(...)
• then, we'll need to flush it after the last iteration returns from graphql.subscribe
• a good example is a mutation that gets us the teamMemberId, but the subscription needs the teamMember object. getting it on the mutation side ruins the nice colocation. Also, something like addGitHubRepo has to fetch newRepo.users, which should also be colocated in the subscription, because there might come a time that the sub receives a call from a mutation & we don't need to listen to the users, so it'll be a wasted fetch.
• to flush after the last call, we can count the number on the pubsub implementation, since it tracks the callbacks in an array.
• we can begin counting in makingTheSubscriptionIter
• we can end the count there, too, and flush the repo on next tick (maybe wait a couple ticks that call won't resolve until the rest of the missing fields do.
• since the time is indeterminate, we'll need to find another way to catch the last one.
• we could grab the name from the sub when it gets established & then if the name matches the sentinel, we flush the cache.

[mattkrick]

closing because there is no straightforward path to doing this. the problem is we need to create and clear the cache from within a graphql resolve call. we can create it by seeing if it exists, and then creating it if it doesn't. however, if a mutation & a subscription call have payloads that share the same GraphQLObject, then we're not guaranteed we'll get everything we need in the source. a good example if the JoinIntegrationPayload. we want to fulfill teamMember, and we can do that by supplying teamMemberId to the source. otherwise, we'll have to look up the teammember based on the globalId, which means 1 more lookup. not the end of the world, since a dataloader could save us more than 1 load in the long run. however, when to destroy is a tricky one. we'd like to destroy after all the subscriptions have run. so, we could set a counter == subs.length & then after each resolve gets called, we subsLen--. however, that's difficult because it's all async, so a mutation might slip in the middle & then we'd clear the cache too soon. we could tweak the pubsub to add a onLastSub callback that runs after before the last sub gets called. that callback would have to mutate the context saying something like channelName: 'lastCall'. then, that resolve function would need to know the channel name it's working for, if any. to do that, we'd have to pass in channelName in the payload so it shows up in source of the resolve function. then, if lastCall is true, we know we can clear it.

[mattkrick]

another issue: ideally, the mutation could share the cached value (or at least set it, since it'll be guaranteed to run first). however, there is no easy way to determine that subscriptions will come after the mutation. so, we can't just store it in the same place like we would a subscription, because if a sub never comes, then it'll just live there forever. the only decent way around this is to build a TTL into it.

[mattkrick]

We finally have some cases where this could be beneficial (get the teamMember name for every archived project). for epic 7 it'll be comically inefficient, which will motivate us to do it right for epic 8

[mattkrick]

well my brain hurts, but i figured out how to do this w/o any changes to graphql:

we could approach this 2 ways:

• fetch the subsequent data in the mutation

  • PRO: easy for the subscriber. you pass the entire payload!
  • CON: messy. the mutation has to know what the subscriber wants so it can fetch those fields. it's possible that different subscribers want different things, so you have to grab everything any subscriber could want.
  • CON: assuming the subscriber wants exactly what the mutation wants, we'd have to call graphql's internal completeFields... from within the mutation. GraphQL doesn't give us everything we need (like the fieldDefs, resolveFn, etc) it'd require a PR to GraphQL that may not get approved.

• fetch the subsequent data in the subscription

  • PRO: doesn't care what the subscriber asks for. it just gives it to them
  • CON: slightly slower. even though it hits the dataloader cache, it still has to reassemble the data
  • CON: it requires mutating the context passed into the mutations & subscriptions.

I opted for the 2nd solution. Here's how it's implemented:

• When a mutation is going to post something to the subscription, it creates a dataloader in the context.subscriptionDataLoaders =. This mutates the object with a random key called mutationId and a value of a new DataLoader.
• It then passes in the mutationId to Redis so the subscription can look it up later
• Each subscription has a unique subscriberId. That lives as long as the subscription is alive
• Within the subscription, we see if the payload has a mutationId. if it does, then we add a row to the dataloader so we can easily get the mutationId from the subscriberId (since mutationId will not be present in the context)
• In fields where we use the dataloader, we see if there is a subscriberId so we know whether this is a subscription or not. if it is, we use that subscriberId to get the mutationId, and with that mutaitonId, we get the dataloader for that mutation.
• when an unsub occurs, we flush the lookups for that subscriberId.
• when a dataloader is created inside a mutation, we trigger it to self destruct within 10 seconds to prevent mem leaks.

[mattkrick]

Considerations:

• Mutation A of id 123 publishes a partial doc to a pub/sub. the subscriber with id 888 gets it & completes the doc with the 123 dataloader
• Mutation A of id 124 publishes a fresh doc and changes the reference for 888 so it points to 124 now before the message for 123 completes.
• worst case scenario: a double lookup. the document gets partially looked up on 123 and then partially looked up on 124.
• If we store this kinda thing for subscriptions, we could do the same for user queries.
• A user query could get invalidated by a mutation, so we wouldn't want it to live forever, unless we made sure we always flushed on mutation, but that'd be very tricky to do because any mutation could invalidate any dataloader.
• we could batch-send the queries to only make 1 network request. if we did that, then we could reuse the dataloader for all
• the mutation dataloader could be shared with subscriptions, too. that'd further reduce lookups by 1. the exception would be if if the mutation payload contained sensitive info that we did not want to share with the subscribers, because if there was, an attacker could expand the scope of the subscription payload to include something like secretToken that is a query that returns the secret token of the caller, (in this case the subscriber is using the cached value from the mutation). this would almost be impossible since subscribers get called first. i suppose a mutation could more easily request something secret from a subscriber, but again timing would be tough. we can completely avoid this if we separate dataloaders for folks who could grab sensitive info.

[mattkrick]

solved, implemented in projets-to-relay