[ 
https://issues.apache.org/jira/browse/PHOENIX-4021?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16086768#comment-16086768
 ] 

James Taylor commented on PHOENIX-4021:
---------------------------------------

Looks like ParallelWriterIndexCommitter would already use 
IndexWriterUtils#CoprocessorHConnectionTableFactory based on calling 
IndexWriterUtils.getDefaultDelegateHTableFactory(env) in the setup method. The 
factory caches an HConnection, so we should be ok there. I think the only thing 
we'd need to do would be to clone the config, override the RPC controller 
factory, and provide a version of 
IndexWriterUtils.getDefaultDelegateHTableFactory(env) that passes in the config
{code}
    @Override
    public void setup(IndexWriter parent, RegionCoprocessorEnvironment env, 
String name) {
        this.env = env;
        Configuration conf = env.getConfiguration();
        // Clone config and setup RpcControllerFactory so that RS-RS index 
update calls
        // are higher priority than normal RPC calls to prevent deadlocks.
        conf = PropertiesUtil.cloneConfig(conf);
        conf.setClass(RpcControllerFactory.CUSTOM_CONTROLLER_CONF_KEY,
            InterRegionServerIndexRpcControllerFactory.class, 
RpcControllerFactory.class);
        setup(IndexWriterUtils.getDefaultDelegateHTableFactory(env, conf),
{code}

[~samarthjain] - since we're already caching the HConnection in the factory, I 
think we should be ok.


> Remove CachingHTableFactory
> ---------------------------
>
>                 Key: PHOENIX-4021
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-4021
>             Project: Phoenix
>          Issue Type: Bug
>    Affects Versions: 4.11.0
>            Reporter: Geoffrey Jacoby
>            Assignee: Geoffrey Jacoby
>              Labels: globalMutableSecondaryIndex
>             Fix For: 4.12.0
>
>         Attachments: PHOENIX-4021.patch
>
>
> CachingHTableFactory is used as a performance optimization when writing to 
> global indexes so that HTable instances are cached and later automatically 
> cleaned up, rather than instantiated each time we write to an index.
> This should be removed for two reasons:
> 1. It opens us up to race conditions, because HTables aren't threadsafe, but 
> CachingHTableFactory doesn't guard against two threads both grabbing the same 
> HTable and using it simultaneously. Since all ops going through a region 
> share the same IndexWriter and ParallelWriterIndexCommitter, and hence the 
> same CachingHTableFactory, that means separate operations can both be holding 
> the same HTable. 
> 2. According to discussion on PHOENIX-3159, and offline discussions I've had 
> with [~apurtell], HBase 1.x and above make creating throwaway HTable 
> instances cheap so the caching is no longer needed.
> For 4.x-HBase-1.x and master, we should remove CachingHTableFactory, and for 
> 4.x-HBase-0.98, we should either get rid of it (if it's not too much of a 
> perf hit) or at least make it threadsafe.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to