From dev-return-52795-archive-asf-public=cust-asf.ponee.io@phoenix.apache.org Thu Jul 5 01:20:09 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id DB09C180674 for ; Thu, 5 Jul 2018 01:20:08 +0200 (CEST) Received: (qmail 8713 invoked by uid 500); 4 Jul 2018 23:20:07 -0000 Mailing-List: contact dev-help@phoenix.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@phoenix.apache.org Delivered-To: mailing list dev@phoenix.apache.org Received: (qmail 8696 invoked by uid 99); 4 Jul 2018 23:20:07 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd1-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 04 Jul 2018 23:20:07 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd1-us-west.apache.org (ASF Mail Server at spamd1-us-west.apache.org) with ESMTP id 8291DC5DFE for ; Wed, 4 Jul 2018 23:20:07 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: -109.501 X-Spam-Level: X-Spam-Status: No, score=-109.501 tagged_above=-999 required=6.31 tests=[ENV_AND_HDR_SPF_MATCH=-0.5, KAM_ASCII_DIVIDERS=0.8, RCVD_IN_DNSWL_MED=-2.3, SPF_PASS=-0.001, USER_IN_DEF_SPF_WL=-7.5, USER_IN_WHITELIST=-100] autolearn=disabled Received: from mx1-lw-us.apache.org ([10.40.0.8]) by localhost (spamd1-us-west.apache.org [10.40.0.7]) (amavisd-new, port 10024) with ESMTP id g_HtdLlaWuJQ for ; Wed, 4 Jul 2018 23:20:06 +0000 (UTC) Received: from mailrelay1-us-west.apache.org (mailrelay1-us-west.apache.org [209.188.14.139]) by mx1-lw-us.apache.org (ASF Mail Server at mx1-lw-us.apache.org) with ESMTP id C74005FBE6 for ; Wed, 4 Jul 2018 23:20:05 +0000 (UTC) Received: from jira-lw-us.apache.org (unknown [207.244.88.139]) by mailrelay1-us-west.apache.org (ASF Mail Server at mailrelay1-us-west.apache.org) with ESMTP id 5A576E1095 for ; Wed, 4 Jul 2018 23:20:04 +0000 (UTC) Received: from jira-lw-us.apache.org (localhost [127.0.0.1]) by jira-lw-us.apache.org (ASF Mail Server at jira-lw-us.apache.org) with ESMTP id D3D2E27530 for ; Wed, 4 Jul 2018 23:20:02 +0000 (UTC) Date: Wed, 4 Jul 2018 23:20:02 +0000 (UTC) From: "ASF GitHub Bot (JIRA)" To: dev@phoenix.apache.org Message-ID: In-Reply-To: References: Subject: [jira] [Commented] (PHOENIX-3534) Support multi region SYSTEM.CATALOG table MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-JIRA-FingerPrint: 30527f35849b9dde25b450d4833f0394 [ https://issues.apache.org/jira/browse/PHOENIX-3534?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16533110#comment-16533110 ] ASF GitHub Bot commented on PHOENIX-3534: ----------------------------------------- Github user JamesRTaylor commented on a diff in the pull request: https://github.com/apache/phoenix/pull/303#discussion_r200206109 --- Diff: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java --- @@ -586,48 +590,359 @@ public void getTable(RpcController controller, GetTableRequest request, builder.setMutationTime(minNonZerodisableIndexTimestamp - 1); } } - - if (table.getTimeStamp() != tableTimeStamp) { + // the PTable of views and indexes on views might get updated because a column is added to one of + // their parents (this won't change the timestamp) + if (table.getType()!=PTableType.TABLE || table.getTimeStamp() != tableTimeStamp) { builder.setTable(PTableImpl.toProto(table)); } done.run(builder.build()); - return; } catch (Throwable t) { logger.error("getTable failed", t); ProtobufUtil.setControllerException(controller, ServerUtil.createIOException(SchemaUtil.getTableName(schemaName, tableName), t)); } } + /** + * Used to add the columns present the ancestor hierarchy to the PTable of the given view or + * view index + * @param table PTable of the view or view index + * @param skipAddingIndexes if true the returned PTable won't include indexes + * @param skipAddingParentColumns if true the returned PTable won't include columns derived from + * ancestor tables + * @param lockedAncestorTable ancestor table table that is being mutated (as we won't be able to + * resolve this table as its locked) + */ + private Pair combineColumns(PTable table, long timestamp, + int clientVersion, boolean skipAddingIndexes, boolean skipAddingParentColumns, + PTable lockedAncestorTable) throws SQLException, IOException { + boolean hasIndexId = table.getViewIndexId() != null; + if (table.getType() != PTableType.VIEW && !hasIndexId) { + return new Pair(table, + MetaDataProtos.MutationCode.TABLE_ALREADY_EXISTS); + } + if (!skipAddingParentColumns) { + table = + addDerivedColumnsFromAncestors(table, timestamp, clientVersion, + lockedAncestorTable); + if (table==null) { + return new Pair(table, + MetaDataProtos.MutationCode.TABLE_NOT_FOUND); + } + // we need to resolve the indexes of views (to get ensure they also have all the columns + // derived from their ancestors) + if (!skipAddingIndexes && !table.getIndexes().isEmpty()) { + List indexes = Lists.newArrayListWithExpectedSize(table.getIndexes().size()); + for (PTable index : table.getIndexes()) { + byte[] tenantIdBytes = + index.getTenantId() == null ? ByteUtil.EMPTY_BYTE_ARRAY + : index.getTenantId().getBytes(); + PTable latestIndex = + doGetTable(tenantIdBytes, index.getSchemaName().getBytes(), + index.getTableName().getBytes(), timestamp, null, clientVersion, true, + false, lockedAncestorTable); + if (latestIndex == null) { + throw new TableNotFoundException( + "Could not find index table while combining columns " + + index.getTableName().getString() + " with tenant id " + + index.getTenantId()); + } + indexes.add(latestIndex); + } + table = PTableImpl.makePTable(table, table.getTimeStamp(), indexes); + } + } + + MetaDataProtos.MutationCode mutationCode = + table != null ? MetaDataProtos.MutationCode.TABLE_ALREADY_EXISTS + : MetaDataProtos.MutationCode.TABLE_NOT_FOUND; + return new Pair(table, mutationCode); + } + + + private PTable addDerivedColumnsFromAncestors(PTable table, long timestamp, + int clientVersion, PTable lockedAncestorTable) throws IOException, SQLException, TableNotFoundException { + // combine columns for view and view indexes + byte[] tenantId = + table.getTenantId() != null ? table.getTenantId().getBytes() + : ByteUtil.EMPTY_BYTE_ARRAY; + byte[] schemaName = table.getSchemaName().getBytes(); + byte[] tableName = table.getTableName().getBytes(); + String fullTableName = SchemaUtil.getTableName(table.getSchemaName().getString(), + table.getTableName().getString()); + boolean hasIndexId = table.getViewIndexId() != null; + boolean isSalted = table.getBucketNum() != null; + if (table.getType() != PTableType.VIEW && !hasIndexId) { + return table; + } + boolean isDiverged = isDivergedView(table); + // here you combine columns from the parent tables the logic is as follows, if the PColumn + // is in the EXCLUDED_COLUMNS remove it, otherwise priority of keeping duplicate columns is + // child -> parent + List ancestorList = Lists.newArrayList(); + TableViewFinderResult viewFinderResult = new TableViewFinderResult(); + if (PTableType.VIEW == table.getType()) { + findAncestorViews(tenantId, schemaName, tableName, viewFinderResult, + table.isNamespaceMapped()); + } else { // is a view index + findAncestorViewsOfIndex(tenantId, schemaName, tableName, viewFinderResult, + table.isNamespaceMapped()); + } + if (viewFinderResult.getResults().isEmpty()) { + // no need to combine columns for local indexes on regular tables + return table; + } + for (TableInfo viewInfo : viewFinderResult.getResults()) { + ancestorList.add(viewInfo); + } + List allColumns = Lists.newArrayList(); + List excludedColumns = Lists.newArrayList(); + // add my own columns first in reverse order + List myColumns = table.getColumns(); + for (int i = myColumns.size() - 1; i >= 0; i--) { + PColumn pColumn = myColumns.get(i); + if (pColumn.isExcluded()) { + excludedColumns.add(pColumn); + } else if (!pColumn.equals(SaltingUtil.SALTING_COLUMN)) { + // skip salted column as it will be added from the base table columns + allColumns.add(pColumn); + } + } + + // initialize map from with indexed expression to list of required data columns + // then remove the data columns that have not been dropped, so that we get the columns that + // have been dropped + Map> indexRequiredDroppedDataColMap = + Maps.newHashMapWithExpectedSize(table.getColumns().size()); + if (hasIndexId) { + int indexPosOffset = (isSalted ? 1 : 0) + (table.isMultiTenant() ? 1 : 0) + 1; + ColumnNameTrackingExpressionCompiler expressionCompiler = + new ColumnNameTrackingExpressionCompiler(); + for (int i = indexPosOffset; i < table.getPKColumns().size(); i++) { + PColumn indexColumn = table.getPKColumns().get(i); + try { + expressionCompiler.reset(); + String expressionStr = IndexUtil.getIndexColumnExpressionStr(indexColumn); + ParseNode parseNode = SQLParser.parseCondition(expressionStr); + parseNode.accept(expressionCompiler); + indexRequiredDroppedDataColMap.put(indexColumn, + Lists.newArrayList(expressionCompiler.getDataColumnNames())); + } catch (SQLException e) { + throw new RuntimeException(e); // Impossible + } + } + } + + // now go up from child to parent all the way to the base table: + PTable baseTable = null; + long maxTableTimestamp = -1; + int numPKCols = table.getPKColumns().size(); + for (int i = 0; i < ancestorList.size(); i++) { + TableInfo parentTableInfo = ancestorList.get(i); + PTable pTable = null; + String fullParentTableName = SchemaUtil.getTableName(parentTableInfo.getSchemaName(), + parentTableInfo.getTableName()); + PName parentTenantId = + (parentTableInfo.getTenantId() != null && parentTableInfo.getTenantId().length!=0) + ? PNameFactory.newName(parentTableInfo.getTenantId()) : null; + PTableKey pTableKey = new PTableKey(parentTenantId, fullParentTableName); + // if we already have the PTable of an ancestor that has been locked, no need to look up + // the table + if (lockedAncestorTable != null && lockedAncestorTable.getKey().equals(pTableKey)) { + pTable = lockedAncestorTable; + } else { + // if we are currently combining columns for a view index and are looking up its + // ancestors we do not add the indexes to the ancestor PTable (or else we end up in + // a circular loop) + // we also don't need to add parent columns of the ancestors as we combine columns + // from all ancestors + pTable = + doGetTable(parentTableInfo.getTenantId(), parentTableInfo.getSchemaName(), + parentTableInfo.getTableName(), timestamp, null, clientVersion, hasIndexId, + true, null); + } + if (pTable == null) { + throw new ParentTableNotFoundException(parentTableInfo, fullTableName); + } else { + // only combine columns for view indexes (and not local indexes on regular tables + // which also have a viewIndexId) + if (i == 0 && hasIndexId && pTable.getType() != PTableType.VIEW) { + return table; + } + if (TABLE.equals(pTable.getType())) { + baseTable = pTable; + } + // set the final table timestamp as the max timestamp of the view/view index or its + // ancestors + maxTableTimestamp = Math.max(maxTableTimestamp, pTable.getTimeStamp()); + if (hasIndexId) { + // add all pk columns of parent tables to indexes + for (PColumn column : pTable.getPKColumns()) { + // don't add the salt column of ancestor tables for view indexes + if (column.equals(SaltingUtil.SALTING_COLUMN) || column.isExcluded()) { --- End diff -- Same comment as before - we should be able to match based on the column being the first column. Note that the salt column would only be in the base physical table. > Support multi region SYSTEM.CATALOG table > ----------------------------------------- > > Key: PHOENIX-3534 > URL: https://issues.apache.org/jira/browse/PHOENIX-3534 > Project: Phoenix > Issue Type: Bug > Reporter: James Taylor > Assignee: Thomas D'Silva > Priority: Major > Fix For: 5.0.0, 4.15.0 > > Attachments: PHOENIX-3534.patch > > > Currently Phoenix requires that the SYSTEM.CATALOG table is single region based on the server-side row locks being held for operations that impact a table and all of it's views. For example, adding/removing a column from a base table pushes this change to all views. > As an alternative to making the SYSTEM.CATALOG transactional (PHOENIX-2431), when a new table is created we can do a lazy cleanup of any rows that may be left over from a failed DDL call (kudos to [~lhofhansl] for coming up with this idea). To implement this efficiently, we'd need to also do PHOENIX-2051 so that we can efficiently find derived views. > The implementation would rely on an optimistic concurrency model based on checking our sequence numbers for each table/view before/after updating. Each table/view row would be individually locked for their change (metadata for a view or table cannot span regions due to our split policy), with the sequence number being incremented under lock and then returned to the client. -- This message was sent by Atlassian JIRA (v7.6.3#76005)