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

Memory leak in version 1.22.0 #496

Open
rausnitz opened this issue Jul 31, 2024 · 10 comments · May be fixed by #497
Open

Memory leak in version 1.22.0 #496

rausnitz opened this issue Jul 31, 2024 · 10 comments · May be fixed by #497
Assignees
Labels
bug Something isn't working

Comments

@rausnitz
Copy link
Sponsor Contributor

rausnitz commented Jul 31, 2024

Describe the issue

Memory usage spiked when we deployed our application with postgres-nio 1.22.0.

Vapor version

4.102.1

Operating system and version

Linux

Swift version

Swift 5.10

Steps to reproduce

Run a Swift backend application that uses postgres-nio and compare the memory usage when on 1.21.5 and 1.22.0.

Outcome

Memory usage steadily increases after starting an application that uses postgres-nio 1.22.0.

Additional notes

We observed this on an instance that runs recurring jobs but does not receive web traffic, so memory usage tends to be stable over time.

In the attached screenshot, the first release to include postgres-nio 1.22.0 was on July 26. The sudden drops in memory usage indicate when new releases went live, restarting the app. The graph stabilizes on July 30 when we reverted postgres-nio to 1.21.5.

postgres-nio-memory

Edit: I'm not sure if the memory usage spikes in 1.21.6 or 1.22.0.

@rausnitz rausnitz added the bug Something isn't working label Jul 31, 2024
@MahdiBM
Copy link
Contributor

MahdiBM commented Jul 31, 2024

Repro (No need to even do a query):

@main
struct EntryPoint {

    static func main() async throws {
        let config = PostgresClient.Configuration(...)
        let client = PostgresClient(configuration: config)

        try await withThrowingTaskGroup(of: Void.self) { taskGroup in
            taskGroup.addTask {
                await client.run()
            }

            taskGroup.addTask {
                for idx in (0..<100_000) {
                    if idx % 10 == 0 {
                        print(idx)
                    }
                    try await Task.sleep(for: .milliseconds(10))
                    try await client.withConnection { conn in
                        /// No need to even do a query
                    }
                }
            }

            try await taskGroup.waitForAll()
        }
    }
}

@MahdiBM
Copy link
Contributor

MahdiBM commented Jul 31, 2024

No need to even be able to create a proper connection?:

@main
struct EntryPoint {
    static func main() async throws {
        let config = PostgresClient.Configuration(
            host: "CORRECT-HOST",
            port: 5432,
            username: "USER",
            password: "BAD_PASSWORD",
            database: "DB",
            tls: .disable
        )

        let client = PostgresClient(configuration: config)

        try await withThrowingTaskGroup(of: Void.self) { taskGroup in
            taskGroup.addTask {
                await client.run()
            }

            taskGroup.addTask {
                for idx in (0..<100_000) {
                    if idx % 10 == 0 {
                        print(idx)
                    }
                    try await Task.sleep(for: .milliseconds(10))
                    try await client.withConnection { conn in

                    }
                }
            }

            try await taskGroup.waitForAll()
        }
    }
}

@petrpavlik
Copy link

I'll just add that memory leak instruments in Xcode is reporting a leak when profiling my backend code.

Screenshot_2024-07-31_at_22 38 41

@fabianfett
Copy link
Collaborator

Okay, found the source of this leak:

With 1.22.0 we started to forward write promises:

private func write<T>(_ task: HandlerTask, cascadingFailureTo promise: EventLoopPromise<T>) {

However in the success case we never forward them to the wire in the PostgresChannelHandler.

func write(context: ChannelHandlerContext, data: NIOAny, promise: EventLoopPromise<Void>?) {

@petrpavlik
Copy link

Would it maybe make sense to revert the commit that has caused the leak before the fix PR is approved and merged in?

@LukasCZ
Copy link

LukasCZ commented Aug 20, 2024

Any update on this? This likely affects vast majority of web servers written in Vapor, and it's been broken for three weeks now. When can we expect it to be fixed and merged in? Or should I pin to the previous version of postgres-nio for now?
Screenshot 2024-08-20 at 13 18 40

@fabianfett
Copy link
Collaborator

@LukasCZ thanks for pinging. Will prepare a revert pr for now.

@fabianfett
Copy link
Collaborator

PR is up: #501

@fabianfett
Copy link
Collaborator

Released as 1.22.1

@LukasCZ
Copy link

LukasCZ commented Aug 20, 2024

Thank you @fabianfett 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants