feat: add process name to 2pc transaction ID by levkk · Pull Request #1248 · pgdogdev/pgdog

levkk · 2026-07-21T17:35:08Z

Globally unique transaction ID

Without hopefully going over the 200 bytes limit.

Example:

__pgdog_2pc_pgdog-node-1_1231424234

The last part (the number) can be up to 20 chars long (usize::MAX), so I'm optimistic people don't name their nodes something 150 characters long, but I may be proven wrong.

Misc

Refactor cluster readiness into its own module.
Maybe? add code coverage to CI for integration tests? Still not sure if its working.
Add explicit SIGTERM handler which calls std::process:exit.

codecov · 2026-07-21T17:48:30Z

Codecov Report

❌ Patch coverage is 88.49840% with 36 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
.../client/query_engine/two_pc/server_transactions.rs	15.78%	16 Missing ⚠️
...src/backend/replication/logical/subscriber/copy.rs	0.00%	6 Missing ⚠️
...src/frontend/client/query_engine/two_pc/manager.rs	66.66%	4 Missing ⚠️
pgdog/src/backend/pool/cluster_launch.rs	96.59%	3 Missing ⚠️
pgdog/src/config/mod.rs	62.50%	3 Missing ⚠️
...c/frontend/client/query_engine/two_pc/statement.rs	94.44%	3 Missing ⚠️
pgdog/src/main.rs	88.88%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

sgrif · 2026-07-22T13:29:31Z

+    fn test_instance_id() {
+        for id in [1024, 11111111, usize::MAX, usize::MIN] {
+            let transaction = TwoPcTransaction(id);
+            let instance_id = instance_id(); // Generate it, it's a singleton.


It feels really gross having TwoPcTransaction rely on global state for its Display impl. Can we separate that out into something that allows the state to be injected?

It does, yeah. We could call it more explicitly for sure, but we also want to avoid ever rendering a "non-unique" ID by accident.

Right, I'm thinking we remove the Display impl and have fn display_name(&self, deployment_id: Option<&str>, instance_id: &str), pushing the state up to the caller and making this easier to test

That'll be a lot of places, which I guess...doesn't really help, since we can fat-finger the calling site. And those are very much invariants/identifiers which we never ever want to confuse.

sgrif · 2026-07-22T13:30:22Z

    instance_id().split("-").last().unwrap().parse()
 }

+static DEPLOYMENT_ID: Lazy<Option<String>> = Lazy::new(|| env::var("DEPLOYMENT_ID").ok());


DEPLOYMENT_ID is a really generic name. Should we prefix this?

Like PGDOG_DEPLOYMENT_ID or something?

Maybe. We already use other similar-named env variables there, e.g. NODE_ID. I don't expect ppl to run pgdog in an env shared by another app, and if they do, they certainly wouldn't be sharding, and if they did, I would be like...damn. But yeah, gotta be careful with breaking changes, like I'd want to change all env variables to match the new PGDOG_ format, but I cant :(

Co-authored-by: Sage Griffin <sage@sagetheprogrammer.com>

sgrif · 2026-07-23T15:21:45Z

+        for shard in self.shards() {
+            shard.wait_schema_loaded().await;
+        }


These should be able to run in parallel.

Ah yeah. Those are just waiters, the actual schema fetching runs in parallel, so these will resolve quickly.

levkk · 2026-07-23T15:29:17Z

Still trying to figure out how to get coverage on integration tests -- will use this PR as guinea pig until I give up / it works.

levkk · 2026-07-23T15:43:44Z

Pretty sure its working guys, let me know if you spot anything:

levkk · 2026-07-23T15:44:29Z

@sgrif can you tell me if this is hallucinated:

pgdog/pgdog/src/main.rs

Line 243 in ae77c99

/// Install a SIGTERM handler that exits the process via [`exit`], running

sgrif · 2026-07-23T15:52:38Z

It's accurate, but if this is purely for code coverage purposes I'd be sending SIGINT instead of SITGERM to the server (presumably we already gracefully shutdown on sigint)

levkk · 2026-07-23T15:53:35Z

It's accurate, but if this is purely for code coverage purposes I'd be sending SIGINT instead of SITGERM to the server

Hmm, I wonder who's sending SIGTERM...nevermind, the call is coming from inside the house, it's our CI script.

levkk · 2026-07-23T15:53:49Z

(presumably we already gracefully shutdown on sigint)

Correct

levkk · 2026-07-23T15:54:23Z

Aight, well...maybe I'll just keep the handler? It seems like a nice to have feature, plus we get a logline, so that's cool.

levkk added 2 commits July 21, 2026 09:58

feat: add instance id to txn name

58c4ff4

tests

4b79ec0

feat: cleanup abandoned 2pc transactions

f7f5e57

levkk changed the title ~~feat: add process name to 2pc transaction ID~~ feat: add process name to 2pc transaction ID + cleanup abandoned Jul 22, 2026

levkk requested review from meskill and sgrif July 22, 2026 00:09

levkk added 2 commits July 21, 2026 17:10

schema

5850007

cippeh

8c7b1f0

sgrif requested changes Jul 22, 2026

View reviewed changes

levkk and others added 4 commits July 22, 2026 06:49

Update pgdog/src/frontend/client/query_engine/two_pc/statement.rs

6f3473c

Co-authored-by: Sage Griffin <sage@sagetheprogrammer.com>

Update pgdog/src/util.rs

72b846e

Co-authored-by: Sage Griffin <sage@sagetheprogrammer.com>

Update pgdog/src/frontend/client/query_engine/two_pc/transaction.rs

3f08c65

Co-authored-by: Sage Griffin <sage@sagetheprogrammer.com>

tests and parallelization and remove the user check which was incorrect

3b3f9af

meskill reviewed Jul 22, 2026

View reviewed changes

Comment thread pgdog-config/src/general.rs Outdated

Comment thread pgdog-config/src/general.rs Outdated

meskill requested changes Jul 22, 2026

View reviewed changes

Comment thread pgdog/src/frontend/client/query_engine/two_pc/statement.rs Outdated

Comment thread pgdog/src/frontend/client/query_engine/two_pc/manager.rs Outdated

Comment thread pgdog/src/frontend/client/query_engine/two_pc/manager.rs Outdated

levkk added 6 commits July 22, 2026 14:49

refactor to better rust

c6f5feb

warn

92e1888

pub(crate)

c140bb6

refactor

6655ac9

fmt

d176b63

remove two pc cleanup

f6a46ed

levkk changed the title ~~feat: add process name to 2pc transaction ID + cleanup abandoned~~ feat: add process name to 2pc transaction ID Jul 23, 2026

schema

2d2ab52

meskill reviewed Jul 23, 2026

View reviewed changes

Comment thread pgdog/src/frontend/client/query_engine/two_pc/transaction.rs Outdated

Comment thread pgdog/src/frontend/client/query_engine/two_pc/manager.rs Outdated

Comment thread pgdog/src/backend/pool/cluster_launch.rs Outdated

Comment thread pgdog/src/backend/pool/cluster_launch.rs

This comment has been minimized.

Sign in to view

levkk added 4 commits July 23, 2026 05:38

fix

4de458a

wrong comments

4e8eb94

correct naming

01a03a4

remove bs comment

369f107

mark cluster offline before shutting down shards

780d83a

levkk requested a review from meskill July 23, 2026 13:08

try to get coverage for integration runs

ae77c99

meskill approved these changes Jul 23, 2026

View reviewed changes

sgrif reviewed Jul 23, 2026

View reviewed changes

sgrif approved these changes Jul 23, 2026

View reviewed changes

levkk merged commit c32b154 into main Jul 23, 2026
45 checks passed

levkk deleted the levkk-pod-transaction-name branch July 23, 2026 15:57

Uh oh!

Conversation

levkk commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Globally unique transaction ID

Misc

Uh oh!

codecov Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

levkk commented Jul 23, 2026

Uh oh!

levkk commented Jul 23, 2026

Uh oh!

levkk commented Jul 23, 2026

Uh oh!

sgrif commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

levkk commented Jul 23, 2026

Uh oh!

levkk commented Jul 23, 2026

Uh oh!

levkk commented Jul 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

levkk commented Jul 21, 2026 •

edited

Loading

codecov Bot commented Jul 21, 2026 •

edited

Loading

sgrif commented Jul 23, 2026 •

edited

Loading