这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@jberryman
Copy link
Collaborator

Make it clear that HASURA_GRAPHQL_PG_CONNECTIONS is a hard limit, and document how it relates to stripes.

The default here seems okay, but I haven't been able to test on anything other than my laptop so far to test whether I can make this a bottleneck. But it's also not clear to me the value of having a low-ish default... I think a better default would be no limit.

Also it would be nice to log when we're about to block, since this is otherwise going to manifest as a really confusing performance issue for users. A starting point is what I was using to play around:

diff --git src/Database/PG/Query/Pool.hs src/Database/PG/Query/Pool.hs
index 271771a..fcf9071 100644
--- src/Database/PG/Query/Pool.hs
+++ src/Database/PG/Query/Pool.hs
@@ -145,7 +145,7 @@ withConn :: (FromPGTxErr e, FromPGConnErr e)
 withConn pool txm f =
   catchConnErr action
   where
-    action  = RP.withResource pool $
+    action  = withLoggingResource pool $
              \connRsrc -> runTxOnConn connRsrc txm f
 
 catchConnErr :: (FromPGConnErr e, MonadError e m, MonadBaseControl IO m)
@@ -179,7 +179,7 @@ runTx' :: (FromPGTxErr e, FromPGConnErr e)
        -> ExceptT e IO a
 runTx' pool tx = do
   res <- liftIO $ runExceptT $ catchConnErr $
-         RP.withResource pool $ \connRsrc -> execTx connRsrc tx
+         withLoggingResource pool $ \connRsrc -> execTx connRsrc tx
   either throwError return res
 
 runTxOnConn' :: PGConn
@@ -194,3 +194,11 @@ sqlFromFile :: FilePath -> Q Exp
 sqlFromFile fp = do
   contents <- qAddDependentFile fp >> runIO (readFile fp)
   [| fromString contents |]
+
+withLoggingResource pool f = do
+  x <- RP.tryWithResource pool f
+  case x of
+    Nothing -> do
+      liftIO $ putStrLn "BLOCKING!"
+      RP.withResource pool f
+    Just b -> return b

Thoughts?

Affected components

  • Docs

@netlify
Copy link

netlify bot commented Dec 5, 2019

Deploy preview for hasura-docs ready!

Built with commit 9413c4d

https://deploy-preview-3495--hasura-docs.netlify.com

@hasura-bot
Copy link
Contributor

Review app for commit 6f2e94a deployed to Heroku: https://hge-ci-pull-3495.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3495-6f2e94a8

@jberryman
Copy link
Collaborator Author

spurious test failure I guess. @praveenweb maybe: have you seen the above? ^

@praveenweb
Copy link
Member

@jberryman - Seems to be an odd Cypress timeout. Re-ran the tests. Tests passed now :)

@marionschleifer marionschleifer added the c/docs Related to docs label Dec 6, 2019
@hasura-bot
Copy link
Contributor

Review app for commit 9413c4d deployed to Heroku: https://hge-ci-pull-3495.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3495-9413c4db

@lexi-lambda lexi-lambda merged commit 55bc9d5 into hasura:master Dec 12, 2019
@hasura-bot
Copy link
Contributor

Review app https://hge-ci-pull-3495.herokuapp.com is deleted

polRk pushed a commit to polRk/graphql-engine that referenced this pull request Feb 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c/docs Related to docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants