Repository navigation
Conversation
This was referenced Jun 2, 2026
mediprtl
force-pushed
the
feat/min-lsn-read-routing
branch
from
June 13, 2026 07:19
d8025a0 to
b3ea7c9
Compare
mediprtl
marked this pull request as ready for review
June 15, 2026 20:46
mediprtl
force-pushed
the
feat/min-lsn-read-routing
branch
from
June 15, 2026 21:17
b3ea7c9 to
fc5705b
Compare
…#1078) Handle listen/notify commands without enabling the parser for all other statements.
- feat: `SHOW LISTENERS` admin database stats on each pub/sub channel - feat: OpenMetrics/OTEL for the same thing - fix: race condition in `LISTEN` command handler when registering for the first time Stats recorded: - number of listeners - number of messages received - number of messages dropped
…pgdogdev#1086) ### Breaking change Using `SET pgdog.shard` and `pgdog.set_sharding_key` inside a transaction (`BEGIN`) binds the entire transaction to that one shard only. Added the following checks to prevent data consistency errors: 1. Omnisharded tables cannot be written to inside direct-to-shard transactions anymore 2. Caller cannot change shards mid-transaction anymore, e.g. by calling `SET pgdog.shard` or by running a direct-to-shard or cross-shard query Technically, this not a breaking change per-se but a but a bug fix, since this was causing silent data corruption errors previously, but still worth calling out. fix pgdogdev#1085 ### Other changes - refactor: rename really poorly named methods in `Route` - refactor: connection binding enum handling was nonsense, now it's less so - refactor: added lots of comments in places where I touched something in this PR - refactor: removed some dead code
Upload `.deb` packages to R2 to make them available from our apt repo at `apt.pgdog.dev`.
Reverts pgdogdev#1091 Maybe not today. A bit too much work to maintain.
…v#1077) Rehaul of sharded_mapping configuration ## What is done - embed mapping configuration inside the `[[sharded_tables]]`instead of it being defined separately with `[[sharded_mapping]]`. The benefits of doing this: - make the relation between those explicit and without ambiguity when the mapping should apply, no need to duplicate fields between two and build the search keys to find the related mapping. - solve the original issue - the mapping/table without table name, how they should be connected and applied together. The mapping is always defined on the proper table/catch-all table - no decoupled mappings in config due to misconfiguration - the old `[[sharded_mapping]]` is left for backward compatibility, but no fixes for it were provided, therefore it's not working properly in some cases and this is intentionally: - if table name is not defined for mapping it will be applied only for the sharded_table without name. No more guessing if it should provided for some other tables etc. as I initially did in this pr. - the schema is not checked for the search and could be applied wrong if the schema is defined on the table/mapping - the mapping is now fully compound and all the settings are used in order (List -> Range -> Default kinds) as it was previously either Range or List + Default - split some types to config and runtime shapes - the config shape with only values parsed from toml and runtime shape with calculated/omitted fields - add multiple validations for mapping configuration (currently shown as warn) - add ..Ref types to avoid clonning but still be able to search in maps - drop unreliable Hash implementations (they often diverge from the type itself) and use other approaches instead ## The config difference <img width="1140" height="432" alt="image" src="https://github.com/user-attachments/assets/3f3d3809-7e8e-4841-881c-5eb1d5dcde4c" /> ## Roadmap Preserve the old definition for some time and remove in some future release leaving only the new way for configuring mapping.
…gdogdev#1095) When adding new replicas to a cluster, `RELOAD` previously dropped all existing backend connections because the target count changed. Connection preservation now uses address-based matching instead of requiring identical replica counts. Existing replicas keep their connections, and new replicas start with empty pools. If a database was paused via `PAUSE` before a `RELOAD`, the new pools now inherit the `paused` state so the database stays paused.
Add `prefer_primary` read_write_split option which will send all queries to the primary unless the query explicitly opts into being sent to a replica.
This is the first in a series of (hopefully smaller from this point) commits that port the codebase over to use the new parser. The function this commit focuses on is StatementParser::run_walk. This commit leaks *way* outside that function, since it required most of the glue code to pass the new AST around. The parsed AST must live at the cache layer, since `pg_raw_parse::ParseResult` is what holds the `MemoryContext`, and all nodes contain a lifetime referring to that struct. We're doing a funky thing where we parse once at the same time as the old parser runs, but then later we unparse and reparse and start passing that around. The reason for this is that the rewriter hasn't been ported yet, so it only operates on the old parser's AST. If we didn't deparse/reparse at that point, code after that which gets ported would be based on the untransformed query. The actual porting code is relatively straightforward. It's mostly 1:1 changes with fewer layers of indirection, but I did clean up some edge cases in advisory locks while I was at it. The new code can handle advisory locks coming from values clauses without an explicit column name given, and also correctly handles completely silly cases where the advisory lock comes form a subselect. These edge cases are unlikely to ever show up in the wild, but it's imporant that if we ever do encounter them, we do something sensible instead of silently doing the wrong thing. And handling the tricky edge cases also gave me an opportunity to stress test the capability of the new parser's AST walking API.
…#1100) Block cross-shard sharding key updates when the row is referenced by a foreign key with a destructive `ON DELETE` action (`CASCADE`, `SET NULL`, or `SET DEFAULT`). Those moves are implemented as `DELETE + INSERT`, so the delete step could silently affect related rows. PgDog now checks loaded schema metadata before the delete/insert and returns a recoverable error; same-shard updates and `ON DELETE RESTRICT / NO ACTION` are unchanged.
I changed the behavior of the parser to not pass the root node to the callback. I found recursing based on pointer equality to be too painful as I work on porting the routing functions. As a side effect, this should be more efficient. We did a full AST walk of each subselect looking for advisory locks, and then walked it again looking for tables. With this change they're now a single walk, and the code should be easier to follow
This port diverges from the old implementation in that it doesn't try to stick any int looking ConstValue::String into an integer, as there wasn't any clear reason for us to be doing so.
…v#1108) - feat: accelerate failover detection by adding an event the shard role monitor listens for when the role changes. Previously, we would poll every 333ms. - feat: if LSN monitor gets a checkout timeout from the connection pool, it will create a standalone connection to perform the LSN check. This prevents LSN detection from failing when the connection pool is completely saturated, e.g. during an incident - fix: unban all fallback didn't handle edge case where all pools reported healthy, e.g. after a complete failover
…v#1112) We now use the new parser for routing for anything other than an INSERT statement, which has its own special logic and justifies its own PR. Really this is just a port of shard_select, but udpates and deletes use the same logic and the only thing that changes is how we construct the context. In any cases where I have a function that I couldn't #[cfg] out on the new parser, I've renamed the old function rather than putting _new on the new ones. Since the intention is to eventually delete the old functions, I figured we'd include the churn now while I'm adding the new functions, rather than adding a lot of churn on the new ones later. For the actual logic, we essentially have two modes: - We're looking for a binary operator that we can use to determine a shard - We're looking for a column name/value expr to resolve that operator In both cases, when we encounter a select statement, we must immediately construct a new context and go see if *it* had what we need to route to a shard. We never get an expression from inside a subselect, but if it ever does find something, or if we encounter an error at any point, we want to immediately abort and return whatever that value was. For this, we make extensive use of the standard library's `ControlFlow` type, which is the underlying type that the `?` operator uses, and what `pg_raw_parse` uses to control recursion in its AST walking function. Any time we have a `Result<Shard, Error>`, we put that in a `ControlFlow::Break` which sends that all the way up the stack. When we're in "search for expression" mode, we do a separate recursive call (so we can go through things like type casts and such arbitrarily), and break *either* if we find a Shard *or* an expression we can resolve. Because each call to `walk_manual` can return an error (if PG received a node it doesn't recognize), and also returns an `Option` (if it walked an entire AST without geting a `ControlFlow::Break`, this can lead to the type signatures getting a little hairy (at the bottom of our expression search we end up with `Result<Option<Result<Either<Shard, SearchResult<'_>>, Error>>, Error>`. At this point I'm fairly certain I want to remove the `Result` from `walk_manual`, and just have it panic since receiving a node we don't recognize shouldn't actually be possible. Once I've done that, the types should get a little bit less confusing, and in particular I'll be able to remove all the `transpose` calls (which may be unfamiliar to any devs who haven't done any functional programming). Because `ControlFlow` is a newer type that isn't as widely used as other try types like `Result` and `Option`, it's missing a lot of the nice convenience methods those types have. I've added an extension trait to convert a `Result` into a `ControlFlow` that breaks whenever it sees an error, and continues when it sees an `Ok` The only other thing of note is that this somehow made the codebase faster. This honestly doesn't make sense given that we still run everything through the old parser, then do the very slow old deparse, then reparse. I can't believe this change somehow makes enough of a performance difference to outweigh the cost of the deparse/reparse, but the numbers don't lie. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
I renamed `new_stmt` to `stmt` and `stmt` to `stmt_old` now to avoid more churn on the ported lines later
The old version of this function failed to detect locks anywhere outside of a CTE or top level query. As part of the port, I've fixed that behavior.
`locking_behavior` was never read, and all the logic around lock functions here was redundant with our checks for locking functions that we do elsewhere. The lists we were searching were far to small to justify the complexity of `Lazy<HashMap<...>>`
Since ultimately that function, cte_writes, and has_locks are all trying to determine the same thing, I've moved all three into a single AST walk.
This is the last function in `parser/query/select.rs`
…gdogdev#1132) There's quite a bit of churn in this one, as I needed to change StatementRewrite to always have the new AST so every test that was previously passing `None` needed to be updated. One file in particular had a *bunch* of repeated code in its test, which I extracted to a helper function. The other bit of funkiness here is that since the return value of this function is actually passed to later functions, and I am still trying to break this port up into pieces, we deparse and reparse to convert to the equivalent node in the old parser
- fix: the primary/replica evaluation order was inverted, so the replica ban never triggered - fix: `prefer_primary` incorrectly excluded primary when all replicas were banned @stewart-glabs 🙏 - feat: show the reason why pools are unbanned (helps debugging)
This is what happens when you mark everything as `pub`, robots
This removes all `pub` from the module, deletes the little bit of dead code there was, and got rid of a few impls that could be replaced by a derive or a one-liner
THIS IS WHAT HAPPENS WHEN YOU MARK EVERYTHING AS PUB, ROBOTS
…ev#1140) GEE IT SURE WOULD BE NICE IF LESS CODE WAS WRITTEN BY ROBOTS WHO MARK EVERYTHING AS PUB
- feat: allow clients to manually pin their connections to do some session-level stuff, e.g. temp tables. Use with caution: this steals connections from the shared pool. - feat: discard temporary tables when connections are pinned - this was leaking them before, e.g. an advisory lock would pin and the client could create temporary tables in the same exchange which would remain behind. - refactor: remove some dead code.
…oken (pgdogdev#1131) Replace custom AbortSignal with CancellationToken to make the cancellation explicit and not relied on futures drop (that holds rx part of channel). Pass cancellation token for graceful cancellation. Early cancellation on errors
I think the original intent here was to be able to have something like `SELECT COUNT(foo::bigint), AVG(foo::bigint)` use the existing count column based on any arbitrary expression. However, if this ever did fire it would have been wrong for anything other than `COUNT`, since it assumed things like typecasts were noops which is very much not the case for any other aggregate function, and none of this matters anyway since I deleted the helper deduplication a while ago. So instead we can just use the index of the output column as the identifier for each helper.
…v#1136) More specifically, this ports over the glue code as well as the last subroutine of that function, `create_stmts`. This should be the beefiest function we have to port (dear god please let this be the beefiest one). The new implementation diverges from the old in a couple of ways: - We deparse things lazily instead of eagerly - The old code was only deparsing eagerly due to the slowness of deparsing. This should no longer be slow enough to justify that. If needed we can memoize the result, but I'd rather have it be a memoized method than something upstream code cares about - ShardingKeyUpdate holds the entire original UpdateStmt rather than smaller pieces - I actually tried having this hold onto references of the original query, but ultimately this struct ends up in a field that can't is a sibling of the original AST, so it can't hold a lifetime like that. I probably will revist that at some point, but for now copying the whole thing made more sense than copying each smaller piece into its own allocation Other than the caveats mentioned above, the actual logic is mostly a line for line port. The syntax is certainly very different for constructing these trees, but it's a clean mapping each way.
This is more or less a line for line rewrite, though with some minor refactoring to make the code flow a bit more clearly. A few tests got a bit funky because they're assuming the AST passed to `Aggregate::parse` is the same AST that gets rewritten later, which is not currently the case. That funkiness should go away as soon as I port the aggregate rewrite code to the new parser
This is mostly a line-for-line port with a few minor refactors to make the code flow more smoothly. The main structural difference in the refactor is that I've removed any `return Ok(None)` that appeared other than cases where the node we're looking at really does mean NULL. Things like `A_Const` with a value other than an integer fall through to the `_` branch of the top level match, so that if/when we change this to treat unrecognized nodes as an error, it all happens in one place.
The API for how mutation works on the new parser is pretty fundamentally different from the old one. It doesn't make sense to have it as a field on the struct, and passing it explicitly will also make it clear when I need to deparse/reparse as I'm porting individual functions over.
…#1151) This is mostly a line for line port, with some minor refactoring to make the code flow better. There is a minor behavioral change in that we generate `SUM(arg::numeric * arg::numeric)` for sum of squares instead of `SUM(POWER(arg::numeric, 2))`. This was slightly more convenient to write, and in most languages `x * x` tends to be more performant than `pow(x, 2)` since generic power functions have to handle fractional exponents
We only do this if we have a CRUD statement, as this function gets called with all the obscure top level statements (transaction, create, etc) that PG errors with if passed to its walker
There's too many cfgs to be able to follow the code at this point
Add an opt-in read-your-writes floor. A client sets the `pgdog.min_lsn`
connection parameter (startup option or SET) to a commit LSN it must observe;
PgDog then routes the read only to a replica that has replayed at least that far.
- Reads carrying `pgdog.min_lsn` are filtered to replicas whose replayed LSN
(from the existing LSN monitor) is >= the floor.
- When no replica qualifies, PgDog raises NoReplicaCaughtUp (SQLSTATE 58000)
carrying a catch-up ETA ("eta ~Ns"), derived from the replica's replication lag
in time (now() - pg_last_xact_replay_timestamp()), so a client can size a retry.
The rejection logs at debug (expected backpressure), not error.
- Optional `min_lsn_primary_fallback`: an unmet floor falls back to the primary
instead of erroring.
- `pgdog.min_lsn` is registered as an untracked parameter; inert when unset or on
setups without replicas.
Includes unit tests (routing, NoReplicaCaughtUp, ETA-from-lag) and a pgx
integration test.
mediprtl
force-pushed
the
feat/min-lsn-read-routing
branch
from
July 7, 2026 07:19
fc5705b to
a2349ce
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3
Summary
Adds a read-your-writes routing floor. A client sets
pgdog.min_lsnand PgDog routes the read only to a replica that has replayed at least that far. No application-side cache or replica pinning.pgdog.min_lsnparam (startupoptions=-corSET pgdog.min_lsn = 'X/Y'), added toUNTRACKED_PARAMSso it is not forwarded to the server. Parsed via the existingLsntype.Requestcarriesmin_lsn: Option<i64>; populated inconnect.rsfrom the connection params (soSETmid-session works).pool.lsn_stats()) is >=min_lsn.NoReplicaCaughtUp(a recoverable no-server error, surfaced asFATAL: no replica caught up to the requested min_lsn, SQLSTATE 58000) so the client retries. Falling back to the primary is opt-in viageneral.min_lsn_primary_fallback(default off).Conservative by construction: replay LSN is monotonic and the cached value is <= actual, so the floor never serves a stale read.
Test plan
integration/load_balancer/pgx/min_lsn_test.go):min_lsn = 0/0-> all reads served by replicas, primary untouched;min_lsn = 7FFFFFFF/FFFFFFFF-> read errors (NoReplicaCaughtUp), nothing served. Green at-count 3.min_lsn_primary_fallback = trueflips the unmet case to succeed via the primary.load_balancersuite unaffected.cargo fmt/cargo clippyclean (no new warnings); JSON schema regenerated.See #3 for design and the alternative considered (ClientAffinity, #1/#2). Relates to upstream pgdogdev#912.