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

James Taylor commented on PHOENIX-1224:
---------------------------------------

Patch looks good. Do the tests pass with this change?
One minor nit: exit early here if you ever get null back from next:

{code}
+        for(int i = 0; i < extraSpan; i++) {
+            next(ptr, position + i + 1, maxOffset);
+        }
{code}

For RowKeySchema.previous(), you're going to do the reverse of your new next() 
implementation. Remember the initial offset and then set the length 
appropriately after calling previous() slotSpan[i] extra times.

The only call to reposition is in SkipScanFilter, so you should be able to just 
pass in the slotSpan array and pass through the new arguments for your new next 
and previous calls.

This bit here in RowKeySchema.reposition() is just an optimization to start at 
the beginning again versus calling previous N times. You can change this to 
call iterator with only the first two args, and then call your version of 
next() here maxOffset-minOffset times.

{code}
            if (nVarLengthBetween > nVarLengthFromBeginning) {
                return iterator(ptr.get(), minOffset, maxOffset-minOffset, ptr, 
newPosition+1);
            }
            do  {
                hasValue = previous(ptr, --oldPosition, minOffset);
            } while (hasValue != null && oldPosition > newPosition);
{code}


> Dead loop in hbase scan when hint SKIP_SCAN is set and there is partial key 
> match in RowValueConstructor
> --------------------------------------------------------------------------------------------------------
>
>                 Key: PHOENIX-1224
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-1224
>             Project: Phoenix
>          Issue Type: Bug
>    Affects Versions: 3.0.0, 4.0.0, 5.0.0
>            Reporter: Maryann Xue
>            Assignee: Kyle Buzsaki
>         Attachments: PHOENIX-1224.patch
>
>   Original Estimate: 48h
>  Remaining Estimate: 48h
>
> The below test will end up in dead loop in hbase scan.
> {code}
>     @Test
>     public void testForceSkipScan() throws Exception {
>         String tempTableWithCompositePK = "TEMP_TABLE_COMPOSITE_PK";
>         Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
>         Connection conn = DriverManager.getConnection(getUrl(), props);
>         try {
>             conn.createStatement().execute("CREATE TABLE " + 
> tempTableWithCompositePK 
>                     + "   (col0 INTEGER NOT NULL, " 
>                     + "    col1 INTEGER NOT NULL, " 
>                     + "    col2 INTEGER NOT NULL, "
>                     + "    col3 INTEGER "
>                     + "   CONSTRAINT pk PRIMARY KEY (col0, col1, col2)) " 
>                     + "   SALT_BUCKETS=4");
>             
>             PreparedStatement upsertStmt = conn.prepareStatement(
>                     "upsert into " + tempTableWithCompositePK + "(col0, col1, 
> col2, col3) " + "values (?, ?, ?, ?)");
>             for (int i = 0; i < 3; i++) {
>                 upsertStmt.setInt(1, i + 1);
>                 upsertStmt.setInt(2, i + 2);
>                 upsertStmt.setInt(3, i + 3);
>                 upsertStmt.setInt(4, i + 5);
>                 upsertStmt.execute();
>             }
>             conn.commit();
>             
>             String query = "SELECT /*+ SKIP_SCAN*/ * FROM " + 
> tempTableWithCompositePK + " WHERE (col0, col1) in ((2, 3), (3, 4), (4, 5))";
>             PreparedStatement statement = conn.prepareStatement(query);
>             ResultSet rs = statement.executeQuery();
>             assertTrue(rs.next());
>             assertEquals(rs.getInt(1), 2);
>             assertEquals(rs.getInt(2), 3);
>             assertEquals(rs.getInt(3), 4);
>             assertEquals(rs.getInt(4), 6);
>             assertTrue(rs.next());
>             assertEquals(rs.getInt(1), 3);
>             assertEquals(rs.getInt(2), 4);
>             assertEquals(rs.getInt(3), 5);
>             assertEquals(rs.getInt(4), 7);
>             assertFalse(rs.next());
>         } finally {
>             conn.close();
>         }
>     }
> {code}
> The dead-loop thread:
> {panel}
> "defaultRpcServer.handler=4,queue=0,port=58945" daemon prio=10 
> tid=0x00007fe4d408c000 nid=0x7bba runnable [0x00007fe4c10cf000]
>    java.lang.Thread.State: RUNNABLE
>         at java.util.ArrayList.size(ArrayList.java:177)
>         at java.util.AbstractList$Itr.hasNext(AbstractList.java:339)
>         at 
> org.apache.hadoop.hbase.filter.FilterList.filterAllRemaining(FilterList.java:199)
>         at 
> org.apache.hadoop.hbase.regionserver.ScanQueryMatcher.match(ScanQueryMatcher.java:263)
>         at 
> org.apache.hadoop.hbase.regionserver.StoreScanner.next(StoreScanner.java:469)
>         at 
> org.apache.hadoop.hbase.regionserver.KeyValueHeap.next(KeyValueHeap.java:140)
>         at 
> org.apache.hadoop.hbase.regionserver.HRegion$RegionScannerImpl.populateResult(HRegion.java:3937)
>         at 
> org.apache.hadoop.hbase.regionserver.HRegion$RegionScannerImpl.nextInternal(HRegion.java:4017)
>         at 
> org.apache.hadoop.hbase.regionserver.HRegion$RegionScannerImpl.nextRaw(HRegion.java:3885)
>         at 
> org.apache.hadoop.hbase.regionserver.HRegion$RegionScannerImpl.nextRaw(HRegion.java:3876)
>         at 
> org.apache.phoenix.coprocessor.ScanRegionObserver$2.nextRaw(ScanRegionObserver.java:366)
>         at 
> org.apache.phoenix.coprocessor.DelegateRegionScanner.nextRaw(DelegateRegionScanner.java:76)
>         at 
> org.apache.hadoop.hbase.regionserver.HRegionServer.scan(HRegionServer.java:3157)
>         - locked <0x0000000778d5dbd8> (a 
> org.apache.phoenix.coprocessor.BaseScannerRegionObserver$1)
>         at 
> org.apache.hadoop.hbase.protobuf.generated.ClientProtos$ClientService$2.callBlockingMethod(ClientProtos.java:29497)
>         at org.apache.hadoop.hbase.ipc.RpcServer.call(RpcServer.java:2027)
>         at org.apache.hadoop.hbase.ipc.CallRunner.run(CallRunner.java:98)
>         at 
> org.apache.hadoop.hbase.ipc.RpcExecutor.consumerLoop(RpcExecutor.java:114)
>         at org.apache.hadoop.hbase.ipc.RpcExecutor$1.run(RpcExecutor.java:94)
>         at java.lang.Thread.run(Thread.java:662)
> {panel}



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

Reply via email to