[SPARK-13792][SQL] Limit logging of bad records in CSVRelation #12173

maropu · 2016-04-05T07:43:12Z

What changes were proposed in this pull request?

Currently in PERMISSIVE and DROPMALFORMED modes we log any record that is going to be ignored. This can generate a lot of logs with large datasets. This pr is to log the parts of malformed records and the number of subsequent records for each partition.
This adds two options as follows;

sqlContext.read
  .format("csv")
  .option("mode", "COUNTMALFORMED")
  .option("maxStoredMalformedPerPartition", 3)
  .load("test.csv").show

A logging message is;

16/04/05 16:42:12 WARN CSVRelation: # of total malformed lines: 25
3 malformed lines extracted and listed as follows;
ab ccc ddd ddd
ab ccc ddd ddd
...

How was this patch tested?

Manual tests done

SparkQA · 2016-04-05T07:55:11Z

Test build #54969 has finished for PR 12173 at commit f458080.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2016-04-05T07:56:42Z

Jenkins, retest this please.

SparkQA · 2016-04-05T09:25:12Z

Test build #54970 has finished for PR 12173 at commit f458080.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2016-04-05T10:22:24Z

@falaki Could you check this to match your intent?

falaki · 2016-04-05T18:08:13Z

I don't think we need a separate parsing mode for this feature. This feature can be part of all existing modes.

maropu · 2016-04-05T23:49:10Z

okay and fixed. Please re-check again? cc: @falaki

SparkQA · 2016-04-06T01:18:03Z

Test build #55054 has finished for PR 12173 at commit 53a95ab.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-04-06T01:20:49Z

Test build #55051 has finished for PR 12173 at commit 2d6be0f.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

falaki · 2016-04-12T01:29:18Z

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVRelation.scala

This can be private[csv]

SparkQA · 2016-04-12T03:08:56Z

Test build #55567 has finished for PR 12173 at commit 01795de.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-04-12T03:15:25Z

Test build #55569 has finished for PR 12173 at commit bd68106.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-04-12T05:11:48Z

Test build #55582 has finished for PR 12173 at commit b8dd628.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-04-12T09:17:02Z

Test build #55598 has finished for PR 12173 at commit a3809ac.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2016-04-12T09:22:21Z

okay, fixed cc: @falaki

maropu · 2016-04-14T07:37:30Z

@falaki ping

maropu · 2016-04-22T09:30:11Z

@falaki ping

falaki · 2016-04-30T07:27:06Z

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVRelation.scala

Do we still want to log this?

This output seems to be noisy to me, so I'll remove this.

falaki · 2016-04-30T07:33:01Z

@maropu sorry for delayed response. I seem to have missed the github notification on this.

maropu · 2016-05-01T02:17:34Z

@falaki okay, I'll fix in a day. thanks!

SparkQA · 2016-05-02T02:57:05Z

Test build #57497 has finished for PR 12173 at commit 4f76109.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-05-02T03:09:43Z

Test build #57499 has finished for PR 12173 at commit 47cccd1.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2016-05-02T04:04:02Z

@falaki okay, fixed.

SparkQA · 2016-05-10T02:16:25Z

Test build #58193 has finished for PR 12173 at commit f2e5f3a.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2016-05-10T02:18:20Z

@falaki ping

maropu · 2016-05-13T08:49:42Z

Jenkins, retest this please.

SparkQA · 2016-05-13T10:14:46Z

Test build #58551 has finished for PR 12173 at commit 0060d6a.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2016-06-02T05:33:47Z

@falaki ping

SparkQA · 2016-06-02T05:42:36Z

Test build #59827 has finished for PR 12173 at commit d517447.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-06-02T07:51:55Z

Test build #59829 has finished for PR 12173 at commit b242a62.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2016-06-04T02:32:06Z

@falaki ping

maropu · 2016-06-11T07:33:23Z

@rxin @falaki ping

rxin · 2016-06-20T07:25:32Z

@maropu thanks for working on this. One thing I don't get is that why we are keeping the malformed lines in memory? Can't we just track a counter and stop logging once the number is greater than a threshold?

maropu · 2016-06-20T07:40:42Z

@rxin yea, the current implementation only holds malformedLineNum malformed lines on memory: https://github.com/apache/spark/pull/12173/files#diff-18b09be18156e81f965df293a2781aefR31
Anything I misunderstood?

rxin · 2016-06-20T07:42:36Z

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CsvUtils.scala

+ */
+private[csv] class MalformedLinesInfo(maxStoreMalformed: Int) extends Serializable {
+
+  var malformedLines = new ArrayBuffer[String]


do we need this?

The some logs of malformed lines seem to be useful for users to understand why this is a malformed one. However, removing this is okay to me.

You can just log them as we find them, can't we? No need to store them in memory?

oh, I see. okay, I'll fix this.

rxin · 2016-06-21T01:14:12Z

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVFileFormat.scala

+      }
+
+      // Appends partition values
+      val fullOutput = requiredSchema.toAttributes ++ partitionSchema.toAttributes


is this change related to the issue you are fixing?

SparkQA · 2016-06-21T02:09:35Z

Test build #60892 has finished for PR 12173 at commit fb6473f.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

## What changes were proposed in this pull request? This pull request adds a new option (maxMalformedLogPerPartition) in CSV reader to limit the maximum of logging message Spark generates per partition for malformed records. The error log looks something like ``` 16/06/20 18:50:14 WARN CSVRelation: Dropping malformed line: adsf,1,4 16/06/20 18:50:14 WARN CSVRelation: Dropping malformed line: adsf,1,4 16/06/20 18:50:14 WARN CSVRelation: Dropping malformed line: adsf,1,4 16/06/20 18:50:14 WARN CSVRelation: Dropping malformed line: adsf,1,4 16/06/20 18:50:14 WARN CSVRelation: Dropping malformed line: adsf,1,4 16/06/20 18:50:14 WARN CSVRelation: Dropping malformed line: adsf,1,4 16/06/20 18:50:14 WARN CSVRelation: Dropping malformed line: adsf,1,4 16/06/20 18:50:14 WARN CSVRelation: Dropping malformed line: adsf,1,4 16/06/20 18:50:14 WARN CSVRelation: Dropping malformed line: adsf,1,4 16/06/20 18:50:14 WARN CSVRelation: Dropping malformed line: adsf,1,4 16/06/20 18:50:14 WARN CSVRelation: More than 10 malformed records have been found on this partition. Malformed records from now on will not be logged. ``` Closes #12173 ## How was this patch tested? Manually tested. Author: Reynold Xin <[email protected]> Closes #13795 from rxin/SPARK-13792. (cherry picked from commit c775bf0) Signed-off-by: Reynold Xin <[email protected]>

maropu force-pushed the SPARK-13792 branch from 2d6be0f to 53a95ab Compare April 5, 2016 23:47

falaki reviewed Apr 12, 2016
View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVRelation.scala Outdated

Copy link

Contributor

falaki Apr 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be private[csv]

maropu force-pushed the SPARK-13792 branch from 53a95ab to 01795de Compare April 12, 2016 01:36

maropu force-pushed the SPARK-13792 branch from b8dd628 to a3809ac Compare April 12, 2016 07:56

falaki reviewed Apr 30, 2016
View reviewed changes

maropu force-pushed the SPARK-13792 branch from a3809ac to 4f76109 Compare May 2, 2016 01:35

maropu force-pushed the SPARK-13792 branch from 47cccd1 to f2e5f3a Compare May 10, 2016 00:49

maropu added 5 commits June 2, 2016 14:30

Add MalformedLinesInfo for storing # of malformed lines

79a7b3b

Drop COUNT_MALFORMED_MODE

5332d50

Add private[csv]

877e4e6

Add tests for MalformedLinesInfo

bd727a8

Apply comments

d810cc3

maropu force-pushed the SPARK-13792 branch from 0060d6a to d517447 Compare June 2, 2016 05:32

Add entries in the documents of CSV options

b242a62

maropu force-pushed the SPARK-13792 branch from d517447 to b242a62 Compare June 2, 2016 05:58

rxin reviewed Jun 20, 2016
View reviewed changes

Apply comments

fb6473f

rxin reviewed Jun 21, 2016
View reviewed changes

rxin mentioned this pull request Jun 21, 2016

[SPARK-13792][SQL] Limit logging of bad records in CSV data source #13795

Closed

asfgit closed this in c775bf0 Jun 21, 2016

maropu deleted the SPARK-13792 branch July 5, 2017 11:47

[SPARK-13792][SQL] Limit logging of bad records in CSVRelation #12173

[SPARK-13792][SQL] Limit logging of bad records in CSVRelation #12173

Uh oh!

Conversation

maropu commented Apr 5, 2016

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

SparkQA commented Apr 5, 2016

Uh oh!

maropu commented Apr 5, 2016

Uh oh!

SparkQA commented Apr 5, 2016

Uh oh!

maropu commented Apr 5, 2016

Uh oh!

falaki commented Apr 5, 2016

Uh oh!

maropu commented Apr 5, 2016

Uh oh!

SparkQA commented Apr 6, 2016

Uh oh!

SparkQA commented Apr 6, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Apr 12, 2016

Uh oh!

SparkQA commented Apr 12, 2016

Uh oh!

SparkQA commented Apr 12, 2016

Uh oh!

SparkQA commented Apr 12, 2016

Uh oh!

maropu commented Apr 12, 2016

Uh oh!

maropu commented Apr 14, 2016

Uh oh!

maropu commented Apr 22, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

falaki commented Apr 30, 2016

Uh oh!

maropu commented May 1, 2016

Uh oh!

SparkQA commented May 2, 2016

Uh oh!

SparkQA commented May 2, 2016

Uh oh!

maropu commented May 2, 2016

Uh oh!

SparkQA commented May 10, 2016

Uh oh!

maropu commented May 10, 2016

Uh oh!

maropu commented May 13, 2016

Uh oh!

SparkQA commented May 13, 2016

Uh oh!

maropu commented Jun 2, 2016

Uh oh!

SparkQA commented Jun 2, 2016

Uh oh!

SparkQA commented Jun 2, 2016

Uh oh!

maropu commented Jun 4, 2016

Uh oh!

maropu commented Jun 11, 2016

Uh oh!

rxin commented Jun 20, 2016

Uh oh!

maropu commented Jun 20, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

maropu commented Jun 20, 2016 •

edited

Loading