> On March 10, 2015, 5:58 p.m., Jarek Cecho wrote: > > Overall looks good to me. I have one suggestion though - can we add test > > that will have two tables such that: > > > > 1) First to import doesn't have PK and will use the new parameter to get to > > 1 mapper > > 2) Second will have the PK and we will expect to use the full number of > > mappers > > > > I'm thinking about test that will ensure that when running > > all-tables-import, the reset to single mapper will happen only for the > > affected table and not for other tables that happen to be listed after the > > affected one.
Good catch. Since we are reusing the options that we provide to the import tool, the number of mappers will be reset and set to 1 for subsequent jobs. I have cloned the options to pass to ImportTool. One thing I am trying to do is to validate this from the tests - but that turned out to be lot harder than I thought. I learnt today that mapred.map.tasks is ignored in local mode. So adding a few test mechanics to handle that > On March 10, 2015, 5:58 p.m., Jarek Cecho wrote: > > src/docs/user/import.txt, line 219 > > <https://reviews.apache.org/r/31839/diff/1/?file=888590#file888590line219> > > > > Nit: Trailing whitespace. Will fix On March 10, 2015, 5:58 p.m., Venkat Ranganathan wrote: > > Jarcec Thanks for the review. The addition of new test turned out to be more work but worth it. Will upload a new patch after validating - Venkat ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31839/#review75902 ----------------------------------------------------------- On March 9, 2015, 1:03 a.m., Venkat Ranganathan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31839/ > ----------------------------------------------------------- > > (Updated March 9, 2015, 1:03 a.m.) > > > Review request for Sqoop. > > > Repository: sqoop-trunk > > > Description > ------- > > Provides a new option autoreset-to-one-mapper which instructs sqoop to reset > the number of mappers to one when primary key is not defined for a table and > no split by column is provided (which is the case for import-all-tables). > New tests added and documentation updated > > > Diffs > ----- > > src/docs/user/import-all-tables-purpose.txt d5df88c > src/docs/user/import-all-tables.txt 166825a > src/docs/user/import.txt a6b23df > src/java/org/apache/sqoop/SqoopOptions.java 0070d0b > src/java/org/apache/sqoop/manager/SqlManager.java c81f170 > src/java/org/apache/sqoop/tool/BaseSqoopTool.java b41ee2d > src/java/org/apache/sqoop/tool/ImportTool.java d5bf1eb > src/test/com/cloudera/sqoop/TestAllTables.java 39e8941 > src/test/com/cloudera/sqoop/TestSqoopOptions.java 60460d9 > > Diff: https://reviews.apache.org/r/31839/diff/ > > > Testing > ------- > > All tests pass including newly added tests > > > Thanks, > > Venkat Ranganathan > >
