From 717c8702bf7090f119c82be1b0461a6dd8a70382 Mon Sep 17 00:00:00 2001 From: Anoop Sam John Date: Sat, 13 Jun 2020 23:19:38 +0530 Subject: [PATCH 1/6] HADOOP-16998 WASB : NativeAzureFsOutputStream#close() throwing java.lang.IllegalArgumentException instead of IOE which causes HBase RS to get aborted. --- .../fs/azure/SyncableDataOutputStream.java | 34 ++++++++++ .../azure/TestSyncableDataOutputStream.java | 66 +++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100644 hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestSyncableDataOutputStream.java diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/SyncableDataOutputStream.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/SyncableDataOutputStream.java index dcfff2fbe3784..f51edee270d16 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/SyncableDataOutputStream.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/SyncableDataOutputStream.java @@ -24,6 +24,8 @@ import org.apache.hadoop.fs.StreamCapabilities; import org.apache.hadoop.fs.Syncable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.apache.hadoop.classification.InterfaceAudience; /** @@ -35,6 +37,8 @@ public class SyncableDataOutputStream extends DataOutputStream implements Syncable, StreamCapabilities { + private static final Logger LOG = LoggerFactory.getLogger(SyncableDataOutputStream.class); + public SyncableDataOutputStream(OutputStream out) { super(out); } @@ -70,4 +74,34 @@ public void hsync() throws IOException { ((Syncable) out).hsync(); } } + + @Override + public void close() throws IOException { + IOException ioeFromFlush = null; + try { + flush(); + } catch (IOException e) { + ioeFromFlush = e; + throw e; + } finally { + try { + this.out.close(); + } catch (IOException e) { + // If there was an Exception during flush(), the Azure SDK will throw back the + // same when we call close on the same stream. When try and finally both throw + // Exception, Java will use Throwable#addSuppressed for one of the Exception so + // that the caller will get one exception back. When within this, if both + // Exceptions are equal, it will throw back IllegalStateException. This makes us + // to throw back a non IOE. The below special handling is to avoid this. + if (ioeFromFlush == e) { + // Do nothing.. + // The close() call gave back the same IOE which flush() gave. Just swallow it + LOG.debug("flush() and close() throwing back same Exception. Just swallowing the latter", e); + } else { + // Let Java handle 2 different Exceptions been thrown from try and finally. + throw e; + } + } + } + } } diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestSyncableDataOutputStream.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestSyncableDataOutputStream.java new file mode 100644 index 0000000000000..da7457287d436 --- /dev/null +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestSyncableDataOutputStream.java @@ -0,0 +1,66 @@ +/* + * 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.hadoop.fs.azure; + +import static org.junit.Assert.fail; + +import java.io.IOException; +import java.io.OutputStream; + +import org.junit.Test; + +public class TestSyncableDataOutputStream { + + @Test + public void testCloseWhenFlushThrowingIOException() throws Exception { + MockOutputStream out = new MockOutputStream(); + SyncableDataOutputStream sdos = new SyncableDataOutputStream(out); + out.flushThrowIOE = true; + try { + sdos.close(); + fail("Expecting IOE"); + } catch (IOException e) { + } + } + + private static class MockOutputStream extends OutputStream { + + boolean flushThrowIOE = false; + private IOException lastException = null; + + @Override + public void write(int arg0) throws IOException { + + } + + @Override + public void flush() throws IOException { + if (this.flushThrowIOE) { + this.lastException = new IOException("An IOE from flush"); + throw this.lastException; + } + } + + @Override + public void close() throws IOException { + if (this.lastException != null) { + throw this.lastException; + } + } + } +} From b0218799109f8a1ed6b1577e6b26b2896206fca3 Mon Sep 17 00:00:00 2001 From: Anoop Sam John Date: Sun, 21 Jun 2020 11:10:47 +0530 Subject: [PATCH 2/6] HADOOP-16998 WASB : NativeAzureFsOutputStream#close() throwing java.lang.IllegalArgumentException instead of IOE which causes HBase RS to get aborted. --- .../hadoop/fs/azure/SyncableDataOutputStream.java | 2 +- .../fs/azure/TestSyncableDataOutputStream.java | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/SyncableDataOutputStream.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/SyncableDataOutputStream.java index f51edee270d16..0011fb7f3bd44 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/SyncableDataOutputStream.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/SyncableDataOutputStream.java @@ -22,11 +22,11 @@ import java.io.IOException; import java.io.OutputStream; +import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.fs.StreamCapabilities; import org.apache.hadoop.fs.Syncable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.apache.hadoop.classification.InterfaceAudience; /** * Support the Syncable interface on top of a DataOutputStream. diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestSyncableDataOutputStream.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestSyncableDataOutputStream.java index da7457287d436..7f00d10d51182 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestSyncableDataOutputStream.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestSyncableDataOutputStream.java @@ -17,11 +17,10 @@ */ package org.apache.hadoop.fs.azure; -import static org.junit.Assert.fail; - import java.io.IOException; import java.io.OutputStream; +import org.apache.hadoop.test.LambdaTestUtils; import org.junit.Test; public class TestSyncableDataOutputStream { @@ -31,11 +30,13 @@ public void testCloseWhenFlushThrowingIOException() throws Exception { MockOutputStream out = new MockOutputStream(); SyncableDataOutputStream sdos = new SyncableDataOutputStream(out); out.flushThrowIOE = true; - try { - sdos.close(); - fail("Expecting IOE"); - } catch (IOException e) { - } + LambdaTestUtils.intercept(IOException.class, "An IOE from flush", () -> sdos.close()); + MockOutputStream out2 = new MockOutputStream(); + out2.flushThrowIOE = true; + LambdaTestUtils.intercept(IOException.class, "An IOE from flush", () -> { + try (SyncableDataOutputStream sdos2 = new SyncableDataOutputStream(out2)) { + } + }); } private static class MockOutputStream extends OutputStream { From 570eca11f1e907be186364247a7bcee6ab7ed46f Mon Sep 17 00:00:00 2001 From: Anoop Sam John Date: Sun, 21 Jun 2020 17:48:50 +0530 Subject: [PATCH 3/6] HADOOP-16998 WASB : NativeAzureFsOutputStream#close() throwing java.lang.IllegalArgumentException instead of IOE which causes HBase RS to get aborted. --- .../apache/hadoop/fs/azure/TestSyncableDataOutputStream.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestSyncableDataOutputStream.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestSyncableDataOutputStream.java index 7f00d10d51182..6a64e952516d8 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestSyncableDataOutputStream.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestSyncableDataOutputStream.java @@ -41,7 +41,7 @@ public void testCloseWhenFlushThrowingIOException() throws Exception { private static class MockOutputStream extends OutputStream { - boolean flushThrowIOE = false; + private boolean flushThrowIOE = false; private IOException lastException = null; @Override From fa29d02edcfb4db83956294825474ab5ae1d7aa8 Mon Sep 17 00:00:00 2001 From: Anoop Sam John Date: Mon, 22 Jun 2020 21:21:19 +0530 Subject: [PATCH 4/6] HADOOP-16998 WASB : NativeAzureFsOutputStream#close() throwing java.lang.IllegalArgumentException instead of IOE which causes HBase RS to get aborted. --- .../apache/hadoop/fs/azure/SyncableDataOutputStream.java | 7 ++++--- .../hadoop/fs/azure/TestSyncableDataOutputStream.java | 1 + 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/SyncableDataOutputStream.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/SyncableDataOutputStream.java index 0011fb7f3bd44..8475588433818 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/SyncableDataOutputStream.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/SyncableDataOutputStream.java @@ -22,12 +22,13 @@ import java.io.IOException; import java.io.OutputStream; -import org.apache.hadoop.classification.InterfaceAudience; -import org.apache.hadoop.fs.StreamCapabilities; -import org.apache.hadoop.fs.Syncable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.hadoop.fs.StreamCapabilities; +import org.apache.hadoop.fs.Syncable; +import org.apache.hadoop.classification.InterfaceAudience; + /** * Support the Syncable interface on top of a DataOutputStream. * This allows passing the sync/hflush/hsync calls through to the diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestSyncableDataOutputStream.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestSyncableDataOutputStream.java index 6a64e952516d8..3d86ead7d7db8 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestSyncableDataOutputStream.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestSyncableDataOutputStream.java @@ -21,6 +21,7 @@ import java.io.OutputStream; import org.apache.hadoop.test.LambdaTestUtils; + import org.junit.Test; public class TestSyncableDataOutputStream { From d72aa7e3e51b656c372c77476551c33ac72c83d3 Mon Sep 17 00:00:00 2001 From: Anoop Sam John Date: Tue, 7 Jul 2020 10:50:27 +0530 Subject: [PATCH 5/6] HADOOP-16998 WASB : NativeAzureFsOutputStream#close() throwing java.lang.IllegalArgumentException instead of IOE which causes HBase RS to get aborted. --- .../org/apache/hadoop/fs/azure/SyncableDataOutputStream.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/SyncableDataOutputStream.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/SyncableDataOutputStream.java index 8475588433818..14ddb02fc4a6b 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/SyncableDataOutputStream.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/SyncableDataOutputStream.java @@ -25,9 +25,9 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.fs.StreamCapabilities; import org.apache.hadoop.fs.Syncable; -import org.apache.hadoop.classification.InterfaceAudience; /** * Support the Syncable interface on top of a DataOutputStream. From e0a8551a17dd552b46230a128bcd84eae6b00f7f Mon Sep 17 00:00:00 2001 From: Steve Loughran Date: Tue, 14 Jul 2020 14:04:32 +0100 Subject: [PATCH 6/6] Update TestSyncableDataOutputStream.java swapped the final two import blocks --- .../apache/hadoop/fs/azure/TestSyncableDataOutputStream.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestSyncableDataOutputStream.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestSyncableDataOutputStream.java index 3d86ead7d7db8..c8c6d93f49d9a 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestSyncableDataOutputStream.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestSyncableDataOutputStream.java @@ -20,10 +20,10 @@ import java.io.IOException; import java.io.OutputStream; -import org.apache.hadoop.test.LambdaTestUtils; - import org.junit.Test; +import org.apache.hadoop.test.LambdaTestUtils; + public class TestSyncableDataOutputStream { @Test