[GitHub] carbondata pull request #2142: [CARBONDATA-2316] Executor task is failed but...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/carbondata/pull/2142


---

[GitHub] carbondata pull request #2142: [CARBONDATA-2316] Executor task is failed but...

[manishgupta88]

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

https://github.com/apache/carbondata/pull/2142#discussion_r180646025
  
--- Diff: 
processing/src/main/java/org/apache/carbondata/processing/merger/AbstractResultProcessor.java
 ---
@@ -37,7 +39,7 @@
* @param resultIteratorList
* @return
*/
-  public abstract boolean execute(List 
resultIteratorList);
+  public abstract boolean execute(List 
resultIteratorList) throws SparkException;
--- End diff --

You can only throw the exception that is sufficient


---

[GitHub] carbondata pull request #2142: [CARBONDATA-2316] Executor task is failed but...

[manishgupta88]

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

https://github.com/apache/carbondata/pull/2142#discussion_r180646899
  
--- Diff: 
processing/src/main/java/org/apache/carbondata/processing/merger/CompactionResultSortProcessor.java
 ---
@@ -165,8 +168,12 @@ public boolean execute(List 
resultIteratorList) {
 readAndLoadDataFromSortTempFiles();
   }
   isCompactionSuccess = true;
-} catch (Exception e) {
+} catch (BadRecordFoundException e) {
+  LOGGER.error(e, "Compaction failed: " + e.getMessage());
+}
+catch (Exception e) {
   LOGGER.error(e, "Compaction failed: " + e.getMessage());
+  throw new SparkException(e.getMessage());
--- End diff --

Remove throwing sparkException from everywhere in the code


---

[GitHub] carbondata pull request #2142: [CARBONDATA-2316] Executor task is failed but...

[manishgupta88]

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

https://github.com/apache/carbondata/pull/2142#discussion_r180645796
  
--- Diff: processing/pom.xml ---
@@ -34,6 +34,11 @@
   
 
   
+
+  org.apache.spark
+  spark-core_${scala.binary.version}
+  ${spark.version}
+
--- End diff --

Remove this dependency. Do not add spark dependency in processing layer 
until and unless really required


---

[GitHub] carbondata pull request #2142: [CARBONDATA-2316] Executor task is failed but...

[manishgupta88]

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

https://github.com/apache/carbondata/pull/2142#discussion_r180646949
  
--- Diff: 
processing/src/main/java/org/apache/carbondata/processing/merger/RowResultMergerProcessor.java
 ---
@@ -157,10 +160,15 @@ public boolean execute(List 
resultIteratorList) {
 this.dataHandler.finish();
   }
   mergeStatus = true;
-} catch (Exception e) {
+} catch (BadRecordFoundException e) {
--- End diff --

Remove this catch block


---

[GitHub] carbondata pull request #2142: [CARBONDATA-2316] Executor task is failed but...

[manishgupta88]

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

https://github.com/apache/carbondata/pull/2142#discussion_r180646852
  
--- Diff: 
processing/src/main/java/org/apache/carbondata/processing/merger/CompactionResultSortProcessor.java
 ---
@@ -165,8 +168,12 @@ public boolean execute(List 
resultIteratorList) {
 readAndLoadDataFromSortTempFiles();
   }
   isCompactionSuccess = true;
-} catch (Exception e) {
+} catch (BadRecordFoundException e) {
--- End diff --

BadRecordFoundException  will never come in case of compaction so remove 
this catch block


---