Skip to content
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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions mongo/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,9 @@ func (c *Client) StartSession(opts ...*options.SessionOptions) (Session, error)
if sopts.Snapshot != nil {
coreOpts.Snapshot = sopts.Snapshot
}
if sopts.RequireNew != nil {
coreOpts.RequireNew = sopts.RequireNew
}

sess, err := session.NewClientSession(c.sessionPool, c.id, coreOpts)
if err != nil {
Expand Down
14 changes: 14 additions & 0 deletions mongo/options/sessionoptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ type SessionOptions struct {
// be set to true if CausalConsistency is set to true. Transactions and write operations are not allowed on
// snapshot sessions and will error. The default value is false.
Snapshot *bool

// RequireNew ensures that starting a session with a client creates a new
// server session rather than checking one out from the pool. Since this
// avoids pooling, closing the server session is up to the caller.
RequireNew *bool
}

// Session creates a new SessionOptions instance.
Expand Down Expand Up @@ -96,6 +101,12 @@ func (s *SessionOptions) SetSnapshot(b bool) *SessionOptions {
return s
}

// SetRequireNew sets the value for the RequireNew field.
func (s *SessionOptions) SetRequireNew(b bool) *SessionOptions {
s.RequireNew = &b
return s
}

// MergeSessionOptions combines the given SessionOptions instances into a single SessionOptions in a last-one-wins
// fashion.
//
Expand Down Expand Up @@ -125,6 +136,9 @@ func MergeSessionOptions(opts ...*SessionOptions) *SessionOptions {
if opt.Snapshot != nil {
s.Snapshot = opt.Snapshot
}
if opt.RequireNew != nil {
s.RequireNew = opt.RequireNew
}
}
if s.CausalConsistency == nil && (s.Snapshot == nil || !*s.Snapshot) {
s.CausalConsistency = &DefaultCausalConsistency
Expand Down
16 changes: 15 additions & 1 deletion x/mongo/driver/session/client_session.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ type Client struct {
RecoveryToken bson.Raw
PinnedConnection LoadBalancedTransactionConnection
SnapshotTime *primitive.Timestamp
RequireNew bool
}

func getClusterTime(clusterTime bson.Raw) (uint32, uint32) {
Expand Down Expand Up @@ -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
Expand All @@ -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()

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.

Copy link
Author

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?

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.

Choose a reason for hiding this comment

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

The session.Server created here will be returned to the pool when EndSession is called. To allow the CheckedOut counter to track this new checked-out session, we should call Pool.createServerSession here instead.

Suggested change
c.Server, err = newServerSession()
c.Server, err = c.pool.createServerSession()

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?

Copy link
Author

Choose a reason for hiding this comment

The 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
}

Expand Down
4 changes: 4 additions & 0 deletions x/mongo/driver/session/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type ClientOptions struct {
DefaultReadPreference *readpref.ReadPref
DefaultMaxCommitTime *time.Duration
Snapshot *bool
RequireNew *bool
}

// TransactionOptions represents all possible options for starting a transaction in a session.
Expand Down Expand Up @@ -56,6 +57,9 @@ func mergeClientOptions(opts ...*ClientOptions) *ClientOptions {
if opt.Snapshot != nil {
c.Snapshot = opt.Snapshot
}
if opt.RequireNew != nil {
c.RequireNew = opt.RequireNew
}
}

return c
Expand Down