hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mac Yang" <mac.y...@gmail.com>
Subject Re: Review Request: HIVE-1910: Provide config parameters to control cache object pinning
Date Wed, 26 Jan 2011 06:17:04 GMT


> On 2011-01-25 20:44:24, Carl Steinbach wrote:
> >

Hi Carl, thanks for the review.


> 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.

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?


> 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.

- 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?


- Mac


-----------------------------------------------------------
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