benjie / ouch-my-finger

16 stars 19 forks source link

Subscriptions caching issue with List + Lambda #11

Closed gitrojones closed 5 months ago

gitrojones commented 5 months ago

Hello,

Here's a fork with reproduction of the subscription issue I was running into involving Lambda + List.

Setup:

  1. Setup .env, replacing DATABASE_URL with your own
  2. Setup the schema w/ the basic-schema.sql file or equivalent.
  3. Run the server via yarn serve (Assumes Node20)

Steps:

  1. Create a subscription in Ruru for Foo
    foo(fooId: $fooId) {
    foo {
      createdAt
      fieldToUpdate
      fooId
      id
    }
    event
    }
    }
    --- VARIABLES
    {
    "fooId": 1
    }
  2. Insert a new record into the foo table (assuming id === 1)

Expectations: On insert/update we fetch the latest state of the record.

Reality: We fetch the latest state of the record once, then serve from cache moving forward.

Why is this a bug? If you remove the list from the lambda in subscribePlan all works as expected. It's only when a list is supplied that the record seems to be cached.

Example of bug: image image image image image image

I did some poking around in the Grafast source but couldn't find anything obvious. Best guess is something to do with the dependency tracking with ListStep and determining safe cache vs unsafe cache.

Thanks!

benjie commented 5 months ago

Issue reproduced.

benjie commented 5 months ago

The issue specifically relates to the context[this.$$cache] used by PgExecutor not being reset between subscription payloads. This is specifically a bug in PgExecutor, a false assumption about the appropriateness of caching. This cache should also be cleared just before a new mutation field executes.

https://github.com/graphile/crystal/blob/91e87ab6516490a4cc7b7fc6400efb7623fbd331/grafast/dataplan-pg/src/executor.ts#L403

benjie commented 5 months ago

Moving discussion to https://github.com/graphile/crystal/issues/2079