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