-
Notifications
You must be signed in to change notification settings - Fork 8
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.
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
HELP-58644 Add RequireNew session option #13
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -133,6 +133,7 @@ type Client struct { | |||||
RecoveryToken bson.Raw | ||||||
PinnedConnection LoadBalancedTransactionConnection | ||||||
SnapshotTime *primitive.Timestamp | ||||||
RequireNew bool | ||||||
} | ||||||
|
||||||
func getClusterTime(clusterTime bson.Raw) (uint32, uint32) { | ||||||
|
@@ -207,6 +208,9 @@ func NewClientSession(pool *Pool, clientID uuid.UUID, opts ...*ClientOptions) (* | |||||
if mergedOpts.Snapshot != nil { | ||||||
c.Snapshot = *mergedOpts.Snapshot | ||||||
} | ||||||
if mergedOpts.RequireNew != nil { | ||||||
c.RequireNew = *mergedOpts.RequireNew | ||||||
} | ||||||
|
||||||
// For explicit sessions, the default for causalConsistency is true, unless Snapshot is | ||||||
// enabled, then it's false. Set the default and then allow any explicit causalConsistency | ||||||
|
@@ -230,7 +234,17 @@ func NewClientSession(pool *Pool, clientID uuid.UUID, opts ...*ClientOptions) (* | |||||
// SetServer will check out a session from the client session pool. | ||||||
func (c *Client) SetServer() error { | ||||||
var err error | ||||||
c.Server, err = c.pool.GetSession() | ||||||
if c.RequireNew { | ||||||
c.Server, err = newServerSession() | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Suggested change
Alternatively, if there's concern about pooling these sessions in the Go Driver, we need to find a way to bypass the pool when we end the session. Will pooling these new sessions in the Go Driver create any issues? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch, suggest marking the server as dirty when requesting a new server. |
||||||
|
||||||
// Mark the server as dirty so that it won't be returned to the pool. | ||||||
if c.Server != nil { | ||||||
c.Server.MarkDirty() | ||||||
} | ||||||
} else { | ||||||
c.Server, err = c.pool.GetSession() | ||||||
} | ||||||
|
||||||
return err | ||||||
} | ||||||
|
||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we pass the flag to the pool to alter the behavior there? So users can use the session identically regardless of future changes in the pool.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you elaborate on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same concern as Matt's underneath. Will join the discussion in his thread.