internal/datastore/crdb: split read/write connpools #1179
Conversation
jzelinskie
force-pushed
the
rw-conn-pool
branch
from
32a28ed
to
16ff88c
Compare
jzelinskie
force-pushed
the
rw-conn-pool
branch
2 times, most recently
from
6f80962
to
bec09ef
Compare
| mysql.ConnMaxIdleTime(opts.MaxIdleTime), | ||
| mysql.ConnMaxLifetime(opts.MaxLifetime), | ||
| mysql.MaxOpenConns(opts.MaxOpenConns), | ||
| mysql.MaxOpenConns(opts.ReadConnPool.MaxOpenConns), |
There was a problem hiding this comment.
I think it could be argued some folks may consider setting the WriteConnPool parameter instead for those databases that do not support 2 conn pools. As a consequence, the user may think the flag is properly set, when it's in fact being ignored.
Ideally the application makes it difficult to make a mistake here.
There was a problem hiding this comment.
I agree -- especially since this PR doesn't split the pools for all of the datastores.
What's the precedent? We've got some other flags that are CRDB specific, right? What are we doing there?
Let's just be consistent with the existing behavior in this PR and if we have to clean up that behavior, do everything at once in another PR.
There was a problem hiding this comment.
let's assume a user wants to move to use MySQL with the new connection flag, because the old ones are deprecated. They may think they have to set the write flags, when in fact they have to set the read flags. Does that make sense?
There was a problem hiding this comment.
Because MySQL doesn't use pgx, let's get this merged for now and we can work on getting parity to MySQL before the next release.
github-actions
bot
added
the
area/tooling
Because CRDB and Postgres have a stateful protocol that consumes a connection while awaiting a response and writes can starve readers for connections from the pool when there is sufficient load on SpiceDB.
This pull request splits out reads and writes for CockroachDB and PostgreSQL into two different connection pools. This should cause write latency to only impact writes rather than both reads and writes.