[incubator-gobblin] branch master updated: [GOBBLIN-1338] Log and suppress exceptions in FsDataWriter.getFinalState

[hu...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

hutran pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-gobblin.git


The following commit(s) were added to refs/heads/master by this push:
     new 38765e7  [GOBBLIN-1338] Log and suppress exceptions in FsDataWriter.getFinalState
38765e7 is described below

commit 38765e7dc0010637c58f8e2072799fd2a9adbd9f
Author: Hung Tran <hu...@linkedin.com>
AuthorDate: Tue Dec 15 09:39:19 2020 -0800

    [GOBBLIN-1338] Log and suppress exceptions in FsDataWriter.getFinalState
    
    Closes #3175 from htran1/final_state_exception
---
 .../org/apache/gobblin/writer/FsDataWriter.java    | 10 +++-
 .../gobblin/writer/PartitionedDataWriter.java      |  2 +-
 .../apache/gobblin/writer/FsDataWriterTest.java    | 63 ++++++++++++++++++++++
 3 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/gobblin-core/src/main/java/org/apache/gobblin/writer/FsDataWriter.java b/gobblin-core/src/main/java/org/apache/gobblin/writer/FsDataWriter.java
index f2b16dd..96d6a69 100644
--- a/gobblin-core/src/main/java/org/apache/gobblin/writer/FsDataWriter.java
+++ b/gobblin-core/src/main/java/org/apache/gobblin/writer/FsDataWriter.java
@@ -318,12 +318,20 @@ public abstract class FsDataWriter<D> implements DataWriter<D>, FinalState, Meta
   public State getFinalState() {
     State state = new State();
 
-    state.setProp("RecordsWritten", recordsWritten());
+    try {
+      state.setProp("RecordsWritten", recordsWritten());
+    } catch (Exception exception) {
+      // If Writer fails to return recordsWritten, it might not be implemented, or implemented incorrectly.
+      // Omit property instead of failing.
+      LOG.warn("Failed to get final state recordsWritten", exception);
+    }
+
     try {
       state.setProp("BytesWritten", bytesWritten());
     } catch (Exception exception) {
       // If Writer fails to return bytesWritten, it might not be implemented, or implemented incorrectly.
       // Omit property instead of failing.
+      LOG.warn("Failed to get final state bytesWritten", exception);
     }
 
     return state;
diff --git a/gobblin-core/src/main/java/org/apache/gobblin/writer/PartitionedDataWriter.java b/gobblin-core/src/main/java/org/apache/gobblin/writer/PartitionedDataWriter.java
index c96cd83..7a14ecb 100644
--- a/gobblin-core/src/main/java/org/apache/gobblin/writer/PartitionedDataWriter.java
+++ b/gobblin-core/src/main/java/org/apache/gobblin/writer/PartitionedDataWriter.java
@@ -355,7 +355,7 @@ public class PartitionedDataWriter<S, D> extends WriterWrapper<D> implements Fin
       state.setProp("RecordsWritten", recordsWritten());
       state.setProp("BytesWritten", bytesWritten());
     } catch (Exception exception) {
-      log.warn("Failed to get final state." + exception.getMessage());
+      log.warn("Failed to get final state.", exception);
       // If Writer fails to return bytesWritten, it might not be implemented, or implemented incorrectly.
       // Omit property instead of failing.
     }
diff --git a/gobblin-core/src/test/java/org/apache/gobblin/writer/FsDataWriterTest.java b/gobblin-core/src/test/java/org/apache/gobblin/writer/FsDataWriterTest.java
new file mode 100644
index 0000000..872b7b2
--- /dev/null
+++ b/gobblin-core/src/test/java/org/apache/gobblin/writer/FsDataWriterTest.java
@@ -0,0 +1,63 @@
+/*
+ * 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.gobblin.writer;
+
+import java.io.IOException;
+import org.apache.gobblin.configuration.State;
+import org.mockito.Mockito;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+
+/**
+ * Test {@link FsDataWriter}
+ */
+public class FsDataWriterTest {
+
+  @Test
+  public void testExceptionOnGetRecordsWritten() throws IOException {
+    FsDataWriter writer = Mockito.mock(FsDataWriter.class);
+    final long BYTES_WRITTEN = 1000;
+
+    Mockito.when(writer.getFinalState()).thenCallRealMethod();
+    Mockito.when(writer.recordsWritten()).thenThrow(new RuntimeException("Test exception"));
+    Mockito.when(writer.bytesWritten()).thenReturn(BYTES_WRITTEN);
+
+
+    State finalState = writer.getFinalState();
+
+    Assert.assertNull(finalState.getProp("RecordsWritten"));
+    Assert.assertEquals(finalState.getPropAsLong("BytesWritten"), BYTES_WRITTEN);
+  }
+
+  @Test
+  public void testExceptionOnGetBytesWritten() throws IOException {
+    FsDataWriter writer = Mockito.mock(FsDataWriter.class);
+    final long RECORDS_WRITTEN = 1000;
+
+    Mockito.when(writer.getFinalState()).thenCallRealMethod();
+    Mockito.when(writer.bytesWritten()).thenThrow(new RuntimeException("Test exception"));
+    Mockito.when(writer.recordsWritten()).thenReturn(RECORDS_WRITTEN);
+
+
+    State finalState = writer.getFinalState();
+
+    Assert.assertNull(finalState.getProp("BytesWritten"));
+    Assert.assertEquals(finalState.getPropAsLong("RecordsWritten"), RECORDS_WRITTEN);
+  }
+}