[ 
https://issues.apache.org/jira/browse/PHOENIX-1249?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14139821#comment-14139821
 ] 

James Taylor commented on PHOENIX-1249:
---------------------------------------

Thanks for the patch, [~rajesh23]. Nice perf improvement! Is this on 0.98.6 
that you've measured this? Here's some feedback:
- How about just starting at numNonKVColumns instead of 0 in the loop here?
{code}
@@ -170,9 +171,18 @@ public class DeleteCompiler {
         if (!hasImmutableIndex(tableRef)) {
             return false;
         }
+        boolean isMultiTenant = tableRef.getTable().isMultiTenant();
         for (PTable index : tableRef.getTable().getIndexes()) {
-            for (PColumn column : index.getPKColumns()) {
-                if (!IndexUtil.isDataPKColumn(column)) {
+            List<PColumn> pkColumns = index.getPKColumns();
+            boolean isLocalIndex = index.getIndexType() == IndexType.LOCAL;
+            int nIndexSaltBuckets =
+                    index.getBucketNum() == null ? 0 : index.getBucketNum();
+            int numNonKVColumns =
+                    (isMultiTenant ? 1 : 0) + (!isLocalIndex && 
nIndexSaltBuckets > 0 ? 1 : 0)
+                            + (isLocalIndex ? 1 : 0);
+            for (int i = 0; i < pkColumns.size(); i++) {
+                if(i < numNonKVColumns) continue;
+                if (!IndexUtil.isDataPKColumn(pkColumns.get(i))) {
{code}
- Why use a different UUID property name here? We're going to start allowing a 
mix of mutable and immutable indexes on a table, declaring the index itself as 
immutable, so we'll need to differentiate on the server side based on the 
particular IndexMaintainer. Just add an IndexMaintainer.isLocalIndex() method 
and an isImmutable boolean too in IndexMaintainer instead and use that to drive 
your logic on the server side.
{code}
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/index/PhoenixIndexCodec.java 
b/phoenix-core/src/main/java/org/apache/phoenix/index/PhoenixIndexCodec.java
index f061b8f..8bae573 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/index/PhoenixIndexCodec.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/index/PhoenixIndexCodec.java
@@ -59,6 +59,7 @@ import com.google.common.collect.Lists;
 public class PhoenixIndexCodec extends BaseIndexCodec {
     public static final String INDEX_MD = "IdxMD";
     public static final String INDEX_UUID = "IdxUUID";
+    public static final String LOCAL_INDEX_UUID = "LclIdxUUID";
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/index/PhoenixIndexCodec.java 
b/phoenix-core/src/main/java/org/apache/phoenix/index/PhoenixIndexCodec.java
index f061b8f..8bae573 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/index/PhoenixIndexCodec.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/index/PhoenixIndexCodec.java
@@ -59,6 +59,7 @@ import com.google.common.collect.Lists;
 public class PhoenixIndexCodec extends BaseIndexCodec {
     public static final String INDEX_MD = "IdxMD";
     public static final String INDEX_UUID = "IdxUUID";
+    public static final String LOCAL_INDEX_UUID = "LclIdxUUID";
{code}
- Can we call this enabledGlobalIndexIterator instead?
{code}
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 68cdb26..3468722 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
@@ -135,6 +135,15 @@ public class IndexMaintainer implements Writable, 
Iterable<ColumnReference> {
         });
     }
     
+    public static Iterator<PTable> nonLocalIndexIterator(Iterator<PTable> 
indexes) {
{code}
- Can we not have a forceSerialize flag and instead base this on whether the 
dataTable has indexes for which you'd want to send index maintainers (i.e. 
check that the index is local or not)?
{code}
+    public static void serialize(PTable dataTable, ImmutableBytesWritable ptr,
+            List<PTable> indexes, boolean forceSerialize) {
         Iterator<PTable> indexesItr = 
nonDisabledIndexIterator(indexes.iterator());
-        if ((dataTable.isImmutableRows()) || !indexesItr.hasNext()) {
+        if ((!forceSerialize && dataTable.isImmutableRows()) || 
!indexesItr.hasNext()) {
{code}
- Minor nit: In IndexUtil, would you mind just moving your new 
generateIndexData method above the one that just calls it with null, null args 
so the diff is better? Otherwise it's hard to get a sense for what changed.





> Support local immutable index 
> ------------------------------
>
>                 Key: PHOENIX-1249
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-1249
>             Project: Phoenix
>          Issue Type: Sub-task
>    Affects Versions: 4.1
>         Environment: Hbase 0.98.4-Hadoop2
> Phoenix 4.1
>            Reporter: Sun Fulin
>            Assignee: rajeshbabu
>             Fix For: 5.0.0, 4.2
>
>         Attachments: PHOENIX-1249.patch, PHOENIX-1249_v2.patch
>
>
> Currently local indexing are forced created as default mutable index which 
> requires index maintenance and server side processing, while immutable 
> indexes are appropriate for write-once/append-only use case that may fit for 
> many use cases. Hope some work for capability to create local index as 
> immutable index.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to