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

Concurrency Issue with AfterFind with PostgreSQL and MySQL #530

Open
breml opened this issue Apr 6, 2020 · 7 comments
Open

Concurrency Issue with AfterFind with PostgreSQL and MySQL #530

breml opened this issue Apr 6, 2020 · 7 comments
Labels
bug Something isn't working

Comments

@breml
Copy link
Contributor

breml commented Apr 6, 2020

Description

In one of my models I implement the AfterFindable interface in order to query dependent entities from the database (via foreign keys).
If I access the list endpoint for said entity, the access fails from time to time with the following error message from the lib/pg driver:

unable to fetch records: pq: unexpected Parse response 'C'

This problem only appeared with PostgreSQL, I was not able to reproduce it with MySQL (did not try sqlite).

Possible fix

I was able to fix this problem with PostgreSQL by changing callbacks.go lines 31-45

from:

	wg := &errgroup.Group{}
	for i := 0; i < rv.Len(); i++ {
		func(i int) {
			wg.Go(func() error {
				y := rv.Index(i)
				y = y.Addr()
				if x, ok := y.Interface().(AfterFindable); ok {
					return x.AfterFind(c)
				}
				return nil
			})
		}(i)
	}

	return wg.Wait()

to:

	for i := 0; i < rv.Len(); i++ {
		y := rv.Index(i)
		y = y.Addr()
		x, ok := y.Interface().(AfterFindable)
		if !ok {
			continue
		}
		if err := x.AfterFind(c); err != nil {
			return err
		}
	}
	return nil

This is why I think, the problem is related to a concurrency issue. The issue might be related to lib/pq#81.

I understand, that from a performance point of view, the existing implementation is better, because it processes the the AfterFind callbacks for each item in the result set in concurrently.
Based on the age of above mentioned bug in lib/pq, I don't think this bug will be solved soon and I therefore think, this problem needs to be addressed in pop. I propose to add a database dialect specific implementation of afterFind for PostgreSQL.

Steps to Reproduce the Problem

  1. Two tables, T1 has foreign key (t2_id) on T2.id
  2. Add a method AfterFind to the model for T1, which queries T2 based on the foreign key (e.g. c.Find(&t2, t1.t2ID)
  3. Access the list endpoint of T1 (GET /t1/list)

Expected Behavior

The endpoint GET /t1/list should always be successful with PostgreSQL the same way it is with MySQL. The pg driver should not fail.

Actual Behavior

From time to time, the request to GET /t1/list does return an error like:

unable to fetch records: pq: unexpected Parse response 'C'

Info

  • OS: Linux 4.15.0-91-generic #92-Ubuntu SMP Fri Feb 28 11:09:48 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
  • pop version: github.com/gobuffalo/pop/v5 v5.0.9
  • pop through buffalo: yes
@breml
Copy link
Contributor Author

breml commented Apr 6, 2020

@breml
Copy link
Contributor Author

breml commented Apr 20, 2020

I just hit the same problem while using MySQL. The mysql db driver reports the following errors:

[mysql] 2020/04/20 10:31:35 packets.go:446: busy buffer
[mysql] 2020/04/20 10:31:35 connection.go:158: bad connection
[mysql] 2020/04/20 10:31:35 packets.go:446: busy buffer
[mysql] 2020/04/20 10:31:35 packets.go:427: busy buffer

Again for me, the patch mentioned above did solve the problem for me.

@breml breml changed the title Concurrency Issue with AfterFind with PostgreSQL Concurrency Issue with AfterFind with PostgreSQL and MySQL Apr 20, 2020
breml added a commit to breml/pop that referenced this issue Apr 20, 2020
@breml
Copy link
Contributor Author

breml commented Apr 20, 2020

To unblock myself I created a fork with the above mentioned fix in place: https://github.com/breml/pop/tree/fix-afterfind. Please let me know, if you would like to have a PR.

@aeneasr
Copy link
Member

aeneasr commented Jun 29, 2020

Now that we moved away from lib/pq to jackc/pgx this should no longer be an issue. Could you check with the latest master if that's the case?

@breml
Copy link
Contributor Author

breml commented Jun 30, 2020

@aeneasr The same problem exists also with MySQL. How will the move to jackc/pgx help there? I think the problem is not related to the actual DB driver but to the way the drivers are used in a concurrent way.

@aeneasr
Copy link
Member

aeneasr commented Jun 30, 2020

Well, we would only need to fix it in MySQL ;)

@lzaldivarkt
Copy link

I got hit by the same issue on mysql, while eager saving. Doesn't happen all the time, but often enough to be noticeable.

My workaround was to just save one item at a time :(

breml added a commit to breml/pop that referenced this issue May 3, 2021
@sio4 sio4 added this to the v6.1.0 milestone Sep 20, 2022
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

No branches or pull requests

5 participants