fergusstrange / embedded-postgres

Run a real Postgres database locally on Linux, OSX or Windows as part of another Go application or test
MIT License
851 stars 89 forks source link

Support for dynamic port allocation #128

Open mainbrain opened 9 months ago

mainbrain commented 9 months ago

To be able to start my postgres in a test environment where other tests for other services may run in parallel I'd like to dynamically allocate the port and retrieve the actually allocated port when postgres is started. net.Listen() already supports dynamic port allocation. This is done by passing the port number 0. When I tried using port 0 with embedded-postgres I got this error message:

waiting for server to start....2024-01-26 08:39:40.670 GMT [47041] FATAL: 0 is outside the valid range for parameter "port" (1 .. 65535) stopped waiting pg_ctl: could not start server

fergusstrange commented 8 months ago

This is definitely someting that seems useful.

If I have some spare time over the coming months I will look into putting something together. In the meantime feel free to try something yourself. This would need to be stored and returned in the https://github.com/fergusstrange/embedded-postgres/blob/0db55ea34dc3cfaa0488e1cd8d8dc11adef4ac95/config.go#L140 method.

Cheers

mainbrain commented 8 months ago

@fergusstrange thanks a lot for the positive and welcoming feedback.

I started implementing the feature in the place you suggested and even though it is certainly possible it looks out of place. Config looks like it is intended as a container for configurations, it does not compute anything right now. Lack of error handling possibilities due to chaining and the missing logger makes the code ugly. See for yourself:

// Port sets the runtime port that Postgres can be accessed on. The value can be 0 to dynamically allocate a port.
func (c Config) Port(port uint32) Config {
    c.port = port

    const dynamicallyAllocatedPort = 0
    if port == dynamicallyAllocatedPort {
        conn, err := net.Listen("tcp", fmt.Sprintf("localhost:%d", port))
        if err != nil {
            _, _ = fmt.Fprintf(c.logger, "unable to dynamically allocate port %d", port)
            return c
        }
        c.port = uint32(conn.Addr().(*net.TCPAddr).Port)
        if err := conn.Close(); err != nil {
            _, _ = fmt.Fprintf(c.logger, "unable to close dynamically allocate port %d. The embedded postgres won't be able to use it.", c.port)
            return c
        }
    }
    return c
}

A cleaner solution would be to resolve the real port at a later time inside embedded_postgres.go, for instance inside Start(). There we would have proper error handling and a logger, which would make the implementation much cleaner. The downside of this approach would be, that GetConnectionURL() would have to be moved to embedded_postgres.go, which would be a breaking change.

Either way is fine for me. Just let me know and I'll get a PR ready.

mainbrain commented 8 months ago

I also forgot to mention a technical detail of either of those solutions. What is happening in my example is, that we ask the the OS to dynamically allocate a port and immediately close it. After that we ask Postgers to start using that very same port. In the meantime the resource is freed ant the OS may allocate it for some other process. It is possible to end up with a race condition here.

Race conditions can not happen if the process who is using the port is also the same who allocated it dynamically. In this case the solution without race conditions, would be to ask postgres to allocate a port dynamically and later on find out which port was allocated by it. This would overly complicate things for this minor feature. I guess it would be enough to document the problem and implement a simpler solution.

fergusstrange commented 8 months ago

Hey, completely agree with all the points above.

func (c Config) Port(port uint32) Config should definitely only be a simple setter.

If possible you should try implementing around where the ensurePortAvailable method is right at the beginning of the process execution.

You'll also need to ensure GetConnectionURL is returning the dynamically allocated port. Potentially by just updating the configuration ep.config.port inline where the dynamic allocation happens.