You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2020/12/15 18:43:11 UTC
[GitHub] [phoenix] kadirozde commented on a change in pull request #973: PHOENIX-6211 Paged scan filters
kadirozde commented on a change in pull request #973:
URL: https://github.com/apache/phoenix/pull/973#discussion_r543593516
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/GlobalIndexRegionScanner.java
##########
@@ -1343,6 +1346,36 @@ public int prepareIndexMutations(Put put, Delete del, Map<byte[], List<Mutation>
return indexMutations.size();
}
+ static boolean adjustScanFilter(Scan scan) {
+ // For rebuilds we use count (*) as query for regular tables which ends up setting the FKOF on scan
Review comment:
ok
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/BaseScannerRegionObserver.java
##########
@@ -241,6 +231,12 @@ public void preScannerOpen(org.apache.hadoop.hbase.coprocessor.ObserverContext<R
// last possible moment. You need to swap the start/stop and make the
// start exclusive and the stop inclusive.
ScanUtil.setupReverseScan(scan);
+ if (!(scan.getFilter() instanceof PagedFilter)) {
+ byte[] pageSizeMsBytes = scan.getAttribute(BaseScannerRegionObserver.SERVER_PAGE_SIZE_MS);
+ if (pageSizeMsBytes != null) {
+ scan.setFilter(new PagedFilter(scan.getFilter(), Bytes.toLong(pageSizeMsBytes)/2));
Review comment:
Good suggestion.
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/GlobalIndexRegionScanner.java
##########
@@ -1351,26 +1384,18 @@ protected RegionScanner getLocalScanner() throws IOException {
Scan incrScan = new Scan(scan);
incrScan.setTimeRange(minTimestamp, scan.getTimeRange().getMax());
incrScan.setRaw(true);
- incrScan.setMaxVersions();
+ incrScan.readAllVersions();
incrScan.getFamilyMap().clear();
incrScan.setCacheBlocks(false);
for (byte[] family : scan.getFamilyMap().keySet()) {
incrScan.addFamily(family);
}
- // For rebuilds we use count (*) as query for regular tables which ends up setting the FKOF on scan
- // This filter doesn't give us all columns and skips to the next row as soon as it finds 1 col
- // For rebuilds we need all columns and all versions
- if (scan.getFilter() instanceof FirstKeyOnlyFilter) {
- incrScan.setFilter(null);
- } else if (scan.getFilter() != null) {
- // Override the filter so that we get all versions
- incrScan.setFilter(new AllVersionsIndexRebuildFilter(scan.getFilter()));
- }
+ adjustScanFilter(incrScan);
if(nextStartKey != null) {
incrScan.setStartRow(nextStartKey);
}
List<KeyRange> keys = new ArrayList<>();
- try(RegionScanner scanner = region.getScanner(incrScan)) {
+ try(RegionScanner scanner = new PagedRegionScanner(region, region.getScanner(incrScan), incrScan)) {
Review comment:
ok
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/util/ScanUtil.java
##########
@@ -1223,21 +1232,66 @@ public static void getDummyResult(List<Cell> result) {
getDummyResult(EMPTY_BYTE_ARRAY, result);
}
+ public static Tuple getDummyTuple(byte[] rowKey) {
+ List<Cell> result = new ArrayList<Cell>(1);
+ getDummyResult(rowKey, result);
+ return new ResultTuple(Result.create(result));
+ }
+
+ public static Tuple getDummyTuple() {
+ List<Cell> result = new ArrayList<Cell>(1);
+ getDummyResult(result);
+ return new ResultTuple(Result.create(result));
+ }
+
+ public static Tuple getDummyTuple(Tuple tuple) {
+ ImmutableBytesWritable ptr = new ImmutableBytesWritable();
+ tuple.getKey(ptr);
+ return getDummyTuple(ptr.copyBytes());
+ }
+
+ public static boolean isDummy(Cell cell) {
+ return CellUtil.matchingColumn(cell, EMPTY_BYTE_ARRAY, EMPTY_BYTE_ARRAY);
+ }
+
public static boolean isDummy(Result result) {
- // Check if the result is a dummy result
if (result.rawCells().length != 1) {
return false;
}
Cell cell = result.rawCells()[0];
- return CellUtil.matchingColumn(cell, EMPTY_BYTE_ARRAY, EMPTY_BYTE_ARRAY);
+ return isDummy(cell);
}
public static boolean isDummy(List<Cell> result) {
- // Check if the result is a dummy result
if (result.size() != 1) {
return false;
}
Cell cell = result.get(0);
- return CellUtil.matchingColumn(cell, EMPTY_BYTE_ARRAY, EMPTY_BYTE_ARRAY);
+ return isDummy(cell);
+ }
+
+ public static boolean isDummy(Tuple tuple) {
+ if (tuple instanceof ResultTuple) {
+ isDummy(((ResultTuple) tuple).getResult());
+ }
+ return false;
+ }
+
+ public static PagedFilter getPhoenixPageFilter(Scan scan) {
+ Filter filter = scan.getFilter();
Review comment:
PagedFilter is added on the server side by coprocs after the client prepares the scan, in order to wrap the filter set by the client. It is not used by the client. The method name here should be renamed to getPhoenixPagedFilter().
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/PhoenixTTLRegionObserver.java
##########
@@ -41,35 +43,41 @@
import org.slf4j.LoggerFactory;
import java.io.IOException;
+import java.sql.SQLException;
import java.util.Iterator;
import java.util.List;
+import java.util.Optional;
-import static org.apache.phoenix.coprocessor.BaseScannerRegionObserver.EMPTY_COLUMN_FAMILY_NAME;
-import static org.apache.phoenix.coprocessor.BaseScannerRegionObserver.EMPTY_COLUMN_QUALIFIER_NAME;
+import static org.apache.phoenix.util.ScanUtil.getDummyResult;
+import static org.apache.phoenix.util.ScanUtil.getPageSizeMs;
+import static org.apache.phoenix.util.ScanUtil.isDummy;
/**
* Coprocessor that checks whether the row is expired based on the TTL spec.
*/
-public class PhoenixTTLRegionObserver extends BaseRegionObserver {
+public class PhoenixTTLRegionObserver extends BaseScannerRegionObserver implements RegionCoprocessor {
private static final Logger LOG = LoggerFactory.getLogger(PhoenixTTLRegionObserver.class);
private MetricsPhoenixTTLSource metricSource;
- @Override public void start(CoprocessorEnvironment e) throws IOException {
- super.start(e);
- metricSource = MetricsPhoenixCoprocessorSourceFactory.getInstance().getPhoenixTTLSource();
+ @Override
+ public Optional<RegionObserver> getRegionObserver() {
+ return Optional.of(this);
}
- @Override public void stop(CoprocessorEnvironment e) throws IOException {
- super.stop(e);
+ @Override
+ public void start(CoprocessorEnvironment e) throws IOException {
+ metricSource = MetricsPhoenixCoprocessorSourceFactory.getInstance().getPhoenixTTLSource();
Review comment:
We have changed the super from BaseRegionObserver to BaseScannerRegionObserver which does not have start() or something equivalent.
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/schema/tuple/ResultTuple.java
##########
@@ -26,6 +27,9 @@
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.phoenix.hbase.index.util.GenericKeyValueBuilder;
import org.apache.phoenix.util.PhoenixKeyValueUtil;
+import org.apache.phoenix.util.ScanUtil;
Review comment:
Ok
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org