You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Sudheesh Katkam <sk...@maprtech.com> on 2015/03/03 01:53:45 UTC
Review Request 31651: DRILL-2322: CSV record reader should log which
file and which record caused an error in the reader
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31651/
-----------------------------------------------------------
Review request for drill and Venki Korukanti.
Repository: drill-git
Description
-------
Adding file split information to logs
Diffs
-----
exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e413921
exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java 7c1f888
Diff: https://reviews.apache.org/r/31651/diff/
Testing
-------
Thanks,
Sudheesh Katkam
Re: Review Request 31651: DRILL-2322: CSV record reader should log
which file and which record caused an error in the reader
Posted by Sudheesh Katkam <sk...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31651/
-----------------------------------------------------------
(Updated March 18, 2015, 6:49 p.m.)
Review request for drill and Venki Korukanti.
Changes
-------
recordsRead => totalRecordsRead
Repository: drill-git
Description
-------
Adding file split information to logs
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e413921
exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java 7c1f888
Diff: https://reviews.apache.org/r/31651/diff/
Testing
-------
Thanks,
Sudheesh Katkam
Re: Review Request 31651: DRILL-2322: CSV record reader should log
which file and which record caused an error in the reader
Posted by Venki Korukanti <ve...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31651/#review76903
-----------------------------------------------------------
Ship it!
Ship It!
- Venki Korukanti
On March 3, 2015, 6:12 p.m., Sudheesh Katkam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31651/
> -----------------------------------------------------------
>
> (Updated March 3, 2015, 6:12 p.m.)
>
>
> Review request for drill and Venki Korukanti.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Adding file split information to logs
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e413921
> exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java 7c1f888
>
> Diff: https://reviews.apache.org/r/31651/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sudheesh Katkam
>
>
Re: Review Request 31651: DRILL-2322: CSV record reader should log
which file and which record caused an error in the reader
Posted by Sudheesh Katkam <sk...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31651/
-----------------------------------------------------------
(Updated March 3, 2015, 6:12 p.m.)
Review request for drill and Venki Korukanti.
Repository: drill-git
Description
-------
Adding file split information to logs
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e413921
exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java 7c1f888
Diff: https://reviews.apache.org/r/31651/diff/
Testing
-------
Thanks,
Sudheesh Katkam
Re: Review Request 31651: DRILL-2322: CSV record reader should log
which file and which record caused an error in the reader
Posted by Sudheesh Katkam <sk...@maprtech.com>.
> On March 3, 2015, 4:03 a.m., Venki Korukanti wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java, line 103
> > <https://reviews.apache.org/r/31651/diff/1/?file=882472#file882472line103>
> >
> > Is it possible to store the reference to FileSplit and create the info string only when an error occurs (inside the catch block)?
Two options, which option is better?
(1) split.getLocations() throws IOException. I thought it would be awkward to have another try..catch in the catch block.
(2) Locations are "list of nodes by name where the data for the split would be local". Do we use data locality?
> On March 3, 2015, 4:03 a.m., Venki Korukanti wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java, line 191
> > <https://reviews.apache.org/r/31651/diff/1/?file=882472#file882472line191>
> >
> > recordCount here refers to the number of records filled in current batch and not the current record position in RecordReader. May be we need to maintain a global count in member variable or get current position through RecordReader.getPos()?
Added a global variable.
- Sudheesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31651/#review74883
-----------------------------------------------------------
On March 3, 2015, 12:53 a.m., Sudheesh Katkam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31651/
> -----------------------------------------------------------
>
> (Updated March 3, 2015, 12:53 a.m.)
>
>
> Review request for drill and Venki Korukanti.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Adding file split information to logs
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e413921
> exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java 7c1f888
>
> Diff: https://reviews.apache.org/r/31651/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sudheesh Katkam
>
>
Re: Review Request 31651: DRILL-2322: CSV record reader should log
which file and which record caused an error in the reader
Posted by Sudheesh Katkam <sk...@maprtech.com>.
> On March 3, 2015, 4:03 a.m., Venki Korukanti wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java, line 103
> > <https://reviews.apache.org/r/31651/diff/1/?file=882472#file882472line103>
> >
> > Is it possible to store the reference to FileSplit and create the info string only when an error occurs (inside the catch block)?
>
> Sudheesh Katkam wrote:
> Two options, which option is better?
> (1) split.getLocations() throws IOException. I thought it would be awkward to have another try..catch in the catch block.
> (2) Locations are "list of nodes by name where the data for the split would be local". Do we use data locality?
Oops.. forgot to add. In option (2), if locations need not be logged, then I can do what you suggested.
- Sudheesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31651/#review74883
-----------------------------------------------------------
On March 3, 2015, 12:53 a.m., Sudheesh Katkam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31651/
> -----------------------------------------------------------
>
> (Updated March 3, 2015, 12:53 a.m.)
>
>
> Review request for drill and Venki Korukanti.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Adding file split information to logs
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e413921
> exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java 7c1f888
>
> Diff: https://reviews.apache.org/r/31651/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sudheesh Katkam
>
>
Re: Review Request 31651: DRILL-2322: CSV record reader should log
which file and which record caused an error in the reader
Posted by Venki Korukanti <ve...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31651/#review74883
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java
<https://reviews.apache.org/r/31651/#comment121704>
Is it possible to store the reference to FileSplit and create the info string only when an error occurs (inside the catch block)?
exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java
<https://reviews.apache.org/r/31651/#comment121705>
This seems like not needed because we have already initialized above.
exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java
<https://reviews.apache.org/r/31651/#comment121706>
Lets make this generic Exception.
exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java
<https://reviews.apache.org/r/31651/#comment121707>
recordCount here refers to the number of records filled in current batch and not the current record position in RecordReader. May be we need to maintain a global count in member variable or get current position through RecordReader.getPos()?
- Venki Korukanti
On March 3, 2015, 12:53 a.m., Sudheesh Katkam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31651/
> -----------------------------------------------------------
>
> (Updated March 3, 2015, 12:53 a.m.)
>
>
> Review request for drill and Venki Korukanti.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Adding file split information to logs
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e413921
> exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java 7c1f888
>
> Diff: https://reviews.apache.org/r/31651/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sudheesh Katkam
>
>