Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple Local API queries in hooks will never resolve (Postgres) #6743

Closed
lekterable opened this issue Jun 12, 2024 · 8 comments
Closed

Multiple Local API queries in hooks will never resolve (Postgres) #6743

lekterable opened this issue Jun 12, 2024 · 8 comments
Labels

Comments

@lekterable
Copy link

lekterable commented Jun 12, 2024

Link to reproduction

No response

Describe the Bug

Running more than one Local API query in hooks will never resolve.

I originally reported it on Discord here, but I never got a response.

After playing with it a little bit more it seems that it's a Postgres specific issue as after changing the db to Mongo it doesn't happen anymore.

Looks like a pretty big limitation for the Postgres adapter.

 beforeValidate: [
      async ({ data, operation, req }) => {
        if (operation === 'update') return data

        const {
          docs: [product]
        } = await req.payload.find({
          collection: 'products',
          where: { medusa_id: data.product_id }
        })

        console.log('before validate')

        const res = await req.payload.find({
          collection: 'products',
          where: { medusa_id: data.product_id }
        })

        console.log('after')

        data.product = product.id

        return data
      }
    ],

The above example will never get to the console.log('after') line while using Postgres, it does work on Mongo though.

To Reproduce

  1. Call the Local API in a hook more than once (.find in my case)
  2. Observe that the second API call will never resolve nor throw an error.

Payload Version

2.19.3

Adapters and Plugins

db-postgres

@lekterable lekterable added status: needs-triage Possible bug which hasn't been reproduced yet v2 labels Jun 12, 2024
@jmikrut
Copy link
Member

jmikrut commented Jun 12, 2024

Hey @lekterable — I just reproduced this with your exact code and I can't seem to reproduce.

In order to help here, we're going to need you to provide a minimal reproduction - but I'd be happy to help once we can understand how to reproduce!

@jmikrut jmikrut added the status: cant-reproduce If an issue cannot be reproduced label Jun 12, 2024
@github-actions github-actions bot removed the status: needs-triage Possible bug which hasn't been reproduced yet label Jun 12, 2024
@lekterable
Copy link
Author

huh, okay that's very strange, because the only thing that helped me was changing the adapter to Mongo

so you're able to query twice in the same hooks execution from the Postgres db? @jmikrut

I'll prepare a repro when I have a moment

@jmikrut
Copy link
Member

jmikrut commented Jun 12, 2024

Yep. I did two consecutive finds in the same beforeValidate hook just as you did. And I logged the result for each find and both worked accordingly.

Are you setting max connections or anything like that? One thing you could try is to pass the req through to your find queries in your hook, so that the Payload operations are all running on the same transaction / connection.

Every time you do a payload.find or any local API method, Payload will either:

  1. Start a new transaction
  2. Use the same transaction that already exists if you pass the req as an argument

So your code as-is will start 3 different transactions (one for the main create / update op, and one each for your find ops). If you pass the req to your find operations, the find operations will re-use the existing transaction.

Try that?

@lekterable
Copy link
Author

Are you setting max connections or anything like that?

Not explicitly, I'm using the default settings which Railway provides. I only copied the DB_URL

So your code as-is will start 3 different transactions (one for the main create / update op, and one each for your find ops). If you pass the req to your find operations, the find operations will re-use the existing transaction.

Try that?

Thanks for the tip! I think I'll just stick to Mongo as it seems to be more matured and doesn't cause any issues, but I'll try to provide a full repro when I have a minute.

@jmikrut
Copy link
Member

jmikrut commented Jun 12, 2024

Of course.

It's worth noting that MongoDB would have this same issue if you use MongoDB with replica sets (you need replica sets in order to get MongoDB to work with transactions).

This would explain why in MongoDB, you do not see any issues (you probably don't have replica sets, so transactions are not enabled).

This whole transactions topic is a database-wide paradigm, in that transactions operate by reserving a connection for a specific transaction. Doesn't matter if it's Postgres or MongoDB - both work the same way when enabled.

I do think that for most use cases, Payload lends itself well to MongoDB though. So if you can switch, I wouldn't stop ya!

If you do get around to getting a repro put together, please post it here and I will re-open the issue.

Thank you!

@jmikrut jmikrut closed this as completed Jun 12, 2024
@github-actions github-actions bot removed the status: cant-reproduce If an issue cannot be reproduced label Jun 12, 2024
@christopherpickering
Copy link

I have the same issue, but am not sure how to reproduce exactly. In hooks I was able to fix by adding req to the queries as suggested, but in ad-hock local api code (running in the onInit function to load my search data) there is no req object..? It seems to leave behind 9 random queries in "idle in transaction"

Here's an example repository https://github.com/christopherpickering/idle_in_transaction

  1. Create a user and run the demo seed from admin
  2. I added locales
  3. I added a localized field on pages > title
  4. I added an "onInit"

In my code, I have around 12 larger pages. Here's the onInit that fails in my code:

import { locales } from '../locales'

export const preloadSearch = async ({ payload }): Promise<void> => {

  const sync =[{ collection: 'products'}, {collection: 'pages'}
    ]

  // in this example, this works.... but if you add more pages, around 12 in my app,
  // the query takes longer and this leaves behind some idle in transactions.
  // sync.map(c => {
  //   Promise.all(locales.locales.map(l => preload({ payload, sync: c, locale: l.code })))
  // })

  // this fixes the problem
  sync.map(c => {
    Promise.all(locales.locales.map((l, index) => {
      setTimeout(function () {
          preload({ payload, sync: c, locale: l.code })
    }, index * 1000);
    }))
  })
}

export const preload = async ({ payload, sync, locale }): Promise<void> => {
  console.log(sync, locale)
  let r = await payload.find({
    req: {payload},
    collection: sync.collection,
    depth: 1,
    limit: 10000,
    locale: locale,
    pagination: false,
  })
}

@christopherpickering
Copy link

christopherpickering commented Jul 30, 2024

In this particular case, the other thing that seems to fix the problem for me (lots of queries running at the same time) is increasing the max connection limit on the pool from 10 to 50.

db: postgresAdapter({
    pool: {
      connectionString: process.env.DATABASE_URI,
      max:50
    },
  }),

I tried to follow it back through /node_modules/drizzle-orm/node-postgres/session.cjs, and it seems like on line 127 const result = await..., it sometimes doesn't return. Its as is two queries on the same tables/joins cross paths and freeze?

Copy link

github-actions bot commented Sep 6, 2024

This issue has been automatically locked.
Please open a new issue if this issue persists with any additional detail.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants