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

fatal error: concurrent map writes in EnumCodec.lookupAndCacheString #2089

Closed
jamesroutley opened this issue Jul 16, 2024 · 6 comments
Closed
Labels

Comments

@jamesroutley
Copy link

Describe the bug

We've been seeing fatal error: concurrent map writes errors when running queries using PGX that queries data from tables that have a column with an enum type.

Here's an example stack trace, which should be read bottom to top:

image

I think this is because there's nothing in that object preventing concurrent map writes. I've had a stab at a fix in #2088

To Reproduce

I haven't created a minimal reproduction unfortunately (sorry!).

Expected behavior

I would expect to be able to concurrently query tables which contain an uncached enum value

Actual behavior

We get a fatal error: concurrent map writes

Version

  • Go: $ go version -> go version go1.22.5 darwin/arm64
  • PostgreSQL: $ psql --no-psqlrc --tuples-only -c 'select version()' -> PostgreSQL 14.9 on aarch64-unknown-linux-gnu, compiled by aarch64-unknown-linux-gnu-gcc (GCC) 9.5.0, 64-bit
  • pgx: $ grep 'github.com/jackc/pgx/v[0-9]' go.mod -> v5.5.5

Additional context
n/a

@jackc
Copy link
Owner

jackc commented Jul 16, 2024

EnumCodec (and any Codec for that matter) is not expected to be concurrency safe. How is this being used?

@jamesroutley
Copy link
Author

Hi, sorry for the long delay here.

We're getting this error when reading from a table that has a column who's type is a custom enum (created with create type T as enum (X, Y, Z). Attached is a slightly longer stack trace showing that the error is originating from a DB read (ReadPaymentInstrument just does a select * from table where ...), which calls pgxpool.Pool.QueryRow.

From the trace, it looks like reading from a table that has a custom enum column calls EnumCodec.DecodeValue, which calls the non-concurrency-safe method lookupAndCacheString. Our service performs multiple reads concurrently - do you know of anything between QueryRow and lookupAndCacheString that would prevent concurrent calling of the latter?

Thanks

image

@jackc
Copy link
Owner

jackc commented Aug 13, 2024

Every connection gets its own type map and set of codecs. EnumCodec is not concurrency safe -- but I wouldn't have expected it to be possible for it to be used concurrently. 🤔

@jamesroutley
Copy link
Author

Okay thanks that's interesting information. I'll dig a bit more into our setup and see if I can reproduce a minimal example

@jamesroutley
Copy link
Author

I'm going to close this - I wasn't able to get to the root cause of this but it's stopped happening for us

@a-churchill
Copy link

In case anybody else comes across this, one potential cause is loading custom types (e.g. as suggested here or here) once but using them for every new connection. Here's a sketch of the bad code:

customTypes := make([]*pgtype.Type, 0, len(customTypeNames))

// ... load custom types (only happens once, reused for every connection)

config.AfterConnect = func(ctx context.Context, conn *pgx.Conn) error {
	for _, t := range customTypes {
		conn.TypeMap().RegisterType(t)
	}

	return nil
}

And here's a sketch of the fixed code:

config.AfterConnect = func(ctx context.Context, conn *pgx.Conn) error {
        customTypes := make([]*pgtype.Type, 0, len(customTypeNames))

        // ... load custom types (now happens once per connection)

	for _, t := range customTypes {
		conn.TypeMap().RegisterType(t)
	}

	return nil
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants