Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Hive, mail # dev - Review Request: implement a udf to keep hive session alive for certain amount of time


Copy link to this message
-
Re: Review Request: implement a udf to keep hive session alive for certain amount of time
Johnny Zhang 2012-11-09, 23:08


> On Nov. 4, 2012, 4:59 a.m., Mark Grover wrote:
> > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSleep.java, line 36
> > <https://reviews.apache.org/r/7848/diff/1/?file=185096#file185096line36>
> >
> >     I don't have a strong opinion about this but is sleep the right name for this UDF? Sleep is how this UDF keeps the Hive session alive but it might not convey to a user what this UDF does. How about something like session_keep_alive? I am open to other suggestions as well.
> >    
> >     Again, not a deal-breaker:-) However, if you do decide to change the name, don't forget to change all references of "sleep" in the code (log statements, exception messages, etc.).

the reason give a name 'sleep' is because Hadoop used to have a similar example job http://grepcode.com/file/repository.cloudera.com/content/repositories/releases/com.cloudera.hadoop/hadoop-examples/0.20.2-737/org/apache/hadoop/examples/SleepJob.java
which does nothing but keeping running a MR job, which does nothing.
Let's see what's other people's opinion
> On Nov. 4, 2012, 4:59 a.m., Mark Grover wrote:
> > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSleep.java, line 37
> > <https://reviews.apache.org/r/7848/diff/1/?file=185096#file185096line37>
> >
> >     Specify in the explain statement what the units of the duration being specified are (seconds?)

agree, will fix it
> On Nov. 4, 2012, 4:59 a.m., Mark Grover wrote:
> > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSleep.java, line 41
> > <https://reviews.apache.org/r/7848/diff/1/?file=185096#file185096line41>
> >
> >     Better to use GenericUDFSleep.class as argument

agree, will fix it
> On Nov. 4, 2012, 4:59 a.m., Mark Grover wrote:
> > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSleep.java, line 52
> > <https://reviews.apache.org/r/7848/diff/1/?file=185096#file185096line52>
> >
> >     1. A better exception to throw here is UDFArgumentLengthException
> >     2. It's always nice to see as a user what was the expected and the actual value when something goes wrong. Consider printing out the type of the argument received in the exception message. This type can be retrieved by arguments[0].getTypeName()

agree that need to use 'arguments[0].getTypeName()' to print out what's the argument type of user input, will fix it.
> On Nov. 4, 2012, 4:59 a.m., Mark Grover wrote:
> > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSleep.java, line 52
> > <https://reviews.apache.org/r/7848/diff/1/?file=185096#file185096line52>
> >
> >     I am being nitpicky here but a better exception to throw here would be: UDFArgumentTypeException. Also, when seeing an error message as a user, it's always nice to contrast the actual vs. expected. Here is the expected type is int but it will nice to print out the type of the argument that the UDF received. You can retrieve by arguments[i].getTypeName()

agree, will fix it
> On Nov. 4, 2012, 4:59 a.m., Mark Grover wrote:
> > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSleep.java, line 55
> > <https://reviews.apache.org/r/7848/diff/1/?file=185096#file185096line55>
> >
> >     The UDF is returning a Map<Int, Int> even though you don't really want to return anything. I think you should use a void object inspector. For details, look at http://svn.apache.org/viewvc/hive/trunk/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/PrimitiveObjectInspectorFactory.java?view=markup

agree, will change it to return PrimitiveObjectInspectorFactory.writableStringObjectInspector since will print message in the end
> On Nov. 4, 2012, 4:59 a.m., Mark Grover wrote:
> > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSleep.java, line 62

agree, will fix it.

yes, that's the reason. Just want to print something so that people know what's going on, especially when it sleeps for a while.

agree, will fix it

the reason I divide it by 2 is because I found it the evaluate() function run before and during the MR job. As you suggested offline, after adding annotation @UDFType(deterministic = false), I am be able to make it run only during the MR job, so divide by 2 is not needed anymore. Thanks.
- Johnny
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7848/#review13082
On Nov. 3, 2012, 2:56 a.m., Johnny Zhang wrote: