-
Notifications
You must be signed in to change notification settings - Fork 6
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
Store pid of the backend when connecting to Postgres #71
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
module Database.PostgreSQL.PQTypes.Internal.BackendPid | ||
( BackendPid (..) | ||
) where | ||
|
||
-- | Process ID of the server process attached to the current session. | ||
newtype BackendPid = BackendPid Int | ||
deriving newtype (Eq, Ord, Show) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,9 @@ | ||
{-# LANGUAGE TypeApplications #-} | ||
|
||
module Database.PostgreSQL.PQTypes.Internal.Connection | ||
( -- * Connection | ||
Connection (..) | ||
, getBackendPidIO | ||
, ConnectionData (..) | ||
, withConnectionData | ||
, ConnectionStats (..) | ||
|
@@ -26,10 +29,11 @@ import Control.Exception qualified as E | |
import Control.Monad | ||
import Control.Monad.Base | ||
import Control.Monad.Catch | ||
import Data.Bifunctor | ||
import Data.ByteString.Char8 qualified as BS | ||
import Data.Foldable qualified as F | ||
import Data.Functor.Identity | ||
import Data.IORef | ||
import Data.Int | ||
import Data.Kind | ||
import Data.Pool | ||
import Data.Set qualified as S | ||
|
@@ -42,12 +46,14 @@ import Foreign.Ptr | |
import GHC.Conc (closeFdWith) | ||
import GHC.Stack | ||
|
||
import Database.PostgreSQL.PQTypes.Internal.BackendPid | ||
import Database.PostgreSQL.PQTypes.Internal.C.Interface | ||
import Database.PostgreSQL.PQTypes.Internal.C.Types | ||
import Database.PostgreSQL.PQTypes.Internal.Composite | ||
import Database.PostgreSQL.PQTypes.Internal.Error | ||
import Database.PostgreSQL.PQTypes.Internal.Error.Code | ||
import Database.PostgreSQL.PQTypes.Internal.Exception | ||
import Database.PostgreSQL.PQTypes.Internal.QueryResult | ||
import Database.PostgreSQL.PQTypes.Internal.Utils | ||
import Database.PostgreSQL.PQTypes.SQL.Class | ||
import Database.PostgreSQL.PQTypes.SQL.Raw | ||
|
@@ -114,6 +120,8 @@ initialStats = | |
data ConnectionData = ConnectionData | ||
{ cdPtr :: !(Ptr PGconn) | ||
-- ^ Pointer to connection object. | ||
, cdBackendPid :: !BackendPid | ||
-- ^ Process ID of the server process attached to the current session. | ||
, cdStats :: !ConnectionStats | ||
-- ^ Statistics associated with the connection. | ||
, cdPreparedQueries :: !(IORef (S.Set T.Text)) | ||
|
@@ -125,6 +133,11 @@ newtype Connection = Connection | |
{ unConnection :: MVar (Maybe ConnectionData) | ||
} | ||
|
||
getBackendPidIO :: Connection -> IO BackendPid | ||
getBackendPidIO conn = do | ||
withConnectionData conn "getBackendPidIO" $ \cd -> do | ||
pure (cd, cdBackendPid cd) | ||
|
||
withConnectionData | ||
:: Connection | ||
-> String | ||
|
@@ -133,7 +146,9 @@ withConnectionData | |
withConnectionData (Connection mvc) fname f = | ||
modifyMVar mvc $ \mc -> case mc of | ||
Nothing -> hpqTypesError $ fname ++ ": no connection" | ||
Just cd -> first Just <$> f cd | ||
Just cd -> do | ||
(cd', r) <- f cd | ||
cd' `seq` pure (Just cd', r) | ||
|
||
-- | Database connection supplier. | ||
newtype ConnectionSourceM m = ConnectionSourceM | ||
|
@@ -215,12 +230,25 @@ connect ConnectionSettings {..} = mask $ \unmask -> do | |
Just | ||
ConnectionData | ||
{ cdPtr = connPtr | ||
, cdBackendPid = noBackendPid | ||
, cdStats = initialStats | ||
, cdPreparedQueries = preparedQueries | ||
} | ||
F.forM_ csRole $ \role -> runQueryIO conn $ "SET ROLE " <> role | ||
|
||
let selectPid = "SELECT pg_backend_pid()" :: RawSQL () | ||
(_, res) <- runQueryIO conn selectPid | ||
case F.toList $ mkQueryResult @(Identity Int32) selectPid noBackendPid res of | ||
[pid] -> withConnectionData conn fname $ \cd -> do | ||
pure (cd {cdBackendPid = BackendPid $ fromIntegral pid}, ()) | ||
pids -> do | ||
let err = HPQTypesError $ "unexpected backend pid: " ++ show pids | ||
rethrowWithContext selectPid noBackendPid $ toException err | ||
|
||
pure conn | ||
where | ||
noBackendPid = BackendPid 0 | ||
|
||
fname = "connect" | ||
|
||
openConnection :: (forall r. IO r -> IO r) -> CString -> IO (Ptr PGconn) | ||
|
@@ -317,6 +345,7 @@ runPreparedQueryIO conn (QueryName queryName) sql = do | |
E.throwIO | ||
DBException | ||
{ dbeQueryContext = sql | ||
, dbeBackendPid = cdBackendPid | ||
, dbeError = HPQTypesError "runPreparedQueryIO: unnamed prepared query is not supported" | ||
, dbeCallStack = callStack | ||
} | ||
|
@@ -329,7 +358,7 @@ runPreparedQueryIO conn (QueryName queryName) sql = do | |
-- succeeds, we need to reflect that fact in cdPreparedQueries since | ||
-- you can't prepare a query with the same name more than once. | ||
res <- c_PQparamPrepare cdPtr nullPtr param cname query | ||
void . withForeignPtr res $ verifyResult sql cdPtr | ||
void . withForeignPtr res $ verifyResult sql cdBackendPid cdPtr | ||
modifyIORef' cdPreparedQueries $ S.insert queryName | ||
(,) | ||
<$> (fromIntegral <$> c_PQparamCount param) | ||
|
@@ -353,7 +382,7 @@ runQueryImpl fname conn sql execSql = do | |
-- runtime system is used) and react appropriately. | ||
queryRunner <- async . restore $ do | ||
(paramCount, res) <- execSql cd | ||
affected <- withForeignPtr res $ verifyResult sql cdPtr | ||
affected <- withForeignPtr res $ verifyResult sql cdBackendPid cdPtr | ||
stats' <- case affected of | ||
Left _ -> | ||
return | ||
|
@@ -370,8 +399,7 @@ runQueryImpl fname conn sql execSql = do | |
, statsValues = statsValues cdStats + (rows * columns) | ||
, statsParams = statsParams cdStats + paramCount | ||
} | ||
-- Force evaluation of modified stats to squash a space leak. | ||
stats' `seq` return (cd {cdStats = stats'}, (either id id affected, res)) | ||
return (cd {cdStats = stats'}, (either id id affected, res)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, although there's tons of return in other places, so whether I replace this one or not is irrelevant. Let's leave it for later when hlint is enabled for this repo, it can be then done systematically for all occurrences. |
||
-- If we receive an exception while waiting for the execution to complete, | ||
-- we need to send a request to PostgreSQL for query cancellation and wait | ||
-- for the query runner thread to terminate. It is paramount we make the | ||
|
@@ -399,10 +427,11 @@ runQueryImpl fname conn sql execSql = do | |
verifyResult | ||
:: (HasCallStack, IsSQL sql) | ||
=> sql | ||
-> BackendPid | ||
-> Ptr PGconn | ||
-> Ptr PGresult | ||
-> IO (Either Int Int) | ||
verifyResult sql conn res = do | ||
verifyResult sql pid conn res = do | ||
-- works even if res is NULL | ||
rst <- c_PQresultStatus res | ||
case rst of | ||
|
@@ -421,7 +450,7 @@ verifyResult sql conn res = do | |
_ | otherwise -> return . Left $ 0 | ||
where | ||
throwSQLError = | ||
rethrowWithContext sql | ||
rethrowWithContext sql pid | ||
=<< if res == nullPtr | ||
then | ||
return . E.toException . QueryError | ||
|
@@ -451,6 +480,7 @@ verifyResult sql conn res = do | |
E.throwIO | ||
DBException | ||
{ dbeQueryContext = sql | ||
, dbeBackendPid = pid | ||
, dbeError = HPQTypesError ("verifyResult: string returned by PQcmdTuples is not a valid number: " ++ show sn) | ||
, dbeCallStack = callStack | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would dispense with the
do
here, to be honest, but I guess from our hlint.yaml there is no in-house policy against redundantdo
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While
do
is technically redundant, it's there to appease haskell-mode formatter in emacs to do the right thing 👻