You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by pt...@apache.org on 2022/10/04 16:36:08 UTC
[ignite] branch master updated: IGNITE-17815 Java thin: Fix RetryPolicy exception handling, propagate exceptions to API caller (#10288)
This is an automated email from the ASF dual-hosted git repository.
ptupitsyn pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/master by this push:
new 3306f5b27b4 IGNITE-17815 Java thin: Fix RetryPolicy exception handling, propagate exceptions to API caller (#10288)
3306f5b27b4 is described below
commit 3306f5b27b415494af462fa4e1371cdd1577dd32
Author: Pavel Tupitsyn <pt...@apache.org>
AuthorDate: Tue Oct 4 19:35:57 2022 +0300
IGNITE-17815 Java thin: Fix RetryPolicy exception handling, propagate exceptions to API caller (#10288)
Fix API call hang when there is an exception in `RetryPolicy` implementation. Catch exceptions and propagate to the caller.
---
.../internal/client/thin/ReliableChannel.java | 8 +++++-
.../apache/ignite/client/ExceptionRetryPolicy.java | 28 ++++++++++++++++++
.../org/apache/ignite/client/ReliabilityTest.java | 33 ++++++++++++++++++++++
.../Client/ClientConnectionTest.cs | 29 +++++++++++++++++++
.../Client/TestRetryPolicy.cs | 11 ++++++++
5 files changed, 108 insertions(+), 1 deletion(-)
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/client/thin/ReliableChannel.java b/modules/core/src/main/java/org/apache/ignite/internal/client/thin/ReliableChannel.java
index b92e8770669..e030979f0b1 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/client/thin/ReliableChannel.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/client/thin/ReliableChannel.java
@@ -874,7 +874,13 @@ final class ReliableChannel implements AutoCloseable {
ClientRetryPolicyContext ctx = new ClientRetryPolicyContextImpl(clientCfg, opType, iteration, exception);
- return plc.shouldRetry(ctx);
+ try {
+ return plc.shouldRetry(ctx);
+ }
+ catch (Throwable t) {
+ exception.addSuppressed(t);
+ return false;
+ }
}
/**
diff --git a/modules/core/src/test/java/org/apache/ignite/client/ExceptionRetryPolicy.java b/modules/core/src/test/java/org/apache/ignite/client/ExceptionRetryPolicy.java
new file mode 100644
index 00000000000..b33bdc8e2f8
--- /dev/null
+++ b/modules/core/src/test/java/org/apache/ignite/client/ExceptionRetryPolicy.java
@@ -0,0 +1,28 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.client;
+
+/**
+ * Retry policy that throws an exception.
+ */
+public class ExceptionRetryPolicy implements ClientRetryPolicy {
+ /** {@inheritDoc} */
+ @Override public boolean shouldRetry(ClientRetryPolicyContext context) {
+ throw new RuntimeException("Error in policy.");
+ }
+}
diff --git a/modules/core/src/test/java/org/apache/ignite/client/ReliabilityTest.java b/modules/core/src/test/java/org/apache/ignite/client/ReliabilityTest.java
index 1cf34e84e11..56b4b648958 100644
--- a/modules/core/src/test/java/org/apache/ignite/client/ReliabilityTest.java
+++ b/modules/core/src/test/java/org/apache/ignite/client/ReliabilityTest.java
@@ -23,6 +23,7 @@ import java.util.List;
import java.util.Map;
import java.util.Random;
import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
@@ -224,6 +225,38 @@ public class ReliabilityTest extends AbstractThinClientTest {
}
}
+ /**
+ * Tests retry policy exception handling.
+ */
+ @Test
+ public void testExceptionInRetryPolicyPropagatesToCaller() {
+ if (isPartitionAware())
+ return;
+
+ try (LocalIgniteCluster cluster = LocalIgniteCluster.start(1);
+ IgniteClient client = Ignition.startClient(getClientConfiguration()
+ .setRetryPolicy(new ExceptionRetryPolicy())
+ .setAddresses(
+ cluster.clientAddresses().iterator().next(),
+ cluster.clientAddresses().iterator().next()))
+ ) {
+ ClientCache<Integer, Integer> cache = client.createCache("cache");
+ dropAllThinClientConnections(Ignition.allGrids().get(0));
+
+ Throwable asyncEx = GridTestUtils.assertThrows(null, () -> cache.getAsync(0).get(),
+ ExecutionException.class, "Channel is closed");
+
+ dropAllThinClientConnections(Ignition.allGrids().get(0));
+
+ Throwable syncEx = GridTestUtils.assertThrows(null, () -> cache.get(0),
+ ClientConnectionException.class, "Channel is closed");
+
+ for (Throwable t : new Throwable[] {asyncEx.getCause(), syncEx}) {
+ assertEquals("Error in policy.", t.getSuppressed()[0].getMessage());
+ }
+ }
+ }
+
/**
* Tests that retry limit of 1 effectively disables retry/failover.
*/
diff --git a/modules/platforms/dotnet/Apache.Ignite.Core.Tests/Client/ClientConnectionTest.cs b/modules/platforms/dotnet/Apache.Ignite.Core.Tests/Client/ClientConnectionTest.cs
index 6e5761a1db6..417cf50c91c 100644
--- a/modules/platforms/dotnet/Apache.Ignite.Core.Tests/Client/ClientConnectionTest.cs
+++ b/modules/platforms/dotnet/Apache.Ignite.Core.Tests/Client/ClientConnectionTest.cs
@@ -855,6 +855,35 @@ namespace Apache.Ignite.Core.Tests.Client
}
}
+ /// <summary>
+ /// Test retry policy exception handling.
+ /// </summary>
+ [Test]
+ public void TestExceptionInRetryPolicyPropagatesToCaller([Values(true, false)] bool async)
+ {
+ Ignition.Start(TestUtils.GetTestConfiguration());
+
+ var cfg = new IgniteClientConfiguration
+ {
+ Endpoints = new[] { "127.0.0.1" },
+ RetryPolicy = new TestRetryPolicy { ShouldThrow = true },
+ RetryLimit = 2
+ };
+
+ using (var client = Ignition.StartClient(cfg))
+ {
+ var cache = client.GetOrCreateCache<int, int>("c");
+
+ Ignition.StopAll(true);
+
+ var ex = async
+ ? Assert.CatchAsync(() => cache.GetAsync(0))
+ : Assert.Catch(() => cache.Get(0));
+
+ Assert.AreEqual("Error in policy.", ex.Message);
+ }
+ }
+
/// <summary>
/// Tests that client stops it's receiver thread upon disposal.
/// </summary>
diff --git a/modules/platforms/dotnet/Apache.Ignite.Core.Tests/Client/TestRetryPolicy.cs b/modules/platforms/dotnet/Apache.Ignite.Core.Tests/Client/TestRetryPolicy.cs
index 5f0c75dcd63..c1528dfda3a 100644
--- a/modules/platforms/dotnet/Apache.Ignite.Core.Tests/Client/TestRetryPolicy.cs
+++ b/modules/platforms/dotnet/Apache.Ignite.Core.Tests/Client/TestRetryPolicy.cs
@@ -17,6 +17,7 @@
namespace Apache.Ignite.Core.Tests.Client
{
+ using System;
using System.Collections.Generic;
using System.Linq;
using Apache.Ignite.Core.Client;
@@ -46,11 +47,21 @@ namespace Apache.Ignite.Core.Tests.Client
/// </summary>
public IReadOnlyList<IClientRetryPolicyContext> Invocations => _invocations;
+ /// <summary>
+ /// Gets or sets a value indicating whether this policy should throw an exception.
+ /// </summary>
+ public bool ShouldThrow { get; set; }
+
/** <inheritDoc /> */
public bool ShouldRetry(IClientRetryPolicyContext context)
{
_invocations.Add(context);
+ if (ShouldThrow)
+ {
+ throw new Exception("Error in policy.");
+ }
+
return _allowedOperations.Contains(context.Operation);
}
}