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.

Sign up for GitHub

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

Jump to bottom

busy loop on too many open files #1003

Open
domenkozar opened this issue Sep 14, 2024 · 2 comments · May be fixed by #1004
Open

busy loop on too many open files #1003

domenkozar opened this issue Sep 14, 2024 · 2 comments · May be fixed by #1004

Comments

@domenkozar
Copy link 

acceptNewConnection = do
    ex <- UnliftIO.tryIO getConnMaker
    case ex of
        Right x -> return $ Just x
        Left e -> do
            let eConnAborted = getErrno eCONNABORTED
                getErrno (Errno cInt) = cInt
            if ioe_errno e == Just eConnAborted
                then acceptNewConnection
                else do
                    settingsOnException set Nothing $ toException e
                    return Nothing

The issue is that this function only checks for the eCONNABORTED error specifically. If any other error occurs, including a "too many open files" error, it will call settingsOnException and then return Nothing, which causes acceptLoop to exit.

However, acceptLoop is called in a loop from acceptConnection, so it will immediately try to accept a new connection again. If the "too many open files" error persists, this will result in a busy loop, constantly trying to accept new connections and failing.

To fix this, you could:

  1. Add a delay before retrying after encountering errors other than eCONNABORTED.
  2. Implement a backoff strategy for repeated errors.
  3. Handle the "too many open files" error specifically and take appropriate action (like waiting for some connections to close).
This was referenced Sep 14, 2024
Closed
Open
@nh2
Copy link

nh2 commented Sep 14, 2024

it [acceptNewConnection] will return Nothing, which causes acceptLoop to exit. [..] acceptLoop is called in a loop from acceptConnection, so it will immediately try to accept a new connection again

@domenkozar I do not follow that:

When acceptNewConnection returns Nothing, acceptLoop will not loop:

    acceptLoop = do
        mx <- acceptNewConnection
        case mx of
            Nothing -> return ()
            Just (mkConn, addr) -> do
                fork set mkConn addr app counter ii
                acceptLoop

I can't see a place where "acceptLoop is called in a loop from acceptConnection".

The whole code is (irrelevant lines removed):

acceptConnection set getConnMaker app counter ii = do
    void $ UnliftIO.mask_ acceptLoop
  where
    acceptLoop = do
        mx <- acceptNewConnection
        case mx of
            Nothing -> return ()
            Just (mkConn, addr) -> do
                fork set mkConn addr app counter ii
                acceptLoop

    acceptNewConnection = do
        ex <- UnliftIO.tryIO getConnMaker
        case ex of
            Right x -> return $ Just x
            Left e -> do
                let eConnAborted = getErrno eCONNABORTED
                    getErrno (Errno cInt) = cInt
                if ioe_errno e == Just eConnAborted
                    then acceptNewConnection
                    else do
                        settingsOnException set Nothing $ toException e
                        return Nothing

@domenkozar
Copy link
Author

@nh2 good catch, thank you! I've pushed a fix to the PR that adds a delay before retrying with a new connection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

delay new connection in case of exausted files cachix/wai
2 participants
@domenkozar @nh2