pop icon indicating copy to clipboard operation
pop copied to clipboard

Concurrency Issue with AfterFind with PostgreSQL and MySQL

Open breml opened this issue 4 years ago • 7 comments

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 https://github.com/lib/pq/issues/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 avatar Apr 06 '20 13:04 breml

Additional links:

  • https://github.com/lib/pq/issues/635
  • https://github.com/lib/pq/issues/713
  • https://github.com/lib/pq/issues/142

breml avatar Apr 06 '20 13:04 breml

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 avatar Apr 20 '20 08:04 breml

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.

breml avatar Apr 20 '20 09:04 breml

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?

aeneasr avatar Jun 29 '20 21:06 aeneasr

@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.

breml avatar Jun 30 '20 05:06 breml

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

aeneasr avatar Jun 30 '20 06:06 aeneasr

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 :(

lzaldivarkt avatar Aug 15 '20 05:08 lzaldivarkt