Repository: phoenix Updated Branches: refs/heads/master 3878f3cbf -> 16d495a68
PHOENIX-3109 Improve and fix the way we are caching column family names for local indexes in IndexMaintainer Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/16d495a6 Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/16d495a6 Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/16d495a6 Branch: refs/heads/master Commit: 16d495a689bab8a0c3bb2b0f06d29e9a4736f4d1 Parents: 3878f3c Author: Samarth <samarth.j...@salesforce.com> Authored: Fri Jul 22 13:45:29 2016 -0700 Committer: Samarth <samarth.j...@salesforce.com> Committed: Fri Jul 22 13:45:29 2016 -0700 ---------------------------------------------------------------------- .../coprocessor/MetaDataEndpointImpl.java | 2 +- .../index/covered/update/ColumnReference.java | 9 ++- .../index/util/ReadOnlyImmutableBytesPtr.java | 59 ------------------ .../apache/phoenix/index/IndexMaintainer.java | 64 +++++++++----------- .../apache/phoenix/schema/MetaDataClient.java | 2 +- .../phoenix/index/IndexMaintainerTest.java | 2 +- 6 files changed, 36 insertions(+), 102 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/16d495a6/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java index 8bea46b..7d3468d 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java @@ -3116,7 +3116,7 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso invalidateList.add(new ImmutableBytesPtr(indexKey)); } // If the dropped column is a covered index column, invalidate the index - else if (indexMaintainer.getCoverededColumns().contains( + else if (indexMaintainer.getCoveredColumns().contains( new ColumnReference(columnToDelete.getFamilyName().getBytes(), columnToDelete .getName().getBytes()))) { invalidateList.add(new ImmutableBytesPtr(indexKey)); http://git-wip-us.apache.org/repos/asf/phoenix/blob/16d495a6/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/covered/update/ColumnReference.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/covered/update/ColumnReference.java b/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/covered/update/ColumnReference.java index 8bd35f8..00348b3 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/covered/update/ColumnReference.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/covered/update/ColumnReference.java @@ -22,7 +22,6 @@ import org.apache.hadoop.hbase.KeyValue; import org.apache.hadoop.hbase.io.ImmutableBytesWritable; import org.apache.hadoop.hbase.util.Bytes; import org.apache.phoenix.hbase.index.util.ImmutableBytesPtr; -import org.apache.phoenix.hbase.index.util.ReadOnlyImmutableBytesPtr; /** * @@ -46,15 +45,15 @@ public class ColumnReference implements Comparable<ColumnReference> { private final ImmutableBytesPtr qualifierPtr; public ColumnReference(byte[] family, byte[] qualifier) { - this.familyPtr = new ReadOnlyImmutableBytesPtr(family); - this.qualifierPtr = new ReadOnlyImmutableBytesPtr(qualifier); + this.familyPtr = new ImmutableBytesPtr(family); + this.qualifierPtr = new ImmutableBytesPtr(qualifier); this.hashCode = calcHashCode(this.familyPtr, this.qualifierPtr); } public ColumnReference(byte[] family, int familyOffset, int familyLength, byte[] qualifier, int qualifierOffset, int qualifierLength) { - this.familyPtr = new ReadOnlyImmutableBytesPtr(family, familyOffset, familyLength); - this.qualifierPtr = new ReadOnlyImmutableBytesPtr(qualifier, qualifierOffset, qualifierLength); + this.familyPtr = new ImmutableBytesPtr(family, familyOffset, familyLength); + this.qualifierPtr = new ImmutableBytesPtr(qualifier, qualifierOffset, qualifierLength); this.hashCode = calcHashCode(this.familyPtr, this.qualifierPtr); } http://git-wip-us.apache.org/repos/asf/phoenix/blob/16d495a6/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/util/ReadOnlyImmutableBytesPtr.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/util/ReadOnlyImmutableBytesPtr.java b/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/util/ReadOnlyImmutableBytesPtr.java deleted file mode 100644 index 6a7334f..0000000 --- a/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/util/ReadOnlyImmutableBytesPtr.java +++ /dev/null @@ -1,59 +0,0 @@ -/* - * 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.hbase.index.util; - -import org.apache.hadoop.hbase.io.ImmutableBytesWritable; - -public class ReadOnlyImmutableBytesPtr extends ImmutableBytesPtr { - - private static final String ERROR_MESSAGE = "Read-only bytes pointer may not be changed"; - - public ReadOnlyImmutableBytesPtr() { - } - - public ReadOnlyImmutableBytesPtr(byte[] bytes) { - super(bytes); - } - - public ReadOnlyImmutableBytesPtr(ImmutableBytesWritable ibw) { - super(ibw.get(), ibw.getOffset(), ibw.getLength()); - } - - public ReadOnlyImmutableBytesPtr(ImmutableBytesPtr ibp) { - super(ibp.get(), ibp.getOffset(), ibp.getLength()); - } - - public ReadOnlyImmutableBytesPtr(byte[] bytes, int offset, int length) { - super(bytes, offset, length); - } - - @Override - public void set(byte[] b) { - throw new UnsupportedOperationException(ERROR_MESSAGE); - } - - @Override - public void set(ImmutableBytesWritable ptr) { - throw new UnsupportedOperationException(ERROR_MESSAGE); - } - - @Override - public void set(byte[] b, int offset, int length) { - throw new UnsupportedOperationException(ERROR_MESSAGE); - } -} http://git-wip-us.apache.org/repos/asf/phoenix/blob/16d495a6/phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java b/phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java index db823de..8cdbb98 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/index/IndexMaintainer.java @@ -71,6 +71,7 @@ import org.apache.phoenix.schema.PColumn; import org.apache.phoenix.schema.PColumnFamily; import org.apache.phoenix.schema.PDatum; import org.apache.phoenix.schema.PIndexState; +import org.apache.phoenix.schema.PName; import org.apache.phoenix.schema.PTable; import org.apache.phoenix.schema.PTable.IndexType; import org.apache.phoenix.schema.PTableType; @@ -274,7 +275,8 @@ public class IndexMaintainer implements Writable, Iterable<ColumnReference> { // columns required to evaluate all expressions in indexedExpressions (this does not include columns in the data row key) private Set<ColumnReference> indexedColumns; private Set<ColumnReference> coveredColumns; - private Map<ColumnReference, ColumnReference> coveredColumnsMap; + // Map used to cache column family of data table and the corresponding column family for the local index + private Map<ImmutableBytesPtr, ImmutableBytesWritable> dataTableLocalIndexFamilyMap; // columns required to create index row i.e. indexedColumns + coveredColumns (this does not include columns in the data row key) private Set<ColumnReference> allColumns; // TODO remove this in the next major release @@ -364,7 +366,7 @@ public class IndexMaintainer implements Writable, Iterable<ColumnReference> { this.indexedColumnTypes = Lists.<PDataType>newArrayListWithExpectedSize(nIndexPKColumns-nDataPKColumns); this.indexedExpressions = Lists.newArrayListWithExpectedSize(nIndexPKColumns-nDataPKColumns); this.coveredColumns = Sets.newLinkedHashSetWithExpectedSize(nIndexColumns-nIndexPKColumns); - this.coveredColumnsMap = Maps.newHashMapWithExpectedSize(nIndexColumns-nIndexPKColumns); + this.dataTableLocalIndexFamilyMap = Maps.newHashMapWithExpectedSize(nIndexColumns-nIndexPKColumns); this.nIndexSaltBuckets = nIndexSaltBuckets == null ? 0 : nIndexSaltBuckets; this.dataEmptyKeyValueCF = SchemaUtil.getEmptyColumnFamily(dataTable); this.emptyKeyValueCFPtr = SchemaUtil.getEmptyColumnFamilyPtr(index); @@ -431,14 +433,10 @@ public class IndexMaintainer implements Writable, Iterable<ColumnReference> { PColumnFamily family = index.getColumnFamilies().get(i); for (PColumn indexColumn : family.getColumns()) { PColumn column = IndexUtil.getDataColumn(dataTable, indexColumn.getName().getString()); - this.coveredColumns.add(new ColumnReference(column.getFamilyName().getBytes(), column.getName().getBytes())); + PName dataTableFamily = column.getFamilyName(); + this.coveredColumns.add(new ColumnReference(dataTableFamily.getBytes(), column.getName().getBytes())); if(isLocalIndex) { - this.coveredColumnsMap.put( - new ColumnReference(column.getFamilyName().getBytes(), column.getName() - .getBytes()), - new ColumnReference(isLocalIndex ? Bytes.toBytes(IndexUtil - .getLocalIndexColumnFamily(column.getFamilyName().getString())) - : column.getFamilyName().getBytes(), column.getName().getBytes())); + this.dataTableLocalIndexFamilyMap.put(new ImmutableBytesPtr(dataTableFamily.getBytes()), new ImmutableBytesPtr(Bytes.toBytes(IndexUtil.getLocalIndexColumnFamily(dataTableFamily.getString())))); } } } @@ -762,7 +760,6 @@ public class IndexMaintainer implements Writable, Iterable<ColumnReference> { // same for all rows in this index) if (!viewConstantColumnBitSet.get(i)) { int pos = rowKeyMetaData.getIndexPkPosition(i-dataPosOffset); - Field dataField = dataRowKeySchema.getField(i); indexFields[pos] = dataRowKeySchema.getField(i); } @@ -862,7 +859,7 @@ public class IndexMaintainer implements Writable, Iterable<ColumnReference> { put.setDurability(!indexWALDisabled ? Durability.USE_DEFAULT : Durability.SKIP_WAL); } int i = 0; - for (ColumnReference ref : this.getCoverededColumns()) { + for (ColumnReference ref : this.getCoveredColumns()) { ImmutableBytesPtr cq = this.indexQualifiers.get(i++); ImmutableBytesWritable value = valueGetter.getLatestValue(ref); byte[] indexRowKey = this.buildRowKey(valueGetter, dataRowKeyPtr, regionStartKey, regionEndKey); @@ -874,8 +871,8 @@ public class IndexMaintainer implements Writable, Iterable<ColumnReference> { } //this is a little bit of extra work for installations that are running <0.94.14, but that should be rare and is a short-term set of wrappers - it shouldn't kill GC if(this.isLocalIndex) { - ColumnReference columnReference = this.coveredColumnsMap.get(ref); - put.add(kvBuilder.buildPut(rowKey, columnReference.getFamilyWritable(), cq, ts, value)); + ImmutableBytesWritable localIndexColFamily = this.dataTableLocalIndexFamilyMap.get(ref.getFamilyWritable()); + put.add(kvBuilder.buildPut(rowKey, localIndexColFamily, cq, ts, value)); } else { put.add(kvBuilder.buildPut(rowKey, ref.getFamilyWritable(), cq, ts, value)); } @@ -963,22 +960,22 @@ public class IndexMaintainer implements Writable, Iterable<ColumnReference> { if (oldState == null || (deleteType=getDeleteTypeOrNull(pendingUpdates)) != null || hasIndexedColumnChanged(oldState, pendingUpdates)) { // Deleting the entire row byte[] emptyCF = emptyKeyValueCFPtr.copyBytesIfNecessary(); Delete delete = new Delete(indexRowKey); - // If table delete was single version, then index delete should be as well - if (deleteType == DeleteType.SINGLE_VERSION) { - for (ColumnReference ref : getCoverededColumns()) { // FIXME: Keep Set<byte[]> for index CFs? - if(this.isLocalIndex) { - ref = this.coveredColumnsMap.get(ref); - } - delete.deleteFamilyVersion(ref.getFamily(), ts); + + for (ColumnReference ref : getCoveredColumns()) { + byte[] family = ref.getFamily(); + if (this.isLocalIndex) { + family = this.dataTableLocalIndexFamilyMap.get(ref.getFamilyWritable()).get(); + } + // If table delete was single version, then index delete should be as well + if (deleteType == DeleteType.SINGLE_VERSION) { + delete.deleteFamilyVersion(family, ts); + } else { + delete.deleteFamily(family, ts); } + } + if (deleteType == DeleteType.SINGLE_VERSION) { delete.deleteFamilyVersion(emptyCF, ts); } else { - for (ColumnReference ref : getCoverededColumns()) { // FIXME: Keep Set<byte[]> for index CFs? - if(this.isLocalIndex) { - ref = this.coveredColumnsMap.get(ref); - } - delete.deleteFamily(ref.getFamily(), ts); - } delete.deleteFamily(emptyCF, ts); } delete.setDurability(!indexWALDisabled ? Durability.USE_DEFAULT : Durability.SKIP_WAL); @@ -994,15 +991,12 @@ public class IndexMaintainer implements Writable, Iterable<ColumnReference> { delete = new Delete(indexRowKey); delete.setDurability(!indexWALDisabled ? Durability.USE_DEFAULT : Durability.SKIP_WAL); } - ColumnReference columnReference = ref; - if(this.isLocalIndex) { - columnReference = this.coveredColumnsMap.get(ref); - } + byte[] family = this.isLocalIndex ? this.dataTableLocalIndexFamilyMap.get(ref.getFamilyWritable()).get() : ref.getFamily(); // If point delete for data table, then use point delete for index as well if (kv.getTypeByte() == KeyValue.Type.Delete.getCode()) { - delete.deleteColumn(columnReference.getFamily(), IndexUtil.getIndexColumnName(ref.getFamily(), ref.getQualifier()), ts); + delete.deleteColumn(family, IndexUtil.getIndexColumnName(ref.getFamily(), ref.getQualifier()), ts); } else { - delete.deleteColumns(columnReference.getFamily(), IndexUtil.getIndexColumnName(ref.getFamily(), ref.getQualifier()), ts); + delete.deleteColumns(family, IndexUtil.getIndexColumnName(ref.getFamily(), ref.getQualifier()), ts); } } } @@ -1014,7 +1008,7 @@ public class IndexMaintainer implements Writable, Iterable<ColumnReference> { return indexTableName; } - public Set<ColumnReference> getCoverededColumns() { + public Set<ColumnReference> getCoveredColumns() { return coveredColumns; } @@ -1057,14 +1051,14 @@ public class IndexMaintainer implements Writable, Iterable<ColumnReference> { isLocalIndex = encodedCoveredolumnsAndLocalIndex < 0; int nCoveredColumns = Math.abs(encodedCoveredolumnsAndLocalIndex) - 1; coveredColumns = Sets.newLinkedHashSetWithExpectedSize(nCoveredColumns); - coveredColumnsMap = Maps.newHashMapWithExpectedSize(nCoveredColumns); + dataTableLocalIndexFamilyMap = Maps.newHashMapWithExpectedSize(nCoveredColumns); for (int i = 0; i < nCoveredColumns; i++) { byte[] cf = Bytes.readByteArray(input); byte[] cq = Bytes.readByteArray(input); ColumnReference ref = new ColumnReference(cf,cq); coveredColumns.add(ref); if(isLocalIndex) { - coveredColumnsMap.put(ref, new ColumnReference(Bytes.toBytes(IndexUtil.getLocalIndexColumnFamily(Bytes.toString(cf))), cq)); + dataTableLocalIndexFamilyMap.put(ref.getFamilyWritable(), new ImmutableBytesPtr(Bytes.toBytes(IndexUtil.getLocalIndexColumnFamily(Bytes.toString(cf))))); } } // Hack to serialize whether the index row key is optimizable http://git-wip-us.apache.org/repos/asf/phoenix/blob/16d495a6/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java index 7d2de29..d0e749f 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java @@ -3304,7 +3304,7 @@ public class MetaDataClient { // get the columns required for the index pk Set<ColumnReference> indexColumns = indexMaintainer.getIndexedColumns(); // get the covered columns - Set<ColumnReference> coveredColumns = indexMaintainer.getCoverededColumns(); + Set<ColumnReference> coveredColumns = indexMaintainer.getCoveredColumns(); List<PColumn> indexColumnsToDrop = Lists.newArrayListWithExpectedSize(columnRefs.size()); for(PColumn columnToDrop : tableColumnsToDrop) { ColumnReference columnToDropRef = new ColumnReference(columnToDrop.getFamilyName().getBytes(), columnToDrop.getName().getBytes()); http://git-wip-us.apache.org/repos/asf/phoenix/blob/16d495a6/phoenix-core/src/test/java/org/apache/phoenix/index/IndexMaintainerTest.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/test/java/org/apache/phoenix/index/IndexMaintainerTest.java b/phoenix-core/src/test/java/org/apache/phoenix/index/IndexMaintainerTest.java index e59a407..e2cf27d 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/index/IndexMaintainerTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/index/IndexMaintainerTest.java @@ -144,7 +144,7 @@ public class IndexMaintainerTest extends BaseConnectionlessQueryTest { byte[] mutablelndexRowKey = im1.buildRowKey(valueGetter, ptr, null, null); byte[] immutableIndexRowKey = indexKeyPtr.copyBytes(); assertArrayEquals(immutableIndexRowKey, mutablelndexRowKey); - for (ColumnReference ref : im1.getCoverededColumns()) { + for (ColumnReference ref : im1.getCoveredColumns()) { valueMap.get(ref); } byte[] dataRowKey = im1.buildDataRowKey(indexKeyPtr, null);