----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17502/#review33138 -----------------------------------------------------------
Ship it! Looks good. Regarding the jvmname, I have teste the jvm name on Mac/Linux and Windows as part of an earlier project. So should be good - Venkat Ranganathan On Jan. 29, 2014, 5:35 p.m., Jarek Cecho wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17502/ > ----------------------------------------------------------- > > (Updated Jan. 29, 2014, 5:35 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-1273 > https://issues.apache.org/jira/browse/SQOOP-1273 > > > Repository: sqoop-trunk > > > Description > ------- > > I've tweaked the algorithm described on JIRA a bit to lower the probability > of collision: > > * I've increased the time precision from milliseconds to nanoseconds. > * I've changed semantics of tableName to be general salt and rather then > passing null in query based import, I'm using query's hash code. > * Per Venkat's suggestion I've added JVM name that at least on linux uses > Process id and hostname. Not sure what exactly will be there on other > platforms such as Windows. > > So with those addition to get collision, user would have to submit two Sqoop > jobs at the same millisecond for exactly the same table or query on different > machines that do have the same hostname and process id. Seems significantly > less likely :-) > > > Diffs > ----- > > src/java/org/apache/sqoop/tool/ImportTool.java > 59279b6909f4a74e44385ba7a75321c7cf1eab13 > src/java/org/apache/sqoop/util/AppendUtils.java > d7cd6c53054c01226346ee2ebe85043cc718323a > src/test/com/cloudera/sqoop/TestAppendUtils.java > ed66b4aeb58a7ec245126b077101d93544221cfc > > Diff: https://reviews.apache.org/r/17502/diff/ > > > Testing > ------- > > Added AppendUtils test that will exercise --query based import. > > > Thanks, > > Jarek Cecho > >
