phoenix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ChinmaySKulkarni <...@git.apache.org>
Subject [GitHub] phoenix pull request #295: PHOENIX-4579: Add a config to conditionally creat...
Date Thu, 12 Apr 2018 04:07:32 GMT
Github user ChinmaySKulkarni commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/295#discussion_r180960365
  
    --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/SystemCatalogCreationOnConnectionIT.java
---
    @@ -0,0 +1,577 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.phoenix.end2end;
    +
    +import org.apache.hadoop.conf.Configuration;
    +import org.apache.hadoop.hbase.HBaseTestingUtility;
    +import org.apache.hadoop.hbase.HConstants;
    +import org.apache.hadoop.hbase.TableName;
    +import org.apache.phoenix.coprocessor.MetaDataProtocol;
    +import org.apache.phoenix.exception.SQLExceptionCode;
    +import org.apache.phoenix.exception.UpgradeRequiredException;
    +import org.apache.phoenix.jdbc.PhoenixConnection;
    +import org.apache.phoenix.jdbc.PhoenixEmbeddedDriver;
    +import org.apache.phoenix.jdbc.PhoenixTestDriver;
    +import org.apache.phoenix.query.*;
    +import org.apache.phoenix.util.ReadOnlyProps;
    +import org.apache.phoenix.util.UpgradeUtil;
    +import org.junit.After;
    +import org.junit.Before;
    +import org.junit.Test;
    +import org.junit.experimental.categories.Category;
    +
    +import java.io.IOException;
    +import java.sql.Connection;
    +import java.sql.SQLException;
    +import java.util.*;
    +import java.util.concurrent.TimeoutException;
    +
    +import static org.junit.Assert.*;
    +
    +@Category(NeedsOwnMiniClusterTest.class)
    +public class SystemCatalogCreationOnConnectionIT {
    +    private HBaseTestingUtility testUtil = null;
    +    private Set<String> hbaseTables;
    +    private static boolean setOldTimestampToInduceUpgrade = false;
    +    private static int countUpgradeAttempts;
    +    // This flag is used to figure out if the SYSCAT schema was actually upgraded or
not, based on the timestamp of SYSCAT
    +    // (different from an upgrade attempt)
    +    private static int actualSysCatUpgrades;
    +    private static final String PHOENIX_NAMESPACE_MAPPED_SYSTEM_CATALOG = "SYSTEM:CATALOG";
    +    private static final String PHOENIX_SYSTEM_CATALOG = "SYSTEM.CATALOG";
    +    private static final String EXECUTE_UPGRADE_COMMAND = "EXECUTE UPGRADE";
    +
    +    private static final Set<String> PHOENIX_SYSTEM_TABLES = new HashSet<>(Arrays.asList(
    +      "SYSTEM.CATALOG", "SYSTEM.SEQUENCE", "SYSTEM.STATS", "SYSTEM.FUNCTION",
    +      "SYSTEM.MUTEX"));
    +
    +    private static final Set<String> PHOENIX_NAMESPACE_MAPPED_SYSTEM_TABLES = new
HashSet<>(
    +      Arrays.asList("SYSTEM:CATALOG", "SYSTEM:SEQUENCE", "SYSTEM:STATS", "SYSTEM:FUNCTION",
    +        "SYSTEM:MUTEX"));
    +
    +    private static class PhoenixSysCatCreationServices extends ConnectionQueryServicesImpl
{
    +
    +        public PhoenixSysCatCreationServices(QueryServices services, PhoenixEmbeddedDriver.ConnectionInfo
connectionInfo, Properties info) {
    +            super(services, connectionInfo, info);
    +        }
    +
    +        @Override
    +        protected void setUpgradeRequired() {
    +            super.setUpgradeRequired();
    +            countUpgradeAttempts++;
    +        }
    +
    +        @Override
    +        protected long getSystemTableVersion() {
    +            if (setOldTimestampToInduceUpgrade) {
    +                // Return the next lower version where an upgrade was performed to induce
setting the upgradeRequired flag
    +                return MetaDataProtocol.getPriorUpgradeVersion();
    +            }
    +            return MetaDataProtocol.MIN_SYSTEM_TABLE_TIMESTAMP;
    +        }
    +
    +        @Override
    +        protected PhoenixConnection upgradeSystemCatalogIfRequired(PhoenixConnection
metaConnection,
    +          long currentServerSideTableTimeStamp) throws InterruptedException, SQLException,
TimeoutException, IOException {
    +            PhoenixConnection newMetaConnection = super.upgradeSystemCatalogIfRequired(metaConnection,
currentServerSideTableTimeStamp);
    +            if (currentServerSideTableTimeStamp < MetaDataProtocol.MIN_SYSTEM_TABLE_TIMESTAMP)
{
    +                actualSysCatUpgrades++;
    +            }
    +            return newMetaConnection;
    +        }
    +    }
    +    
    +    public static class PhoenixSysCatCreationTestingDriver extends PhoenixTestDriver
{
    +        private ConnectionQueryServices cqs;
    +        private final ReadOnlyProps overrideProps;
    +    
    +        public PhoenixSysCatCreationTestingDriver(ReadOnlyProps props) {
    +            overrideProps = props;
    +        }
    +    
    +        @Override // public for testing
    +        public synchronized ConnectionQueryServices getConnectionQueryServices(String
url, Properties info) throws SQLException {
    +            if (cqs == null) {
    +                cqs = new PhoenixSysCatCreationServices(new QueryServicesTestImpl(getDefaultProps(),
overrideProps), ConnectionInfo.create(url), info);
    +                cqs.init(url, info);
    +            }
    +            return cqs;
    +        }
    +    
    +        // NOTE: Do not use this if you want to try re-establishing a connection from
the client using a previously
    +        // used ConnectionQueryServices instance. This is used only in cases where we
need to test server-side
    +        // changes and don't care about client-side properties set from the init method.
    +        // Reset the Connection Query Services instance so we can create a new connection
to the cluster
    +        public void resetCQS() {
    +            cqs = null;
    +        }
    +    }
    +
    +
    +    @Before
    +    public void resetVariables() {
    +        setOldTimestampToInduceUpgrade = false;
    +        countUpgradeAttempts = 0;
    +        actualSysCatUpgrades = 0;
    +    }
    +
    +    @After
    +    public void tearDownMiniCluster() {
    +        try {
    +            if (testUtil != null) {
    +                testUtil.shutdownMiniCluster();
    +                testUtil = null;
    +            }
    +        } catch (Exception e) {
    +            // ignore
    +        }
    +    }
    +
    +
    +     // Conditions: isDoNotUpgradePropSet is true
    +     // Expected: We do not create SYSTEM.CATALOG even if this is the first connection
to the server
    +    @Test
    +    public void firstConnectionDoNotUpgradePropSet() throws Exception {
    +        startMiniClusterWithToggleNamespaceMapping(Boolean.FALSE.toString());
    +        Properties propsDoNotUpgradePropSet = new Properties();
    +        // Set doNotUpgradeProperty to true
    +        UpgradeUtil.doNotUpgradeOnFirstConnection(propsDoNotUpgradePropSet);
    +        SystemCatalogCreationOnConnectionIT.PhoenixSysCatCreationTestingDriver driver
=
    +          new SystemCatalogCreationOnConnectionIT.PhoenixSysCatCreationTestingDriver(ReadOnlyProps.EMPTY_PROPS);
    +        try {
    +            driver.getConnectionQueryServices(getJdbcUrl(), propsDoNotUpgradePropSet);
    +            fail("Client should not be able to create SYSTEM.CATALOG since we set the
doNotUpgrade property");
    +        } catch (Exception e) {
    +            assertTrue(e instanceof UpgradeRequiredException);
    +        }
    +        hbaseTables = getHBaseTables();
    +        assertFalse(hbaseTables.contains(PHOENIX_SYSTEM_CATALOG) || hbaseTables.contains(PHOENIX_NAMESPACE_MAPPED_SYSTEM_CATALOG));
    +        assertTrue(hbaseTables.size() == 0);
    +        assertEquals(1, countUpgradeAttempts);
    +    }
    +
    +    // Conditions: isAutoUpgradeEnabled is false
    +    // Expected: We do not create SYSTEM.CATALOG even if this is the first connection
to the server. Later, when we manually
    +    // run "EXECUTE UPGRADE", we create SYSTEM tables (except SYSTEM.MUTEX)
    +    @Test
    +    public void firstConnectionAutoUpgradeDisabled() throws Exception {
    +        startMiniClusterWithToggleNamespaceMapping(Boolean.FALSE.toString());
    +        Map<String, String> props = new HashMap<>();
    +        // Note that the isAutoUpgradeEnabled property is set when instantiating connection
query services, not during init
    +        // Here we disable isAutoUpgradeEnabled
    +        props.put(QueryServices.AUTO_UPGRADE_ENABLED, Boolean.FALSE.toString());
    +        ReadOnlyProps readOnlyProps = new ReadOnlyProps(props);
    +        SystemCatalogCreationOnConnectionIT.PhoenixSysCatCreationTestingDriver driver
=
    +          new SystemCatalogCreationOnConnectionIT.PhoenixSysCatCreationTestingDriver(readOnlyProps);
    +        try {
    +            driver.getConnectionQueryServices(getJdbcUrl(), new Properties());
    +            fail("Client should not be able to create SYSTEM.CATALOG since we set the
isAutoUpgradeEnabled property to false");
    +        } catch (Exception e) {
    +            assertTrue(e instanceof UpgradeRequiredException);
    +        }
    +        hbaseTables = getHBaseTables();
    +        assertFalse(hbaseTables.contains(PHOENIX_SYSTEM_CATALOG) || hbaseTables.contains(PHOENIX_NAMESPACE_MAPPED_SYSTEM_CATALOG));
    +        assertTrue(hbaseTables.size() == 0);
    +        assertEquals(1, countUpgradeAttempts);
    +
    +        // We use the same ConnectionQueryServices instance to run "EXECUTE UPGRADE"
    +        Connection conn = driver.getConnectionQueryServices(getJdbcUrl(), new Properties()).connect(getJdbcUrl(),
new Properties());
    +        try {
    +            conn.createStatement().execute(EXECUTE_UPGRADE_COMMAND);
    +        } catch (Exception e) {
    +            fail("EXECUTE UPGRADE should not fail");
    +        } finally {
    +            conn.close();
    +        }
    +        hbaseTables = getHBaseTables();
    +        assertTrue(PHOENIX_SYSTEM_TABLES.containsAll(hbaseTables));
    +        // SYSTEM.MUTEX table is not created
    --- End diff --
    
    @JamesRTaylor In the current (master) implementation of upgradeSystemTables, we only create
SYSMUTEX in case we catch a TableAlreadyExistsException [link](https://github.com/apache/phoenix/blob/master/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java#L2683)
when creating SYSCAT. Why don't we create SYSMUTEX (but not acquire a lock) in case of a NewerTableAlreadyExistsException
or in case SYSCAT is successfully created?
    
    I guess this is because before this JIRA: Since we only intended to use upgradeSystemTables
to actually upgrade SYSTEM tables and not to create them for the first time, SYSMUTEX was
already created at this point. Is my assumption correct?
    
    With this JIRA, we now also use upgradeSystemTables to create all SYSTEM tables for the
first time,  so we should explicitly always try to create SYSMUTEX too.
    
    How about we call createSysMutexTableIfNotExists before trying to create SYSCAT and then
only acquire the upgrade mutex in case of a migration and/or upgrade? We won't need this lock
before creation of other SYSTEM tables since competing clients will get TableAlreadyExistsException
(consistent with current master implementation too).


---

Mime
View raw message