hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Szehon Ho" <sze...@cloudera.com>
Subject Re: Review Request 37591: HIVE-11586 ObjectInspectorFactory.getReflectionObjectInspector is not thread-safe
Date Tue, 18 Aug 2015 23:18:09 GMT


> On Aug. 18, 2015, 10:51 p.m., Szehon Ho wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ReflectionStructObjectInspector.java,
line 165
> > <https://reviews.apache.org/r/37591/diff/2/?file=1043885#file1043885line165>
> >
> >     Very basic question I'm sure you thought of, why can't we synchronize the entire
init() method?  I assume this method will init all the children.  Then we won't need the isFullyInit
methods?
> 
> Jimmy Xiang wrote:
>     Since some type could refer to itself recursively, for example the MyStruct class
in the test, if we synchronize the entire init() method without the isFullyInited method,
we will run into deadlocks.

Ah I see, that's unfortunate.


> On Aug. 18, 2015, 10:51 p.m., Szehon Ho wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ObjectInspectorFactory.java,
line 90
> > <https://reviews.apache.org/r/37591/diff/2/?file=1043884#file1043884line90>
> >
> >     Seems a little risky to me, what if there are some bad GC pause?  
> >     
> >     If it's fully-inited it should get notified , so how about a longer timeout?
> 
> Szehon Ho wrote:
>     Sorry, my question should be, why can't we sleep the thread, and wait for it to get
notified?
> 
> Jimmy Xiang wrote:
>     We have a while loop, so it is fine to take longer than this much time to init. Here,
wait() achieves the same effect as sleep and get notified. It's kind of sleep for up to that
much time unless it is got notified earlier.

I mean, should we wait without a timeout then?  Not sure if we will ever miss a notify?


- Szehon


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37591/#review95789
-----------------------------------------------------------


On Aug. 18, 2015, 9:51 p.m., Jimmy Xiang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37591/
> -----------------------------------------------------------
> 
> (Updated Aug. 18, 2015, 9:51 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-11586
>     https://issues.apache.org/jira/browse/HIVE-11586
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Made ObjectInspectorFactory.getReflectionObjectInspector thread-safe. When it returns,
the object inspector is fully initialized.
> 
> 
> Diffs
> -----
> 
>   serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ObjectInspectorFactory.java
97bb715 
>   serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ReflectionStructObjectInspector.java
78e6066 
>   serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ThriftUnionObjectInspector.java
600abbb 
>   serde/src/test/org/apache/hadoop/hive/serde2/objectinspector/TestReflectionObjectInspectors.java
e2408c6 
> 
> Diff: https://reviews.apache.org/r/37591/diff/
> 
> 
> Testing
> -------
> 
> Unit test
> 
> 
> Thanks,
> 
> Jimmy Xiang
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message