hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Carl Steinbach" <c...@cloudera.com>
Subject Re: Review Request: HIVE-1910: Provide config parameters to control cache object pinning
Date Wed, 26 Jan 2011 22:48:02 GMT


> On 2011-01-25 20:44:24, Carl Steinbach wrote:
> > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java,
line 236
> > <https://reviews.apache.org/r/343/diff/2/?file=10183#file10183line236>
> >
> >     According to javadoc PersistenceManagerFactory.getDataStoreCache() never returns
null, but if it does then we just silently skip this setup. I think we should remove this
null check.
> 
> Mac Yang wrote:
>     Since we call dsc.pinAll() a few lines later, it will throw NPE if the JDO implementation
is not fully compliant with the doc. What do you think if we keep the check and add an else
block with a WARN level log entry?

Good point. I like your solution.


> On 2011-01-25 20:44:24, Carl Steinbach wrote:
> > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java,
line 244
> > <https://reviews.apache.org/r/343/diff/2/?file=10183#file10183line244>
> >
> >     If the user has specified a classname that is not found in the PINCLASSMAP then
we just silently ignore it, which is not a good idea.
> >     
> >     Please add some INFO level logging statements a) echo the METASTORE_CACHE_PINOBJTYPES
value and indicate that this is being used to the set the pin list, and b) log a message if
the list contains a name that is not found in the PINCLASSMAP.
> >     
> >     I also want to suggest that replace the PINCLASSMAP with a list of the actual
classnames, and require METASTORE_CACHE_PINOBJTYPES to specify the actual classname (e.g.
MSerDeInfo, MTable, etc), instead of a name that you transform and then map to the real name.
> 
> Mac Yang wrote:
>     - I will update the patch to include the logging as you suggested.
>     
>     - The reason why I choose to use logical names is to provide some isolation so we
wont break user's config if we ever want to refactor. What are the benefits of using the actual
class names for this config?

Good point about isolation -- I hadn't thought of that. Please add the logging but leave the
rest as is. Thanks.


- Carl


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


On 2011-01-24 11:50:46, Mac Yang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/343/
> -----------------------------------------------------------
> 
> (Updated 2011-01-24 11:50:46)
> 
> 
> Review request for hive.
> 
> 
> Summary
> -------
> 
> Review for HIVE-1910
> 
> 
> This addresses bug HIVE-1910.
>     https://issues.apache.org/jira/browse/HIVE-1910
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
1062921 
>   http://svn.apache.org/repos/asf/hive/trunk/conf/hive-default.xml 1062921 
>   http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
1062921 
> 
> Diff: https://reviews.apache.org/r/343/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Mac
> 
>


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