Skip to content

Commit 497705c

Browse files
KritiRavDivjot Arora
authored andcommitted
GODRIVER-1632 fixing function for retryable writes (#433)
1 parent 6b450cf commit 497705c

File tree

8 files changed

+16
-50
lines changed

8 files changed

+16
-50
lines changed

mongo/change_stream_deployment.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,6 @@ func (c *changeStreamDeployment) SelectServer(context.Context, description.Serve
2727
return c, nil
2828
}
2929

30-
func (c *changeStreamDeployment) SupportsRetryWrites() bool {
31-
return false
32-
}
33-
3430
func (c *changeStreamDeployment) Kind() description.TopologyKind {
3531
return c.topologyKind
3632
}

mongo/client_test.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,6 @@ func (md mockDeployment) SelectServer(context.Context, description.ServerSelecto
4343
return nil, nil
4444
}
4545

46-
func (md mockDeployment) SupportsRetryWrites() bool {
47-
return false
48-
}
49-
5046
func (md mockDeployment) Kind() description.TopologyKind {
5147
return description.Single
5248
}

mongo/integration/mtest/opmsg_deployment.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -106,12 +106,6 @@ func (md *mockDeployment) SelectServer(context.Context, description.ServerSelect
106106
return md, nil
107107
}
108108

109-
// SupportsRetry implements the Deployment interface. It always returns true to allow for testing
110-
// retryability.
111-
func (md *mockDeployment) SupportsRetryWrites() bool {
112-
return true
113-
}
114-
115109
// Kind implements the Deployment interface. It always returns description.Single.
116110
func (md *mockDeployment) Kind() description.TopologyKind {
117111
return description.Single

x/mongo/driver/description/server.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,3 +369,8 @@ func decodeStringMap(element bsoncore.Element, name string) (map[string]string,
369369
}
370370
return m, nil
371371
}
372+
373+
// SupportsRetryWrites returns true if this description represents a server that supports retryable writes.
374+
func (s Server) SupportsRetryWrites() bool {
375+
return s.SessionTimeoutMinutes != 0 && s.Kind != Standalone
376+
}

x/mongo/driver/driver.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
// Deployment is implemented by types that can select a server from a deployment.
1111
type Deployment interface {
1212
SelectServer(context.Context, description.ServerSelector) (Server, error)
13-
SupportsRetryWrites() bool
1413
Kind() description.TopologyKind
1514
}
1615

@@ -98,10 +97,6 @@ func (ssd SingleServerDeployment) SelectServer(context.Context, description.Serv
9897
return ssd.Server, nil
9998
}
10099

101-
// SupportsRetryWrites implements the Deployment interface. It always returns Type(0), because a single
102-
// server does not support retryability.
103-
func (SingleServerDeployment) SupportsRetryWrites() bool { return false }
104-
105100
// Kind implements the Deployment interface. It always returns description.Single.
106101
func (SingleServerDeployment) Kind() description.TopologyKind { return description.Single }
107102

@@ -121,10 +116,6 @@ func (ssd SingleConnectionDeployment) SelectServer(context.Context, description.
121116
return ssd, nil
122117
}
123118

124-
// SupportsRetryWrites implements the Deployment interface. It always returns Type(0), because a single
125-
// connection does not support retryability.
126-
func (ssd SingleConnectionDeployment) SupportsRetryWrites() bool { return false }
127-
128119
// Kind implements the Deployment interface. It always returns description.Single.
129120
func (ssd SingleConnectionDeployment) Kind() description.TopologyKind { return description.Single }
130121

x/mongo/driver/operation.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,7 @@ func (op Operation) retryable(desc description.Server) bool {
509509
if op.Client != nil && (op.Client.Committing || op.Client.Aborting) {
510510
return true
511511
}
512-
if op.Deployment.SupportsRetryWrites() &&
512+
if desc.SupportsRetryWrites() &&
513513
desc.WireVersion != nil && desc.WireVersion.Max >= 6 &&
514514
op.Client != nil && !(op.Client.TransactionInProgress() || op.Client.TransactionStarting()) &&
515515
writeconcern.AckWrite(op.WriteConcern) {

x/mongo/driver/operation_test.go

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -126,11 +126,6 @@ func TestOperation(t *testing.T) {
126126
}
127127
})
128128
t.Run("retryableWrite", func(t *testing.T) {
129-
deploymentRetry := new(mockDeployment)
130-
deploymentRetry.returns.retry = true
131-
132-
deploymentNoRetry := new(mockDeployment)
133-
134129
sessPool := session.NewPool(nil)
135130
id, err := uuid.New()
136131
noerr(t, err)
@@ -152,31 +147,33 @@ func TestOperation(t *testing.T) {
152147
wcAck := writeconcern.New(writeconcern.WMajority())
153148
wcUnack := writeconcern.New(writeconcern.W(0))
154149

155-
descRetryable := description.Server{WireVersion: &description.VersionRange{Min: 0, Max: 7}}
156-
descNotRetryable := description.Server{WireVersion: &description.VersionRange{Min: 0, Max: 5}}
150+
descRetryable := description.Server{WireVersion: &description.VersionRange{Min: 0, Max: 7}, SessionTimeoutMinutes: 1}
151+
descNotRetryableWireVersion := description.Server{WireVersion: &description.VersionRange{Min: 0, Max: 5}, SessionTimeoutMinutes: 1}
152+
descNotRetryableStandalone := description.Server{WireVersion: &description.VersionRange{Min: 0, Max: 7}, SessionTimeoutMinutes: 1, Kind: description.Standalone}
157153

158154
testCases := []struct {
159155
name string
160156
op Operation
161157
desc description.Server
162158
want Type
163159
}{
164-
{"deployment doesn't support", Operation{Deployment: deploymentNoRetry}, description.Server{}, Type(0)},
165-
{"wire version too low", Operation{Deployment: deploymentRetry, Client: sess, WriteConcern: wcAck}, descNotRetryable, Type(0)},
160+
{"deployment doesn't support", Operation{}, description.Server{}, Type(0)},
161+
{"wire version too low", Operation{Client: sess, WriteConcern: wcAck}, descNotRetryableWireVersion, Type(0)},
162+
{"standalone not supported", Operation{Client: sess, WriteConcern: wcAck}, descNotRetryableStandalone, Type(0)},
166163
{
167164
"transaction in progress",
168-
Operation{Deployment: deploymentRetry, Client: sessInProgressTransaction, WriteConcern: wcAck},
165+
Operation{Client: sessInProgressTransaction, WriteConcern: wcAck},
169166
descRetryable, Type(0),
170167
},
171168
{
172169
"transaction starting",
173-
Operation{Deployment: deploymentRetry, Client: sessStartingTransaction, WriteConcern: wcAck},
170+
Operation{Client: sessStartingTransaction, WriteConcern: wcAck},
174171
descRetryable, Type(0),
175172
},
176-
{"unacknowledged write concern", Operation{Deployment: deploymentRetry, Client: sess, WriteConcern: wcUnack}, descRetryable, Type(0)},
173+
{"unacknowledged write concern", Operation{Client: sess, WriteConcern: wcUnack}, descRetryable, Type(0)},
177174
{
178175
"acknowledged write concern",
179-
Operation{Deployment: deploymentRetry, Client: sess, WriteConcern: wcAck, Type: Write},
176+
Operation{Client: sess, WriteConcern: wcAck, Type: Write},
180177
descRetryable, Write,
181178
},
182179
}
@@ -508,9 +505,6 @@ func (m *mockDeployment) SelectServer(ctx context.Context, desc description.Serv
508505
return m.returns.server, m.returns.err
509506
}
510507

511-
func (m *mockDeployment) SupportsRetryWrites() bool {
512-
return m.returns.retry
513-
}
514508
func (m *mockDeployment) Kind() description.TopologyKind { return m.returns.kind }
515509

516510
type mockServerSelector struct{}

x/mongo/driver/topology/topology.go

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -283,16 +283,6 @@ func (t *Topology) RequestImmediateCheck() {
283283
t.serversLock.Unlock()
284284
}
285285

286-
// SupportsSessions returns true if the topology supports sessions.
287-
func (t *Topology) SupportsSessions() bool {
288-
return t.Description().SessionTimeoutMinutes != 0 && t.Description().Kind != description.Single
289-
}
290-
291-
// SupportsRetryWrites returns true if the topology supports retryable writes, which it does if it supports sessions.
292-
func (t *Topology) SupportsRetryWrites() bool {
293-
return t.SupportsSessions()
294-
}
295-
296286
// SelectServer selects a server with given a selector. SelectServer complies with the
297287
// server selection spec, and will time out after severSelectionTimeout or when the
298288
// parent context is done.

0 commit comments

Comments
 (0)