[GitHub] incubator-hawq pull request #1334: HAWQ-1584. Don't ignore exceptions during...

[lavjain]

Github user lavjain closed the pull request at:

https://github.com/apache/incubator-hawq/pull/1334


---

[GitHub] incubator-hawq pull request #1334: HAWQ-1584. Don't ignore exceptions during...

[lavjain]

Github user lavjain commented on a diff in the pull request:

https://github.com/apache/incubator-hawq/pull/1334#discussion_r166122917
  
--- Diff: 
pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/rest/WritableResource.java
 ---
@@ -143,36 +142,38 @@ private static synchronized Response 
synchronizedWriteResponse(Bridge bridge,
 
 private static Response writeResponse(Bridge bridge,
   String path,
-  InputStream inputStream) throws 
Exception {
-
-String returnMsg;
-
+  InputStream inputStream)
+throws Exception {
 // Open the output file
 bridge.beginIteration();
-
 long totalWritten = 0;
+Exception ex = null;
 
 // dataStream will close automatically in the end of the try.
 // inputStream is closed by dataStream.close().
 try (DataInputStream dataStream = new 
DataInputStream(inputStream)) {
 while (bridge.setNext(dataStream)) {
 ++totalWritten;
 }
-} catch (ClientAbortException e) {
-LOG.debug("Remote connection closed by HAWQ", e);
-} catch (Exception ex) {
-LOG.debug("totalWritten so far " + totalWritten + " to " + 
path);
-throw ex;
+} catch (ClientAbortException cae) {
+LOG.error("Remote connection closed by HAWQ", cae);
+} catch (Exception e) {
+LOG.error("Exception: totalWritten so far " + totalWritten + " 
to " + path, e);
+ex = e;
 } finally {
 try {
 bridge.endIteration();
 } catch (Exception e) {
-// ignore ... any significant errors should already have 
been handled
+if (ex == null)
+ex = e;
 }
+// propagate any exceptions
+if (ex != null)
+throw ex;
 }
 
 String censuredPath = Utilities.maskNonPrintables(path);
-returnMsg = "wrote " + totalWritten + " bulks to " + censuredPath;
+String returnMsg = "wrote " + totalWritten + " bulks to " + 
censuredPath;
--- End diff --

returnMsg is also being used in the response.


---

[GitHub] incubator-hawq pull request #1334: HAWQ-1584. Don't ignore exceptions during...

[denalex]

Github user denalex commented on a diff in the pull request:

https://github.com/apache/incubator-hawq/pull/1334#discussion_r166099466
  
--- Diff: 
pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/rest/WritableResource.java
 ---
@@ -143,36 +142,38 @@ private static synchronized Response 
synchronizedWriteResponse(Bridge bridge,
 
 private static Response writeResponse(Bridge bridge,
   String path,
-  InputStream inputStream) throws 
Exception {
-
-String returnMsg;
-
+  InputStream inputStream)
+throws Exception {
 // Open the output file
 bridge.beginIteration();
-
 long totalWritten = 0;
+Exception ex = null;
 
 // dataStream will close automatically in the end of the try.
 // inputStream is closed by dataStream.close().
 try (DataInputStream dataStream = new 
DataInputStream(inputStream)) {
 while (bridge.setNext(dataStream)) {
 ++totalWritten;
 }
-} catch (ClientAbortException e) {
-LOG.debug("Remote connection closed by HAWQ", e);
-} catch (Exception ex) {
-LOG.debug("totalWritten so far " + totalWritten + " to " + 
path);
-throw ex;
+} catch (ClientAbortException cae) {
+LOG.error("Remote connection closed by HAWQ", cae);
+} catch (Exception e) {
+LOG.error("Exception: totalWritten so far " + totalWritten + " 
to " + path, e);
+ex = e;
 } finally {
 try {
 bridge.endIteration();
 } catch (Exception e) {
-// ignore ... any significant errors should already have 
been handled
+if (ex == null)
+ex = e;
--- End diff --

Another way would be to preserve throwing the exception inside original 
catch block (line 162), then here you would say
```
if (ex == null) 
 throw e;
 else 
 throw ex;
``` 
and you would not need the block below (lines 170-172) as the original will 
still be thrown if endIterations() completes without an error.


---

[GitHub] incubator-hawq pull request #1334: HAWQ-1584. Don't ignore exceptions during...

[lavjain]

Github user lavjain closed the pull request at:

https://github.com/apache/incubator-hawq/pull/1334


---

[GitHub] incubator-hawq pull request #1334: HAWQ-1584. Don't ignore exceptions during...

[lavjain]

GitHub user lavjain reopened a pull request:

https://github.com/apache/incubator-hawq/pull/1334

HAWQ-1584. Don't ignore exceptions during bridge.endIteration when writing 
to HDFS



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/lavjain/incubator-hawq fix_writable

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/incubator-hawq/pull/1334.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1334


commit b0e86b7d89f5087582ad89d405ebbba6d962241d
Author: lavjain 
Date:   2018-01-30T07:16:14Z

HAWQ-1584. Do not ignore exceptions during bridge.endIteration when writing 
to HDFS




---