From b563b4b90b4524a9f961a5b1d5f88498932a4ca4 Mon Sep 17 00:00:00 2001 From: Daemonxiao <735462752@qq.com> Date: Mon, 13 Jun 2022 20:27:43 +0800 Subject: [PATCH 1/6] fix TLS reload doesn't work after delete cert file Signed-off-by: Daemonxiao <735462752@qq.com> --- .../org/tikv/common/util/ChannelFactory.java | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/tikv/common/util/ChannelFactory.java b/src/main/java/org/tikv/common/util/ChannelFactory.java index 28d53e93481..8ffdec8008f 100644 --- a/src/main/java/org/tikv/common/util/ChannelFactory.java +++ b/src/main/java/org/tikv/common/util/ChannelFactory.java @@ -89,9 +89,15 @@ public CertWatcher(long pollInterval, List targets, Runnable onChange) { this::tryReload, pollInterval, pollInterval, TimeUnit.SECONDS); } + // If any execution of the task encounters an exception, subsequent executions are suppressed. private void tryReload() { - if (needReload()) { - onChange.run(); + // Add exception handling to avoid schedule stop. + try { + if (needReload()) { + onChange.run(); + } + } catch (Exception e) { + logger.error("Failed to reload cert!" + e); } } @@ -180,11 +186,16 @@ public OpenSslContext(String trustPath, String chainPath, String keyPath) { @Override public SslContextBuilder createSslContextBuilder() { SslContextBuilder builder = GrpcSslContexts.forClient(); - if (trustPath != null) { - builder.trustManager(new File(trustPath)); - } - if (chainPath != null && keyPath != null) { - builder.keyManager(new File(chainPath), new File(keyPath)); + try { + if (trustPath != null) { + builder.trustManager(new File(trustPath)); + } + if (chainPath != null && keyPath != null) { + builder.keyManager(new File(chainPath), new File(keyPath)); + } + } catch (Exception e) { + logger.error("PEM SSL context builder failed!", e); + throw new IllegalArgumentException(e); } return builder; } From 06111e59d10894503a786b605c67d651b1d7be22 Mon Sep 17 00:00:00 2001 From: Daemonxiao <735462752@qq.com> Date: Tue, 14 Jun 2022 14:35:36 +0800 Subject: [PATCH 2/6] update and add unit test Signed-off-by: Daemonxiao <735462752@qq.com> Co-authored-by: iosmanthus --- .../org/tikv/common/util/ChannelFactory.java | 4 +-- .../org/tikv/common/ChannelFactoryTest.java | 35 +++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/tikv/common/util/ChannelFactory.java b/src/main/java/org/tikv/common/util/ChannelFactory.java index 8ffdec8008f..57eda5258e4 100644 --- a/src/main/java/org/tikv/common/util/ChannelFactory.java +++ b/src/main/java/org/tikv/common/util/ChannelFactory.java @@ -97,7 +97,7 @@ private void tryReload() { onChange.run(); } } catch (Exception e) { - logger.error("Failed to reload cert!" + e); + logger.error("Failed to reload cert!", e); } } @@ -194,7 +194,7 @@ public SslContextBuilder createSslContextBuilder() { builder.keyManager(new File(chainPath), new File(keyPath)); } } catch (Exception e) { - logger.error("PEM SSL context builder failed!", e); + logger.error("Failed to create ssl context builder", e); throw new IllegalArgumentException(e); } return builder; diff --git a/src/test/java/org/tikv/common/ChannelFactoryTest.java b/src/test/java/org/tikv/common/ChannelFactoryTest.java index 131943be74d..1ae3d833db9 100644 --- a/src/test/java/org/tikv/common/ChannelFactoryTest.java +++ b/src/test/java/org/tikv/common/ChannelFactoryTest.java @@ -25,7 +25,10 @@ import java.io.File; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import org.junit.Test; import org.tikv.common.util.ChannelFactory; @@ -59,6 +62,38 @@ public void testCertWatcher() throws InterruptedException { assertTrue(changed.get()); } + @Test + public void testCertWatcherWithExceptionTask() throws InterruptedException { + AtomicInteger timesOfReloadTask = new AtomicInteger(0); + AtomicInteger timesOfFakeTask = new AtomicInteger(0); + + Executors.newSingleThreadScheduledExecutor() + .scheduleAtFixedRate( + () -> { + timesOfFakeTask.getAndIncrement(); + System.out.println("Execute fake task for the " + timesOfFakeTask.get() + " times."); + throw new RuntimeException("Mock exception in fake task"); + }, + 1, + 1, + TimeUnit.SECONDS); + + new CertWatcher( + 1, + ImmutableList.of(new File(caPath), new File(clientCertPath), new File(clientKeyPath)), + () -> { + timesOfReloadTask.getAndIncrement(); + touchCert(); + System.out.println("Execute reload task for the " + timesOfReloadTask.get() + " times."); + throw new RuntimeException("Mock exception in reload task"); + }); + + Thread.sleep(5000); + + assertTrue(timesOfFakeTask.get() == 1); + assertTrue(timesOfReloadTask.get() > 1); + } + @Test public void testMultiThreadTlsReload() throws InterruptedException { ChannelFactory factory = createFactory(); From a311d3b558f7633bdd5e870fbb66dc1aedf738d6 Mon Sep 17 00:00:00 2001 From: Daemonxiao <735462752@qq.com> Date: Tue, 14 Jun 2022 15:13:52 +0800 Subject: [PATCH 3/6] update unit test Signed-off-by: Daemonxiao <735462752@qq.com> Co-authored-by: iosmanthus --- src/test/java/org/tikv/common/ChannelFactoryTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/java/org/tikv/common/ChannelFactoryTest.java b/src/test/java/org/tikv/common/ChannelFactoryTest.java index 1ae3d833db9..c3ed67ebcc9 100644 --- a/src/test/java/org/tikv/common/ChannelFactoryTest.java +++ b/src/test/java/org/tikv/common/ChannelFactoryTest.java @@ -71,7 +71,6 @@ public void testCertWatcherWithExceptionTask() throws InterruptedException { .scheduleAtFixedRate( () -> { timesOfFakeTask.getAndIncrement(); - System.out.println("Execute fake task for the " + timesOfFakeTask.get() + " times."); throw new RuntimeException("Mock exception in fake task"); }, 1, @@ -84,7 +83,6 @@ public void testCertWatcherWithExceptionTask() throws InterruptedException { () -> { timesOfReloadTask.getAndIncrement(); touchCert(); - System.out.println("Execute reload task for the " + timesOfReloadTask.get() + " times."); throw new RuntimeException("Mock exception in reload task"); }); From e355142253895bc83d70f243923a40edc6c2d6ac Mon Sep 17 00:00:00 2001 From: Daemonxiao <735462752@qq.com> Date: Tue, 14 Jun 2022 15:25:17 +0800 Subject: [PATCH 4/6] update ChannelFactory.java Signed-off-by: Daemonxiao <735462752@qq.com> Co-authored-by: iosmanthus --- src/main/java/org/tikv/common/util/ChannelFactory.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/tikv/common/util/ChannelFactory.java b/src/main/java/org/tikv/common/util/ChannelFactory.java index 57eda5258e4..20840af7eb5 100644 --- a/src/main/java/org/tikv/common/util/ChannelFactory.java +++ b/src/main/java/org/tikv/common/util/ChannelFactory.java @@ -362,7 +362,9 @@ public void close() { if (certContext != null) { recycler.shutdown(); - certWatcher.close(); + if (certWatcher != null) { + certWatcher.close(); + } } } } From 8e20ffb11e49d87d84c27aabaa7b458cdfc2c705 Mon Sep 17 00:00:00 2001 From: Daemonxiao <735462752@qq.com> Date: Tue, 14 Jun 2022 17:16:35 +0800 Subject: [PATCH 5/6] update ChannelFactoryTest.java Signed-off-by: Daemonxiao <735462752@qq.com> Co-authored-by: iosmanthus --- .../java/org/tikv/common/ChannelFactoryTest.java | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/test/java/org/tikv/common/ChannelFactoryTest.java b/src/test/java/org/tikv/common/ChannelFactoryTest.java index c3ed67ebcc9..00970fb6f70 100644 --- a/src/test/java/org/tikv/common/ChannelFactoryTest.java +++ b/src/test/java/org/tikv/common/ChannelFactoryTest.java @@ -65,18 +65,6 @@ public void testCertWatcher() throws InterruptedException { @Test public void testCertWatcherWithExceptionTask() throws InterruptedException { AtomicInteger timesOfReloadTask = new AtomicInteger(0); - AtomicInteger timesOfFakeTask = new AtomicInteger(0); - - Executors.newSingleThreadScheduledExecutor() - .scheduleAtFixedRate( - () -> { - timesOfFakeTask.getAndIncrement(); - throw new RuntimeException("Mock exception in fake task"); - }, - 1, - 1, - TimeUnit.SECONDS); - new CertWatcher( 1, ImmutableList.of(new File(caPath), new File(clientCertPath), new File(clientKeyPath)), @@ -87,8 +75,6 @@ public void testCertWatcherWithExceptionTask() throws InterruptedException { }); Thread.sleep(5000); - - assertTrue(timesOfFakeTask.get() == 1); assertTrue(timesOfReloadTask.get() > 1); } From fdf0e13c729e479dc2d09e1452642fd9f5949432 Mon Sep 17 00:00:00 2001 From: Daemonxiao <735462752@qq.com> Date: Tue, 14 Jun 2022 17:18:46 +0800 Subject: [PATCH 6/6] update ChannelFactoryTest.java Signed-off-by: Daemonxiao <735462752@qq.com> Co-authored-by: iosmanthus --- src/test/java/org/tikv/common/ChannelFactoryTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/java/org/tikv/common/ChannelFactoryTest.java b/src/test/java/org/tikv/common/ChannelFactoryTest.java index 00970fb6f70..ce071a69653 100644 --- a/src/test/java/org/tikv/common/ChannelFactoryTest.java +++ b/src/test/java/org/tikv/common/ChannelFactoryTest.java @@ -25,8 +25,6 @@ import java.io.File; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.Executors; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong;