Skip to content

GODRIVER-3399: PoolClearedError should have TransientTransactionError label appended to it #2114

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
Jun 30, 2025
+37 −1
Merged

GODRIVER-3399: PoolClearedError should have TransientTransactionError label appended to it #2114

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
bfe0067
add TransientTransactionError label to poolClearedError
zhouselena Jun 24, 2025
f5a2321
Merge branch 'master' of github.com:zhouselena/mongo-go-driver into G…
zhouselena Jun 24, 2025
06308d2
cleanup
zhouselena Jun 24, 2025
f4fc9f4
cleanup
zhouselena Jun 24, 2025
2bf70b3
update to driver.TransientTransactionError
zhouselena Jun 24, 2025
38d2589
wrap error instead of adding errorLabels
zhouselena Jun 24, 2025
d15e4c6
wrap error in driver.error
zhouselena Jun 25, 2025
46de325
wrap all instances of pce
zhouselena Jun 25, 2025
7f6ed4d
pull back idle connections pcerr
zhouselena Jun 25, 2025
2fd17ef
Merge branch 'master' of github.com:zhouselena/mongo-go-driver into G…
zhouselena Jun 30, 2025
9696918
add recommended comment
zhouselena Jun 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions x/mongo/driver/topology/pool.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ type poolClearedError struct {
}

func (pce poolClearedError) Error() string {
return fmt.Sprintf(
"connection pool for %v was cleared because another operation failed with: %v",
pce.address,
pce.err)
wrappedErr := fmt.Errorf(
"%v: connection pool for %v was cleared because another operation failed with: %v %w",
driver.TransientTransactionError, pce.address, pce.err, pce)
return wrappedErr.Error()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error "labels" in this context means that the error type must satisfy the LabeledError interface and have a particular label string. See example code that checks if an error has the "TransientTransactionError" label here.

There are a few ways to accomplish that:

  1. Wrap the poolClearedError with a driver.Error and add the "TransientTransactionError" label to the labels list.
  2. Add a HasErrorLabel(string) bool method to the poolClearedError type and return true if the arg is "TransientTransactionError".

Option 2 is more complex, and it's not clear how we'd determine if a transaction is running when the poolClearedError is created. Option 1 is already used when handling server selection errors while running a transaction (see here), so I recommend that approach.

In the above example, the "TransientTransactionError" label is always added if selectServer returns an error while a transaction is running. However, some errors that server.Connection returns should not have the "TransientTransactionError" label applied. You'll need to check if the returned error is a poolClearedError before adding the "TransientTransactionError" label.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matthewdale I fixed it to wrap the driver.Error and updated the test accordingly! however, I noticed that PoolClearedError is created here as well and passed through a wantConn trydeliver. I'm not super familiar with what exactly wantConn does, but it is a poolClearedError that is being passed through so I was wondering if it also needs to be labelled there?

}

// Retryable returns true. All poolClearedErrors are retryable.
Expand Down
24 changes: 24 additions & 0 deletions x/mongo/driver/topology/pool_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"go.mongodb.org/mongo-driver/v2/internal/eventtest"
"go.mongodb.org/mongo-driver/v2/internal/require"
"go.mongodb.org/mongo-driver/v2/mongo/address"
"go.mongodb.org/mongo-driver/v2/x/mongo/driver"
"go.mongodb.org/mongo-driver/v2/x/mongo/driver/operation"
)

Expand Down Expand Up @@ -1584,3 +1585,26 @@ func TestPool_PoolMonitor(t *testing.T) {
"expected ConnectionCheckOutFailed Duration to be set")
})
}

func TestPool_Error(t *testing.T) {
t.Parallel()

t.Run("should have TransientTransactionError", func(t *testing.T) {
t.Parallel()

p := newPool(poolConfig{})
assert.Equalf(t, poolPaused, p.getState(), "expected new pool to be paused")

// Since new pool is paused, checkout should throw PoolClearedError.
_, err := p.checkOut(context.Background())

var pce poolClearedError
if errors.As(err, &pce) {
assert.Contains(t, pce, driver.TransientTransactionError, `expected error to include the "TransientTransactionError" label`)
} else {
t.Errorf("expected poolClearedError, got %v", err)
}

p.close(context.Background())
})
}
Loading