[jira] [Commented] (PHOENIX-1734) Local index improvements

["James Taylor (JIRA)" <ji...@apache.org>]

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

James Taylor commented on PHOENIX-1734:
---------------------------------------

Thanks for the patch, [~rajeshbabu]. It's coming along nicely. Here's some feedback:
* The way we're handling NotServingRegionException needs to be improved. Maybe you can walk us through that, but I think there are several issues:
** We can't assume the structure (i.e. parent/child iterator) when we detect this situation as we're doing here and in ScanUtil.getNewIterator. That's basically hard coding the query plan. We also wouldn't want to have to insert this logic in every ResultIterator implementation - it needs to be centralized IMO.
{code}
     @Override
     public Tuple next() throws SQLException {
+        Tuple next = null;
+            try {
+                next = nextInternal();
+            } catch(Exception e) {
+                try { // Rethrow as SQLException
+                    throw ServerUtil.parseServerException(e);
+                } catch (StaleRegionBoundaryCacheException e2) {
+                    if(scan != null && ScanUtil.isLocalIndex(scan)) {
+                        TableResultIterator tableResultIterator = ((TableResultIterator)getIterator());
+                        tableResultIterator.close();
+                        setIterator(ScanUtil.getNewIterator(tableResultIterator, this.scan,
+                            this.next == UNINITIALIZED ? null : this.next));
+                        this.next = UNINITIALIZED;
+                        next = nextInternal();
+                    } else {
+                        throw e;
+                    }
+                }
+            }
+        return next;
+    }
{code}
** What would happen if this exception occurred during an aggregation? We have partial state already. Is that factored in correctly?
** Perhaps a more feasible approach would be to detect this up front when the scan starts by sending the expected end region boundary? That way the existing mechanism would handle it (isolated to BaseParallelIterators).
* I don't see any upgrade code in ConnectionQueryServicesImpl to handle existing tables with local indexes. We'll need to handle that smoothly.
* The query plan is not clear at all that a local indexes is being used: 
{code}
WAY RANGE SCAN OVER my_table [-32768, 'foo']
{code}
We should change it to make that clear, as well as offsetting the index id by adding {{-Short.MAX_VALUE}+1}, which I realize isn't related to your change, but it would improve the readability and since you're changing all the query plans, might as well do that now too. Something like this:
{code}
WAY RANGE SCAN OVER LOCAL INDEX ON my_table [1, 'foo']
{code}

Given that we're ready for a 4.7.0 release now, I think it makes sense to push this to a 4.8.0 release. We could follow up with that as soon as this is ready. Probably a good idea not to stuff too much into a single release and that'll give you a little more time to do the above.

WDYT, [~rajeshbabu]?

> Local index improvements
> ------------------------
>
>                 Key: PHOENIX-1734
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-1734
>             Project: Phoenix
>          Issue Type: Improvement
>            Reporter: Rajeshbabu Chintaguntla
>            Assignee: Rajeshbabu Chintaguntla
>             Fix For: 4.7.0
>
>         Attachments: PHOENI-1734-WIP.patch, PHOENIX-1734_v1.patch, PHOENIX-1734_v4.patch, PHOENIX-1734_v5.patch, TestAtomicLocalIndex.java
>
>
> Local index design considerations: 
>  1. Colocation: We need to co-locate regions of local index regions and data regions. The co-location can be a hard guarantee or a soft (best approach) guarantee. The co-location is a performance requirement, and also maybe needed for consistency(2). Hard co-location means that either both the data region and index region are opened atomically, or neither of them open for serving. 
>  2. Index consistency : Ideally we want the index region and data region to have atomic updates. This means that they should either (a)use transactions, or they should (b)share the same WALEdit and also MVCC for visibility. (b) is only applicable if there is hard colocation guarantee. 
>  3. Local index clients : How the local index will be accessed from clients. In case of the local index being managed in a table, the HBase client can be used for doing scans, etc. If the local index is hidden inside the data regions, there has to be a different mechanism to access the data through the data region. 
> With the above considerations, we imagine three possible implementation for the local index solution, each detailed below. 
> APPROACH 1:  Current approach
> (1) Current approach uses balancer as a soft guarantee. Because of this, in some rare cases, colocation might not happen. 
> (2) The index and data regions do not share the same WALEdits. Meaning consistency cannot be achieved. Also there are two WAL writes per write from client. 
> (3) Regular Hbase client can be used to access index data since index is just another table. 
> APPROACH 2: Shadow regions + shared WAL & MVCC 
> (1) Introduce a shadow regions concept in HBase. Shadow regions are not assigned by AM. Phoenix implements atomic open (and split/merge) of region opening for data regions and index regions so that hard co-location is guaranteed. 
> (2) For consistency requirements, the index regions and data regions will share the same WALEdit (and thus recovery) and they will also share the same MVCC mechanics so that index update and data update is visible atomically. 
> (3) Regular Hbase client can be used to access index data since index is just another table.  
> APPROACH 3: Storing index data in separate column families in the table.
>  (1) Regions will have store files for cfs, which is sorted using the primary sort order. Regions may also maintain stores, sorted in secondary sort orders. This approach is similar in vein how a RDBMS keeps data (a B-TREE in primary sort order and multiple B-TREEs in secondary sort orders with pointers to primary key). That means store the index data in separate column families in the data region. This way a region is extended to be more similar to a RDBMS (but LSM instead of BTree). This is sometimes called shadow cf’s as well. This approach guarantees hard co-location.
>  (2) Since everything is in a single region, they automatically share the same WALEdit and MVCC numbers. Atomicity is easily achieved. 
>  (3) Current Phoenix implementation need to change in such a way that column families selection in read/write path is based data table/index table(logical table in phoenix). 
> I think that APPROACH 3 is the best one for long term, since it does not require to change anything in HBase, mainly we don't need to muck around with the split/merge stuff in HBase. It will be win-win.
> However, APPROACH 2 still needs a “shadow regions” concept to be implemented in HBase itself, and also a way to share WALEdits and MVCCs from multiple regions.
> APPROACH 1 is a good start for local indexes, but I think we are not getting the full benefits for the feature. We can support this for the short term, and decide on the next steps for a longer term implementation. 
> we won't be able to get to implementing it immediately, and want to start a brainstorm.



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