Lock / contention when not using a transaction and exceeding pool limit

[saurori]

I've discovered (the hard way) that if you use the transaction middleware app.Use(popmw.Transaction(models.DB)), but you DO NOT use the transaction in a request handler (i.e. just use models.DB directly), the entire application will lock up if the number of concurrent requests exceeds your pool setting for the DB connection.

To test this, create a new buffalo app. Configure the database and add some sample data. Sample config:

development:
  dialect: postgres
  database: my_app
  user: {{envOr "DATABASE_USER" "postgres"}}
  host: {{envOr "DATABASE_HOST" "127.0.0.1"}}
  pool: {{envOr "DATABASE_CONNS" "3"}}

In a handler, query the db and DO NOT use the tx stored in context, example:

func HomeHandler(c buffalo.Context) error {
	var em []models.ExampleModel

	if err := models.DB.All(&em); err != nil {
		return err
	}

	return c.Render(http.StatusOK, r.HTML("index.html"))
}

Make concurrent requests to the endpoint exceeding the pool, I did 10 concurrent requests. The application will completely lock up and not respond at all. Removing the transaction middleware eliminates the problem.

[matthewbordas]

I'm seeing some strange behavior where requests are being retried upon failure and my hunch is that it has something to do with the transaction middleware. Did you get any insight into your issues?

[sio4]

Hi @matthewbordas, Thanks for your comment.

Does your experience match this issue's description? Actually, I didn't have a look at this issue yet but will check it as soon as possible. If you could provide more details, it could be helpful to understand the issue more clearly.

[github-actions]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment. Otherwise, this will be closed in 7 days.

[paganotoni]

@sio4, how do recent changes in Pop affect this issue?

[sio4]

@sio4, how do recent changes in Pop affect this issue?

This issue still exists. I improved Pop's logging so users can track transaction/connection information better, so they can imagine what happened when they use pop middleware and the raw models.DB at the same time, but we need to design another clear way to avoid this issue in the future.

I have no solid idea for now, even though I am thinking about it, and I think fixing this without API/design change could not be possible (since we expose and use context's tx and DB at the same time, and they have different characteristic and changing on them could cause issues on existing apps)

Also, there are related considerations such as supporting "multiple databases (or data sources) in a single app", "multiple connections/transactions in a single handler", and "explicit transaction" (not automated by middleware per a single handler).