[ https://issues.apache.org/jira/browse/MAPREDUCE-5069?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13606770#comment-13606770 ]
Sandy Ryza commented on MAPREDUCE-5069: --------------------------------------- Hi Sangjin, Overall the patch looks great. A couple questions: * Why is a RecordReaderWrapper needed? Why not just just return the relevant RecordReader directly? * Assuming there's a good reason for the above, can SequenceFileRecordReaderWrapper and TextFileRecordReaderWrapper go as private inner classes into their corresponding CombineFileInputFormats? Is there a reason they need to be public? > add concrete common implementations of CombineFileInputFormat > ------------------------------------------------------------- > > Key: MAPREDUCE-5069 > URL: https://issues.apache.org/jira/browse/MAPREDUCE-5069 > Project: Hadoop Map/Reduce > Issue Type: Improvement > Components: mrv1, mrv2 > Affects Versions: 2.0.3-alpha > Reporter: Sangjin Lee > Priority: Minor > Attachments: MAPREDUCE-5069-1.patch, MAPREDUCE-5069-2.patch, > MAPREDUCE-5069.patch > > > CombineFileInputFormat is abstract, and its specific equivalents to > TextInputFormat, SequenceFileInputFormat, etc. are currently not in the > hadoop code base. > These sound like very common need wherever CombineFileInputFormat is used, > and different folks would write the same code over and over to achieve the > same goal. It sounds very natural for hadoop to provide at least the text and > sequence file implementations of the CombineFileInputFormat class. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira