PHOENIX-5795 Supporting selective queries for index rows updated conc… #740
Conversation
| Thread t = new Thread(runnables[i]); | ||
| t.start(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Shouldn't you wait for the threads to finish running?
There was a problem hiding this comment.
It is there in the next line
| for (Cell cell : cellList) { | ||
| if (Bytes.compareTo(CellUtil.cloneFamily(cell), family) == 0 && | ||
| Bytes.compareTo(CellUtil.cloneQualifier(cell), columnReference.getQualifier() ) == 0) { | ||
| has = true; |
There was a problem hiding this comment.
Suggestion: Change cell names and family to indicate whether they belong to index or data table.
I am not sure I understand this loop. If we have index and and data cell having same family and qualifier we just break? Should not it be if they are not equal break?
There was a problem hiding this comment.
Please see line 177. The multiMutationMap attribute holds the data table pending rows. Also indexMaintainer.getIndexedColumns() returns the data column references for indexed columns. The data columns are grouped into three classes, pk columns (data table pk columns), the indexed columns (the columns for which we want to have indexing; they form the prefix for the primary key for the index table (after salt and tenant id)) and covered columns. The purpose of the loop is to find out if all the indexed columns are included in the pending data table mutation pointed by multiMutation. I will add these comments to the code.
| if (copyMutations) { | ||
| // Add the mutation to the batch set | ||
| ImmutableBytesPtr row = new ImmutableBytesPtr(m.getRow()); | ||
| MultiMutation stored = mutationsMap.get(row); |
There was a problem hiding this comment.
Please note I removed these lines for setting the timestamp on cells as they were redundant. It is already done before calling this method. Also note that this method is not called for the rebuild path. This another reason for restructuring. I think I also made the code for a bit more readable.
| // Truncate, rebuild and verify the index table | ||
| PTable pIndexTable = PhoenixRuntime.getTable(conn, indexName); | ||
| TableName physicalTableName = | ||
| org.apache.hadoop.hbase.TableName.valueOf(pIndexTable.getPhysicalName().getBytes()); |
There was a problem hiding this comment.
nit: don't have to fully qualify TableName since it's imported
| } | ||
| boolean has = false; | ||
| for (Cell cell : cellList) { | ||
| if (Bytes.compareTo(CellUtil.cloneFamily(cell), family) == 0 && |
There was a problem hiding this comment.
Can be replaced with CellUtil.matchingColumn(Cell, family, qualifier)
There was a problem hiding this comment.
I will use it. Thx
| context.rowLocks.clear(); | ||
| throw new IOException("One of the concurrent mutations does not have all indexed columns. " + | ||
| "The batch needs to be retried " + | ||
| c.getEnvironment().getRegion().getRegionInfo().getTable().getNameAsString()); |
There was a problem hiding this comment.
Better to pass the TableName in rather than the whole ObserverContext, which has a lot of important dependencies that don't relate to this method.
There was a problem hiding this comment.
+1, thanks @kadirozde . Looks like you need to rebase though.
…urrently