From 5c8e2c40820c521da50ab8d56f091a0cc249824b Mon Sep 17 00:00:00 2001 From: iperevoschikov Date: Fri, 2 Oct 2020 17:57:34 +0500 Subject: [PATCH 1/4] Close connection if error occured while open --- .../CustomNodeTests/AuthenticationTest.cs | 21 ++++++++++++++-- .../Core/ThriftConnection.cs | 24 ++++++++++++++++++- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/Cassandra.ThriftClient.Tests/FunctionalTests/CustomNodeTests/AuthenticationTest.cs b/Cassandra.ThriftClient.Tests/FunctionalTests/CustomNodeTests/AuthenticationTest.cs index 1498e1d..e3a0333 100644 --- a/Cassandra.ThriftClient.Tests/FunctionalTests/CustomNodeTests/AuthenticationTest.cs +++ b/Cassandra.ThriftClient.Tests/FunctionalTests/CustomNodeTests/AuthenticationTest.cs @@ -3,6 +3,8 @@ using Cassandra; +using Moq; + using NUnit.Framework; using SkbKontur.Cassandra.Local; @@ -84,11 +86,26 @@ public void TestNonAuthenticatedConnection() Assert.AreEqual("Provided username non-existent and/or password are incorrect", authenticationException.Why); } - private void SomeActionThatRequiresAuthentication(string username, string password) + [Test] + public void TestThriftConnectionClosedAfterNonSuccessfulAuthentication() + { + var logger = new Mock(MockBehavior.Strict); + logger.Setup(l => l.ForContext(It.IsAny())) + .Returns(logger.Object); + + logger.Setup(l => l.IsEnabledFor(It.IsAny())) + .Returns(level => level == LogLevel.Error); + + logger.Setup(l => l.Log(It.Is(e => e.MessageTemplate == "Error while opening connection. Will try to close."))); + + Assert.Throws(() => SomeActionThatRequiresAuthentication("cassandra", "wrong_password", logger.Object)); + } + + private void SomeActionThatRequiresAuthentication(string username, string password, ILog logger = null) { var settings = node.CreateSettings(); settings.Credentials = new Credentials(username, password); - using (var cluster = new CassandraCluster(settings, new SilentLog())) + using (var cluster = new CassandraCluster(settings, logger ?? new SilentLog())) cluster.RetrieveClusterConnection().RetrieveKeyspaces(); } diff --git a/Cassandra.ThriftClient/Core/ThriftConnection.cs b/Cassandra.ThriftClient/Core/ThriftConnection.cs index a2c252d..65f30a4 100644 --- a/Cassandra.ThriftClient/Core/ThriftConnection.cs +++ b/Cassandra.ThriftClient/Core/ThriftConnection.cs @@ -35,7 +35,29 @@ public ThriftConnection(int timeout, IPEndPoint ipEndPoint, string keyspaceName, var transport = new TFramedTransport(tsocket); cassandraClient = new Apache.Cassandra.Cassandra.Client(new TBinaryProtocol(transport)); creationTimestamp = Timestamp.Now; - OpenTransport(); + + try + { + OpenTransport(); + } + catch (Exception openException) + { + this.logger.Error(openException, "Error while opening connection. Will try to close."); + TryCloseTransport(); + throw; + } + } + + private void TryCloseTransport() + { + try + { + CloseTransport(); + } + catch (Exception closeException) + { + logger.Error(closeException, "Error while closing connection."); + } } public void Dispose() From 633b2b3c3157298167a30e5f6863d50823c873af Mon Sep 17 00:00:00 2001 From: iperevoschikov Date: Fri, 2 Oct 2020 18:02:03 +0500 Subject: [PATCH 2/4] up sdk version --- global.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/global.json b/global.json index 9ee6c25..70db897 100644 --- a/global.json +++ b/global.json @@ -1,5 +1,5 @@ { "sdk": { - "version": "3.1.300" + "version": "3.1.402" } } \ No newline at end of file From bbe1600dbe6a0989d106ed09f54776b4c9d09448 Mon Sep 17 00:00:00 2001 From: iperevoschikov Date: Mon, 5 Oct 2020 10:36:52 +0500 Subject: [PATCH 3/4] Work with login and set keyspace errors separately --- .../CustomNodeTests/AuthenticationTest.cs | 2 +- .../Core/ThriftConnection.cs | 81 +++++++++++-------- 2 files changed, 48 insertions(+), 35 deletions(-) diff --git a/Cassandra.ThriftClient.Tests/FunctionalTests/CustomNodeTests/AuthenticationTest.cs b/Cassandra.ThriftClient.Tests/FunctionalTests/CustomNodeTests/AuthenticationTest.cs index e3a0333..1f9dda3 100644 --- a/Cassandra.ThriftClient.Tests/FunctionalTests/CustomNodeTests/AuthenticationTest.cs +++ b/Cassandra.ThriftClient.Tests/FunctionalTests/CustomNodeTests/AuthenticationTest.cs @@ -96,7 +96,7 @@ public void TestThriftConnectionClosedAfterNonSuccessfulAuthentication() logger.Setup(l => l.IsEnabledFor(It.IsAny())) .Returns(level => level == LogLevel.Error); - logger.Setup(l => l.Log(It.Is(e => e.MessageTemplate == "Error while opening connection. Will try to close."))); + logger.Setup(l => l.Log(It.Is(e => e.MessageTemplate == "Error occured while opening thrift connection. Will try to close open transports."))); Assert.Throws(() => SomeActionThatRequiresAuthentication("cassandra", "wrong_password", logger.Object)); } diff --git a/Cassandra.ThriftClient/Core/ThriftConnection.cs b/Cassandra.ThriftClient/Core/ThriftConnection.cs index 65f30a4..6f38b57 100644 --- a/Cassandra.ThriftClient/Core/ThriftConnection.cs +++ b/Cassandra.ThriftClient/Core/ThriftConnection.cs @@ -35,29 +35,7 @@ public ThriftConnection(int timeout, IPEndPoint ipEndPoint, string keyspaceName, var transport = new TFramedTransport(tsocket); cassandraClient = new Apache.Cassandra.Cassandra.Client(new TBinaryProtocol(transport)); creationTimestamp = Timestamp.Now; - - try - { - OpenTransport(); - } - catch (Exception openException) - { - this.logger.Error(openException, "Error while opening connection. Will try to close."); - TryCloseTransport(); - throw; - } - } - - private void TryCloseTransport() - { - try - { - CloseTransport(); - } - catch (Exception closeException) - { - logger.Error(closeException, "Error while closing connection."); - } + OpenTransport(); } public void Dispose() @@ -132,28 +110,63 @@ private void OpenTransport() if (!cassandraClient.InputProtocol.Transport.Equals(cassandraClient.OutputProtocol.Transport)) cassandraClient.OutputProtocol.Transport.Open(); - if (credentials != null) - cassandraClient.login(new AuthenticationRequest(new Dictionary - { - ["username"] = credentials.Username, - ["password"] = credentials.Password, - })); + WithCloseTransportOnError(Login); + WithCloseTransportOnError(SetKeyspace); + } + } - if (!string.IsNullOrEmpty(keyspaceName)) - cassandraClient.set_keyspace(keyspaceName); + private void WithCloseTransportOnError(Action action) + { + try + { + action(); + } + catch (Exception e) + { + logger.Error(e, "Error occured while opening thrift connection. Will try to close open transports."); + try + { + DoCloseTransport(); + } + catch (Exception closeException) + { + logger.Error(closeException, "Error occured while closing connection's transports."); + } + throw; } } + private void Login() + { + if (credentials != null) + cassandraClient.login(new AuthenticationRequest(new Dictionary + { + ["username"] = credentials.Username, + ["password"] = credentials.Password, + })); + } + + private void SetKeyspace() + { + if (!string.IsNullOrEmpty(keyspaceName)) + cassandraClient.set_keyspace(keyspaceName); + } + private void CloseTransport() { lock (locker) { - cassandraClient.InputProtocol.Transport.Close(); - if (!cassandraClient.InputProtocol.Transport.Equals(cassandraClient.OutputProtocol.Transport)) - cassandraClient.OutputProtocol.Transport.Close(); + DoCloseTransport(); } } + private void DoCloseTransport() + { + cassandraClient.InputProtocol.Transport.Close(); + if (!cassandraClient.InputProtocol.Transport.Equals(cassandraClient.OutputProtocol.Transport)) + cassandraClient.OutputProtocol.Transport.Close(); + } + private Timestamp lastSuccessPingTimestamp; private bool isAlive; From 1845283098b9659f5824fa289b6423c357b6d878 Mon Sep 17 00:00:00 2001 From: iperevoschikov Date: Mon, 5 Oct 2020 12:19:35 +0500 Subject: [PATCH 4/4] log failed action name --- .../CustomNodeTests/AuthenticationTest.cs | 13 +++++++++++-- Cassandra.ThriftClient/Core/ThriftConnection.cs | 8 ++++---- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/Cassandra.ThriftClient.Tests/FunctionalTests/CustomNodeTests/AuthenticationTest.cs b/Cassandra.ThriftClient.Tests/FunctionalTests/CustomNodeTests/AuthenticationTest.cs index 1f9dda3..f1780f0 100644 --- a/Cassandra.ThriftClient.Tests/FunctionalTests/CustomNodeTests/AuthenticationTest.cs +++ b/Cassandra.ThriftClient.Tests/FunctionalTests/CustomNodeTests/AuthenticationTest.cs @@ -1,5 +1,6 @@ using System; using System.IO; +using System.Linq.Expressions; using Cassandra; @@ -94,11 +95,19 @@ public void TestThriftConnectionClosedAfterNonSuccessfulAuthentication() .Returns(logger.Object); logger.Setup(l => l.IsEnabledFor(It.IsAny())) - .Returns(level => level == LogLevel.Error); + .Returns((LogLevel level) => level == LogLevel.Error); - logger.Setup(l => l.Log(It.Is(e => e.MessageTemplate == "Error occured while opening thrift connection. Will try to close open transports."))); + Expression> logAuthFailSetup = l => + l.Log(It.Is( + e => e.Exception is AuthenticationException + && e.MessageTemplate == "Error occured while opening thrift connection. Will try to close open transports. Failed action: {ActionName}." + && e.Properties != null + && e.Properties.ContainsKey("ActionName") + && e.Properties["ActionName"] as string == "login")); + logger.Setup(logAuthFailSetup).Verifiable(); Assert.Throws(() => SomeActionThatRequiresAuthentication("cassandra", "wrong_password", logger.Object)); + logger.Verify(logAuthFailSetup, Times.Exactly(2)); } private void SomeActionThatRequiresAuthentication(string username, string password, ILog logger = null) diff --git a/Cassandra.ThriftClient/Core/ThriftConnection.cs b/Cassandra.ThriftClient/Core/ThriftConnection.cs index 6f38b57..f512c1c 100644 --- a/Cassandra.ThriftClient/Core/ThriftConnection.cs +++ b/Cassandra.ThriftClient/Core/ThriftConnection.cs @@ -110,12 +110,12 @@ private void OpenTransport() if (!cassandraClient.InputProtocol.Transport.Equals(cassandraClient.OutputProtocol.Transport)) cassandraClient.OutputProtocol.Transport.Open(); - WithCloseTransportOnError(Login); - WithCloseTransportOnError(SetKeyspace); + WithCloseTransportOnError(Login, "login"); + WithCloseTransportOnError(SetKeyspace, "set keyspace"); } } - private void WithCloseTransportOnError(Action action) + private void WithCloseTransportOnError(Action action, string actionName) { try { @@ -123,7 +123,7 @@ private void WithCloseTransportOnError(Action action) } catch (Exception e) { - logger.Error(e, "Error occured while opening thrift connection. Will try to close open transports."); + logger.Error(e, "Error occured while opening thrift connection. Will try to close open transports. Failed action: {ActionName}.", new {ActionName = actionName}); try { DoCloseTransport();