Github user JamesRTaylor commented on a diff in the pull request: https://github.com/apache/phoenix/pull/295#discussion_r180946851 --- 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 -- The SYSTEM.MUTEX table should be created, no? The SYSTEM.MUTEX table isn't a Phoenix table, though, but it should show up in the hbaseTables list, right? We need that to ensure that only a single client is able to run the EXECUTE UPGRADE command.
---