Skip to content

Commit bfbc329

Browse files
jeffrymorrisjeffrymorris
committed
NCBC-3769: SDK fails to adapt to new cluster topology if only a port changed
Motivation ---------- If alternate addressing is used and only the port changes, the previous port will continue to be used which will cause the query to fail. Modifications ------------- - Make NodeAdapter use the alt addresses and ports even if the hostname does not vary from the nodeExt hostname. - Minor code cleanup in BucketConfig - Add unit test covering the port changes in alt addresses. Change-Id: I3e85221f434ff2ae1497ccbb5a0687f48e43755c Reviewed-on: https://review.couchbase.org/c/couchbase-net-client/+/209046 Tested-by: Build Bot <build@couchbase.com> Reviewed-by: Richard Ponton <richard.ponton@couchbase.com>
1 parent 89ea7a9 commit bfbc329

File tree

5 files changed

+200
-13
lines changed

5 files changed

+200
-13
lines changed

‎src/Couchbase/Core/Configuration/Server/BucketConfig.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,6 @@ public void SetEffectiveNetworkResolution(ClusterOptions options)
297297
}
298298
//We detect internal or "default" should be used
299299
NetworkResolution = options.EffectiveNetworkResolution = Couchbase.NetworkResolution.Default;
300-
return;
301300
}
302301
else
303302
{
@@ -468,7 +467,7 @@ public void ResolveHostName()
468467
{
469468
var alternateAddress = nodeExt.AlternateAddresses[NetworkResolution];
470469
var port = alternateAddress.Ports.Kv > 0 ? alternateAddress.Ports.Kv : alternateAddress.Ports.KvSsl;
471-
VBucketServerMap.ServerList[i] = alternateAddress.Hostname + ":" + port;
470+
VBucketServerMap.ServerList[i] = $"{alternateAddress.Hostname}:{port}";
472471
}
473472
}
474473
}

‎src/Couchbase/Core/Configuration/Server/NodeAdapter.cs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,9 @@ internal bool UseAlternateNetwork(NodesExt nodeExt, BucketConfig bucketConfig)
118118

119119
if (bucketConfig.NetworkResolution == NetworkResolution.Auto || bucketConfig.NetworkResolution == NetworkResolution.External)
120120
{
121-
return string.Compare(nodeExt.Hostname, nodeExt.AlternateAddresses[Couchbase.NetworkResolution.External].Hostname, StringComparison.InvariantCultureIgnoreCase) != 0;
121+
return string.Compare(nodeExt.Hostname,
122+
nodeExt.AlternateAddresses[NetworkResolution.External].Hostname,
123+
StringComparison.InvariantCultureIgnoreCase) != 0 || nodeExt.HasAlternateAddress;
122124
}
123125

124126
//It's a custom configuration being used - try it.
@@ -134,8 +136,8 @@ internal string GetHostname(NodesExt nodeExt, BucketConfig bucketConfig)
134136
{
135137
if (UseAlternateAddress)
136138
{
137-
var networkResolution = bucketConfig.NetworkResolution == Couchbase.NetworkResolution.Auto
138-
? Couchbase.NetworkResolution.External : bucketConfig.NetworkResolution;
139+
var networkResolution = bucketConfig.NetworkResolution == NetworkResolution.Auto
140+
? NetworkResolution.External : bucketConfig.NetworkResolution;
139141

140142
var hostname = nodeExt.AlternateAddresses[networkResolution].Hostname;
141143
_mappedNodeInfo = $"NetworkResolution [{bucketConfig.NetworkResolution}] using alternate mapping {nodeExt.Hostname} to {hostname} - rev {bucketConfig.ConfigVersion}.";
@@ -150,8 +152,8 @@ internal Services GetServicePorts(NodesExt nodeExt, BucketConfig bucketConfig)
150152
{
151153
if (UseAlternateAddress)
152154
{
153-
var networkResolution = bucketConfig.NetworkResolution == Couchbase.NetworkResolution.Auto
154-
? Couchbase.NetworkResolution.External : bucketConfig.NetworkResolution;
155+
var networkResolution = bucketConfig.NetworkResolution == NetworkResolution.Auto
156+
? NetworkResolution.External : bucketConfig.NetworkResolution;
155157

156158
var ports = nodeExt.AlternateAddresses[networkResolution].Ports;
157159
if (ports != null)

‎tests/Couchbase.UnitTests/Core/ClusterContextTests.cs‎

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,36 @@ public ClusterContextTests(ITestOutputHelper output)
3434
_output = output;
3535
}
3636

37+
[Theory]
38+
[InlineData(@"Documents\Configs\config-localhost-alt-addresses-8093.json", 8093)]
39+
[InlineData(@"Documents\Configs\config-localhost-alt-addresses-5555.json", 5555)]
40+
public void Use_Alternate_Address_Query_Port(string configPath, int expectedPort)
41+
{
42+
// Arrange
43+
44+
var config = ResourceHelper.ReadResource(configPath, InternalSerializationContext.Default.BucketConfig);
45+
var options = new ClusterOptions
46+
{
47+
NetworkResolution = NetworkResolution.External
48+
};
49+
config.SetEffectiveNetworkResolution(options);
50+
51+
var nodeAdapter = new NodeAdapter(null, config.NodesExt.First(), config);
52+
var clusterNode = CreateMockedNode("localhost", 11210, nodeAdapter);
53+
54+
var context = new ClusterContext();
55+
context.AddNode(clusterNode);
56+
57+
// Act
58+
59+
var serviceUriProvider = new ServiceUriProvider(context);
60+
var uri = serviceUriProvider.GetRandomQueryUri();
61+
62+
//Assert
63+
64+
Assert.Equal(expectedPort, uri.Port);
65+
}
66+
3767
[Fact]
3868
public void PruneNodes_Removes_Rebalanced_Node()
3969
{
@@ -57,7 +87,7 @@ public void PruneNodes_Removes_Rebalanced_Node()
5787
Assert.DoesNotContain(context.Nodes, node => node.EndPoint.Equals(removed));
5888
}
5989

60-
private IClusterNode CreateMockedNode(string hostname, int port)
90+
private IClusterNode CreateMockedNode(string hostname, int port, NodeAdapter nodeAdapter = null)
6191
{
6292
var mockConnectionPool = new Mock<IConnectionPool>();
6393

@@ -66,18 +96,20 @@ private IClusterNode CreateMockedNode(string hostname, int port)
6696
.Setup(m => m.Create(It.IsAny<ClusterNode>()))
6797
.Returns(mockConnectionPool.Object);
6898

99+
nodeAdapter ??= new NodeAdapter
100+
{
101+
Hostname = hostname,
102+
KeyValue = port
103+
};
104+
69105
var clusterNode = new ClusterNode(new ClusterContext(), mockConnectionPoolFactory.Object,
70106
new Mock<ILogger<ClusterNode>>().Object,
71107
new DefaultObjectPool<OperationBuilder>(new OperationBuilderPoolPolicy()),
72108
new Mock<ICircuitBreaker>().Object,
73109
new Mock<ISaslMechanismFactory>().Object,
74110
new TypedRedactor(RedactionLevel.None),
75111
new HostEndpointWithPort(hostname, port),
76-
new NodeAdapter
77-
{
78-
Hostname = hostname,
79-
KeyValue = port
80-
},
112+
nodeAdapter,
81113
NoopRequestTracer.Instance,
82114
new Mock<IOperationConfigurator>().Object
83115
)

‎tests/Couchbase.UnitTests/Documents/Configs/config-localhost-alt-addresses-5555.json‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
{
2+
"rev": 49323,
3+
"nodesExt": [
4+
{
5+
"services": {
6+
"backupAPI": 8097,
7+
"backupAPIHTTPS": 18097,
8+
"backupGRPC": 9124,
9+
"capi": 8092,
10+
"capiSSL": 18092,
11+
"cbas": 8095,
12+
"cbasSSL": 18095,
13+
"eventingAdminPort": 8096,
14+
"eventingDebug": 9140,
15+
"eventingSSL": 18096,
16+
"fts": 8094,
17+
"ftsGRPC": 9130,
18+
"ftsGRPCSSL": 19130,
19+
"ftsSSL": 18094,
20+
"indexAdmin": 9100,
21+
"indexHttp": 9102,
22+
"indexHttps": 19102,
23+
"indexScan": 9101,
24+
"indexStreamCatchup": 9104,
25+
"indexStreamInit": 9103,
26+
"indexStreamMaint": 9105,
27+
"kv": 11210,
28+
"kvSSL": 11207,
29+
"mgmt": 8091,
30+
"mgmtSSL": 18091,
31+
"n1ql": 8093,
32+
"n1qlSSL": 18093,
33+
"projector": 9999
34+
},
35+
"thisNode": true,
36+
"alternateAddresses": {
37+
"external": {
38+
"hostname": "localhost",
39+
"ports": {
40+
"mgmt": 8091,
41+
"kv": 11210,
42+
"projector": 9999,
43+
"n1ql": 5555,
44+
"indexAdmin": 9100,
45+
"indexScan": 9101,
46+
"indexHttp": 9102,
47+
"indexStreamInit": 9103,
48+
"indexStreamCatchup": 9104,
49+
"indexStreamMaint": 9105,
50+
"fts": 8094,
51+
"ftsGRPC": 9130,
52+
"capi": 8092
53+
}
54+
}
55+
}
56+
}
57+
],
58+
"revEpoch": 1,
59+
"clusterCapabilitiesVer": [
60+
1,
61+
0
62+
],
63+
"clusterCapabilities": {
64+
"n1ql": [
65+
"costBasedOptimizer",
66+
"indexAdvisor",
67+
"javaScriptFunctions",
68+
"inlineFunctions",
69+
"enhancedPreparedStatements",
70+
"readFromReplica"
71+
],
72+
"search": [
73+
"vectorSearch",
74+
"scopedSearchIndex"
75+
]
76+
}
77+
}

‎tests/Couchbase.UnitTests/Documents/Configs/config-localhost-alt-addresses-8093.json‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
{
2+
"rev": 49323,
3+
"nodesExt": [
4+
{
5+
"services": {
6+
"backupAPI": 8097,
7+
"backupAPIHTTPS": 18097,
8+
"backupGRPC": 9124,
9+
"capi": 8092,
10+
"capiSSL": 18092,
11+
"cbas": 8095,
12+
"cbasSSL": 18095,
13+
"eventingAdminPort": 8096,
14+
"eventingDebug": 9140,
15+
"eventingSSL": 18096,
16+
"fts": 8094,
17+
"ftsGRPC": 9130,
18+
"ftsGRPCSSL": 19130,
19+
"ftsSSL": 18094,
20+
"indexAdmin": 9100,
21+
"indexHttp": 9102,
22+
"indexHttps": 19102,
23+
"indexScan": 9101,
24+
"indexStreamCatchup": 9104,
25+
"indexStreamInit": 9103,
26+
"indexStreamMaint": 9105,
27+
"kv": 11210,
28+
"kvSSL": 11207,
29+
"mgmt": 8091,
30+
"mgmtSSL": 18091,
31+
"n1ql": 8093,
32+
"n1qlSSL": 18093,
33+
"projector": 9999
34+
},
35+
"thisNode": true,
36+
"alternateAddresses": {
37+
"external": {
38+
"hostname": "localhost",
39+
"ports": {
40+
"mgmt": 8091,
41+
"kv": 11210,
42+
"projector": 9999,
43+
"n1ql": 8093,
44+
"indexAdmin": 9100,
45+
"indexScan": 9101,
46+
"indexHttp": 9102,
47+
"indexStreamInit": 9103,
48+
"indexStreamCatchup": 9104,
49+
"indexStreamMaint": 9105,
50+
"fts": 8094,
51+
"ftsGRPC": 9130,
52+
"capi": 8092
53+
}
54+
}
55+
}
56+
}
57+
],
58+
"revEpoch": 1,
59+
"clusterCapabilitiesVer": [
60+
1,
61+
0
62+
],
63+
"clusterCapabilities": {
64+
"n1ql": [
65+
"costBasedOptimizer",
66+
"indexAdvisor",
67+
"javaScriptFunctions",
68+
"inlineFunctions",
69+
"enhancedPreparedStatements",
70+
"readFromReplica"
71+
],
72+
"search": [
73+
"vectorSearch",
74+
"scopedSearchIndex"
75+
]
76+
}
77+
}

0 commit comments

Comments
 (0)