Mailing-List: contact dev-help@phoenix.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@phoenix.apache.org
Date: Fri, 14 Jul 2017 03:01:00 +0000 (UTC)
From: "James Taylor (JIRA)" <jira@apache.org>
To: dev@phoenix.apache.org
Message-ID: <JIRA.13087023.1499981853000.245071.1500001260126@Atlassian.JIRA>
In-Reply-To: <JIRA.13087023.1499981853000@Atlassian.JIRA>
References: <JIRA.13087023.1499981853000@Atlassian.JIRA> <JIRA.13087023.1499981853975@jira-lw-us.apache.org>
Subject: [jira] [Commented] (PHOENIX-4021) Remove CachingHTableFactory
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
archived-at: Fri, 14 Jul 2017 03:01:04 -0000


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