Skip to content

Commit 554fe47

Browse files
committed
fix: Admin server dies silently
This is a workaround for Warp crashing with "thread blocked indefinitely in an STM transaction" when receiving eMFILE from accept. The fix is to provide Warp with a custom accept function that handles eMFILE by logging it and keeping accepting connections.
1 parent 9336604 commit 554fe47

5 files changed

Lines changed: 45 additions & 13 deletions

File tree

src/PostgREST/Admin.hs

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
1+
{-# LANGUAGE MonadComprehensions #-}
12
module PostgREST.Admin
23
( runAdmin
34
) where
45

56
import qualified Data.Aeson as JSON
7+
import Foreign.C.Error (Errno (..), eMFILE)
8+
import GHC.IO.Exception
69
import qualified Network.HTTP.Types.Status as HTTP
710
import qualified Network.Wai as Wai
811
import qualified Network.Wai.Handler.Warp as Warp
@@ -13,6 +16,7 @@ import Network.Socket.ByteString
1316

1417
import PostgREST.AppState (AppState, getConfig)
1518
import PostgREST.Config (AppConfig (..))
19+
import PostgREST.Debounce (makeDebouncer)
1620
import PostgREST.MediaType (MediaType (..), toContentType)
1721
import PostgREST.Metrics (metricsToText)
1822
import PostgREST.Network (resolveSocketToAddress)
@@ -23,18 +27,46 @@ import qualified PostgREST.AppState as AppState
2327
import qualified Network.Socket as NS
2428
import Protolude
2529

26-
runAdmin :: AppState -> Maybe NS.Socket -> IO (Maybe NS.Socket) -> Warp.Settings -> IO ()
27-
runAdmin appState maybeAdminSocket getSocketREST settings = do
30+
runAdmin :: AppState -> Maybe NS.Socket -> IO (Maybe NS.Socket) -> Warp.ServerState -> Warp.Settings -> IO ()
31+
runAdmin appState maybeAdminSocket getSocketREST serverState settings = do
2832
conf <- getConfig appState
2933
whenJust maybeAdminSocket $ \adminSocket -> do
3034
address <- resolveSocketToAddress adminSocket
35+
-- log EMFILE at most once every 10s
36+
logEMFILE <- makeDebouncer $ observer AdminServerAcceptEMFILEFailure *> threadDelay 10_000_000
37+
let
38+
-- Keep accepting on eMFILE
39+
safeAccept sock =
40+
accept sock `catchEMFile`
41+
do
42+
logEMFILE
43+
44+
connCount <- Warp.currentOpenConnections serverState
45+
join $ atomically $ do
46+
if connCount > 0 then do
47+
-- There are open connections
48+
-- wait for closing some and restart accepting
49+
nextConnCount <- Warp.currentOpenConnectionsSTM serverState
50+
check (nextConnCount < connCount)
51+
pure $ safeAccept sock
52+
else
53+
pure $ do
54+
-- Backoff for 1ms so that we don't enter busy accept loop
55+
-- this should let the system recover fd's once the pressure is gone
56+
-- then restart accepting
57+
threadDelay 1_000
58+
safeAccept sock
59+
3160
void . forkIO $ handle (onError adminSocket) $
32-
Warp.runSettingsSocket (adminServerSettings conf address) adminSocket adminApp
61+
Warp.runSettingsSocket (adminServerSettings conf address safeAccept) adminSocket adminApp
3362
where
3463
adminApp = admin appState getSocketREST
3564
observer = AppState.getObserver appState
36-
adminServerSettings config addr=
65+
isEMFILE err = Just eMFILE == fmap Errno (ioe_errno err)
66+
catchEMFile action handler = handleJust (\e -> [ e | isEMFILE e]) (const handler) action
67+
adminServerSettings config addr safeAccept = do
3768
settings
69+
& Warp.setAccept safeAccept
3870
& Warp.setBeforeMainLoop (observer $ AdminStartObs addr)
3971
& maybe identity Warp.setPort (configAdminServerPort config)
4072

src/PostgREST/App.hs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,8 @@ run appState = do
8989
readIORef mainSocketRef >>= foldMap NS.close
9090
Unix.installSignalHandlers observer closeSockets (AppState.schemaCacheLoader appState) (AppState.readInDbConfig False appState)
9191

92-
Admin.runAdmin appState adminSocket (readIORef mainSocketRef) (serverSettings conf)
92+
(adminState, adminSettings) <- fmap (serverSettings conf) <$> Warp.makeSettingsAndServerState
93+
Admin.runAdmin appState adminSocket (readIORef mainSocketRef) adminState adminSettings
9394

9495
Listener.runListener appState
9596

@@ -106,7 +107,7 @@ run appState = do
106107
address <- resolveSocketToAddress mainSocket
107108

108109
let
109-
appServerSettings = serverSettings conf
110+
appServerSettings = serverSettings conf defaultSettings
110111
& setPort (configServerPort conf)
111112
& setOnException onWarpException
112113
& setBeforeMainLoop (observer $ AppServerAddressObs address)
@@ -130,9 +131,9 @@ run appState = do
130131
| Just (ioeGetErrorType -> et) <- fromException se, et == ResourceVanished || et == InvalidArgument = False
131132
| otherwise = True
132133

133-
serverSettings :: AppConfig -> Warp.Settings
134-
serverSettings AppConfig{..} =
135-
defaultSettings
134+
serverSettings :: AppConfig -> Warp.Settings -> Warp.Settings
135+
serverSettings AppConfig{..} settings =
136+
settings
136137
& setHost (fromString $ toS configServerHost)
137138
& setServerName ("postgrest/" <> prettyVersion)
138139

src/PostgREST/Logger.hs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,8 @@ observationMessages = \case
134134
pure $ "Admin server listening on " <> address
135135
AdminServerCrashedObs ex ->
136136
pure $ "FAILURE: Admin server crashed unexpectedly: " <> (showOnSingleLine '\t' . show) ex
137+
AdminServerAcceptEMFILEFailure ->
138+
pure "Admin server failed to accept connection due to file descriptor exhaustion (EMFILE)"
137139
AppStartObs ver ->
138140
pure $ "Starting PostgREST " <> T.decodeUtf8 ver <> "..."
139141
AppServerAddressObs address ->

src/PostgREST/Observation.hs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import Protolude hiding (toList)
2626
data Observation
2727
= AdminStartObs Text
2828
| AdminServerCrashedObs SomeException
29+
| AdminServerAcceptEMFILEFailure
2930
| AppStartObs ByteString
3031
| AppServerAddressObs Text
3132
| ExitUnsupportedPgVersion PgVersion PgVersion

test/io/test_io.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -846,10 +846,6 @@ def test_admin_live_dependent_on_main_app(defaultenv):
846846
assert response.status_code == 500
847847

848848

849-
@pytest.mark.xfail(
850-
reason="Admin server crashes when the first admin accept hits EMFILE",
851-
strict=True,
852-
)
853849
def test_admin_server_does_not_crash_when_file_limit_is_reached(defaultenv):
854850
"Admin server should keep running when accept reaches the open file limit."
855851

0 commit comments

Comments
 (0)