stoty commented on a change in pull request #1394: URL: https://github.com/apache/phoenix/pull/1394#discussion_r813682779
########## File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/SnapshotTestTemplateIT.java ########## @@ -0,0 +1,119 @@ +/* + * 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 static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; +import java.util.Map.Entry; + +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hbase.snapshot.ExportSnapshot; +import org.apache.hadoop.hbase.util.CommonFSUtils; +import org.apache.phoenix.query.BaseTest; +import org.apache.phoenix.query.QueryServices; +import org.apache.phoenix.query.QueryServicesOptions; +import org.apache.phoenix.thirdparty.com.google.common.collect.Maps; +import org.apache.phoenix.util.ReadOnlyProps; +import org.junit.BeforeClass; +import org.junit.Ignore; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * This is a not a standard IT. + * It is starting point for writing ITs that load specific tables from a snapshot. + * Tests based on this IT are meant for debugging specific problems where HBase table snapshots are + * available for replication, and are not meant to be part of the standard test suite + * (or even being committed to the ASF branches) + */ + +@Ignore +@Category(NeedsOwnMiniClusterTest.class) +public class SnapshotTestTemplateIT extends BaseTest { + + private static final Logger LOGGER = LoggerFactory.getLogger( + SnapshotTestTemplateIT.class); + + private static final HashMap<String, String> SNAPSHOTS_TO_LOAD; + + static { + SNAPSHOTS_TO_LOAD = new HashMap<>(); + //Add any HBase tables, including Phoenix System tables + SNAPSHOTS_TO_LOAD.put("SYSTEM.CATALOG_SNAPSHOT", "/path/to/local/snapshot_dir"); + } + + @BeforeClass + public static synchronized void doSetup() throws Exception { + Map<String, String> serverProps = Maps.newHashMapWithExpectedSize(2); + serverProps.put(QueryServices.EXTRA_JDBC_ARGUMENTS_ATTRIB, QueryServicesOptions.DEFAULT_EXTRA_JDBC_ARGUMENTS); + serverProps.put(QueryServices.IS_NAMESPACE_MAPPING_ENABLED, "true"); + Map<String, String> clientProps = Maps.newHashMapWithExpectedSize(2); + clientProps.put(QueryServices.IS_NAMESPACE_MAPPING_ENABLED, "true"); + + //Start minicluster without Phoenix first + checkClusterInitialized(new ReadOnlyProps(serverProps.entrySet().iterator())); + + //load snapshots int HBase + for (Entry<String, String> snapshot : SNAPSHOTS_TO_LOAD.entrySet()) { + importSnapshot(snapshot.getKey(), snapshot.getValue()); + } + + //Now we can start Phoenix + setUpTestDriver(new ReadOnlyProps(serverProps.entrySet().iterator()), new ReadOnlyProps(clientProps.entrySet() + .iterator())); + } + + private static void importSnapshot(String key, String value) throws IOException { + LOGGER.info("importing {} snapshot from {}", key, value); + // copy local snapshot dir to Minicluster HDFS + Path localPath = new Path(value); + assertTrue(FileSystem.getLocal(config).exists(new Path(localPath, ".hbase-snapshot"))); + FileSystem hdfsFs = FileSystem.get(config); + Path hdfsImportPath = new Path(hdfsFs.getHomeDirectory(), "snapshot-import" + "/" + key + "/"); + assertTrue(hdfsFs.mkdirs(hdfsImportPath)); + hdfsFs.copyFromLocalFile(localPath, hdfsImportPath); + hdfsImportPath = new Path(hdfsImportPath, localPath.getName()); + assertTrue(hdfsFs.exists(new Path(hdfsImportPath, ".hbase-snapshot"))); + + //import the snapshot + ExportSnapshot exportTool = new ExportSnapshot(); Review comment: Looking at the code, I couldn't find any reference to starting a MiniMRCluster in BaseTest, and MiniHbaseCluster doesn't seem start it either, so I guess that it's the local runner. We use MR the same way in all the bulkload, indextool, etc ITs. ########## File path: phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java ########## @@ -4142,12 +4124,12 @@ private PhoenixConnection upgradeOtherSystemTablesIfRequired( // by this point we would have already taken mutex lock, hence // we can proceed with creation of snapshots and add table to // snapshot entries in systemTableToSnapshotMap + metaConnection = upgradeSystemStats(metaConnection, + systemTableToSnapshotMap); Review comment: Probably not. I'm gonna revert it, and re-run the tests. ########## File path: phoenix-core/src/main/java/org/apache/phoenix/util/MetaDataUtil.java ########## @@ -275,7 +277,9 @@ public static long encodeVersion(String hbaseVersionStr, Configuration config) { long version = // Encode HBase major, minor, patch version (hbaseVersion << (Byte.SIZE * 5)) - // Encode if systemMappingEnabled are enabled on the server side + // Encode if table namespace mapping is enabled on the server side + // Note that we DO NOT return information on whether system tables are mapped + // on the server side Review comment: I'm quite hazy of what is the use case for disabling NS mapping for system tables, but the fact that is a purely client side setting, and having it set (or not set) on a single client triggers a cluster-wide upgrade gives me a pause. I expected it to be handled the same way as isTableNamespaceMappingEnabled, to ensure consistency between client and server. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
