feat: add process name to 2pc transaction ID + cleanup abandoned 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.

Cleanup abandoned

Without durability, 2pc transactions can be left unfinished on PgDog crash/dirty shutdown. Leaving those around is bad: they can block vacuum.

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

Codecov Report

❌ Patch coverage is 92.71654% with 37 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
pgdog/src/backend/pool/cluster_launch.rs	90.51%	11 Missing ⚠️
...src/frontend/client/query_engine/two_pc/manager.rs	83.07%	11 Missing ⚠️
...src/backend/replication/logical/subscriber/copy.rs	0.00%	6 Missing ⚠️
.../client/query_engine/two_pc/server_transactions.rs	76.19%	5 Missing ⚠️
...rc/frontend/client/query_engine/end_transaction.rs	0.00%	3 Missing ⚠️
...og/src/backend/replication/logical/orchestrator.rs	0.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

sgrif · 2026-07-22T13:19:55Z

+    /// WARNING: This only happens if durability for 2pc is off. Running this
+    /// will cause data loss.
+    ///
+    pub async fn cleanup_abandoned(&self) -> Result<(), Error> {


Suggested change

pub async fn cleanup_abandoned(&self) -> Result<(), Error> {

pub(crate) async fn cleanup_abandoned(&self) -> Result<(), Error> {

This one I'm calling from main.rs actually so needs to be pub

Ah. We should fix that so main doesn't have different visibility rules than lib but that's a separate change

sgrif · 2026-07-22T13:20:35Z

 /// Build `PREPARE TRANSACTION`, `COMMIT PREPARED`, or `ROLLBACK PREPARED`
 /// for a shard participant.
-pub fn phase_control(transaction: &str, shard: usize, phase: TwoPcPhase) -> String {
+pub fn phase_control(transaction: TwoPcTransaction, shard: usize, phase: TwoPcPhase) -> String {


Suggested change

pub fn phase_control(transaction: TwoPcTransaction, shard: usize, phase: TwoPcPhase) -> String {

pub(crate) fn phase_control(transaction: TwoPcTransaction, shard: usize, phase: TwoPcPhase) -> String {

sgrif · 2026-07-22T13:21:05Z


    /// Execute two-phase commit transaction control statements.
-    pub async fn two_pc(&mut self, name: &str, phase: TwoPcPhase) -> Result<(), Error> {
+    pub async fn two_pc(


Suggested change

pub async fn two_pc(

pub(crate) async fn two_pc(

sgrif · 2026-07-22T13:28:09Z

    }
+
+    /// This transaction was created by this process.
+    pub(crate) fn is_mine(&self) -> bool {


This function name confused me, I think we can make it clearer.

Suggested change

pub(crate) fn is_mine(&self) -> bool {

pub(crate) fn created_by_this_deployment(&self) -> bool {

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>

meskill · 2026-07-22T17:03:57Z

+    #[serde(default = "General::two_phase_commit_rollback_abandoned")]
+    pub two_phase_commit_rollback_abandoned: bool,
+
+    /// Maximum amount of time to block startup in order to rollback abandoned
+    /// transactions.
+    #[serde(default = "General::two_phase_commit_rollback_abandoned_timeout")]
+    pub two_phase_commit_rollback_abandoned_timeout: u64,


raising hands to the sky: when would we split the configuration into the subsections and not put everything in general

LOL. Yeah....might be time, huh?

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

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 5 commits July 22, 2026 14:49

refactor to better rust

c6f5feb

warn

92e1888

pub(crate)

c140bb6

refactor

6655ac9

fmt

d176b63

	pub async fn cleanup_abandoned(&self) -> Result<(), Error> {
	pub(crate) async fn cleanup_abandoned(&self) -> Result<(), Error> {

	pub fn phase_control(transaction: TwoPcTransaction, shard: usize, phase: TwoPcPhase) -> String {
	pub(crate) fn phase_control(transaction: TwoPcTransaction, shard: usize, phase: TwoPcPhase) -> String {

	pub(crate) fn is_mine(&self) -> bool {
	pub(crate) fn created_by_this_deployment(&self) -> bool {

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

Cleanup abandoned

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!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

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!

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!

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

levkk Jul 22, 2026 •

edited

Loading