virajjasani commented on code in PR #1751:
URL: https://github.com/apache/phoenix/pull/1751#discussion_r1509896530
##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/BaseScannerRegionObserver.java:
##########
@@ -527,6 +535,28 @@ public static boolean isMaxLookbackTimeEnabled(long
maxLookbackTime){
return maxLookbackTime > 0L;
}
+ public static long
getMaxLookbackAge(ObserverContext<RegionCoprocessorEnvironment> c) {
Review Comment:
nit: `private`
##########
phoenix-core-client/src/main/java/org/apache/phoenix/jdbc/PhoenixDatabaseMetaData.java:
##########
@@ -432,6 +432,8 @@ public class PhoenixDatabaseMetaData implements
DatabaseMetaData {
public static final String SYSTEM_TRANSFORM_TABLE = "TRANSFORM";
public static final String SYSTEM_TRANSFORM_NAME =
SchemaUtil.getTableName(SYSTEM_CATALOG_SCHEMA, SYSTEM_TRANSFORM_TABLE);
+ public static final String MAX_LOOKBACK_AGE = "MAX_LOOKBACK_AGE";
Review Comment:
`public static final String MAX_LOOKBACK_AGE =
BaseScannerRegionObserverConstants.MAX_LOOKBACK_AGE`
##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java:
##########
@@ -1587,6 +1623,56 @@ private PTable getTableFromCells(List<Cell>
tableCellList, List<List<Cell>> allC
return builder.build();
}
+ private Long scanMaxLookbackAgeFromParent(byte[] key, long
clientTimeStamp) throws IOException {
+ Scan scan = MetaDataUtil.newTableRowsScan(key, MIN_TABLE_TIMESTAMP,
clientTimeStamp);
+ Table sysCat = ServerUtil.getHTableForCoprocessorScan(this.env,
+ SchemaUtil.getPhysicalTableName(SYSTEM_CATALOG_NAME_BYTES,
env.getConfiguration()));
Review Comment:
We need to close the Table instance, let's wrap in try-with-resources
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java:
##########
@@ -27,16 +27,15 @@
import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.TABLE_SEQ_NUM;
import static org.apache.phoenix.query.QueryConstants.DEFAULT_COLUMN_FAMILY;
import static
org.apache.phoenix.query.QueryConstants.ENCODED_CQ_COUNTER_INITIAL_VALUE;
-import static org.apache.phoenix.util.TestUtil.TEST_PROPERTIES;
-import static org.apache.phoenix.util.TestUtil.closeConnection;
-import static org.apache.phoenix.util.TestUtil.closeStatement;
Review Comment:
you can adjust IDE settings to avoid the reformat from doing such changes
##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/BaseScannerRegionObserver.java:
##########
@@ -527,6 +535,28 @@ public static boolean isMaxLookbackTimeEnabled(long
maxLookbackTime){
return maxLookbackTime > 0L;
}
+ public static long
getMaxLookbackAge(ObserverContext<RegionCoprocessorEnvironment> c) {
+ TableName tableName =
c.getEnvironment().getRegion().getRegionInfo().getTable();
+ String fullTableName = tableName.getNameAsString();
+ Configuration conf = c.getEnvironment().getConfiguration();
+ PTable table;
+ try(PhoenixConnection conn = QueryUtil.getConnectionOnServer(
+ conf).unwrap(PhoenixConnection.class)) {
+ table = conn.getTableNoCache(fullTableName);
+ }
+ catch (Exception e) {
Review Comment:
Let's keep it `SQLException`
##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java:
##########
@@ -1587,6 +1623,56 @@ private PTable getTableFromCells(List<Cell>
tableCellList, List<List<Cell>> allC
return builder.build();
}
+ private Long scanMaxLookbackAgeFromParent(byte[] key, long
clientTimeStamp) throws IOException {
+ Scan scan = MetaDataUtil.newTableRowsScan(key, MIN_TABLE_TIMESTAMP,
clientTimeStamp);
+ Table sysCat = ServerUtil.getHTableForCoprocessorScan(this.env,
+ SchemaUtil.getPhysicalTableName(SYSTEM_CATALOG_NAME_BYTES,
env.getConfiguration()));
+ ResultScanner scanner = sysCat.getScanner(scan);
Review Comment:
Same here, ResultScanner needs to be closed, try-with-resources will be
helpful
##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java:
##########
@@ -1463,6 +1485,7 @@ private PTable getTableFromCells(List<Cell>
tableCellList, List<List<Cell>> allC
PTable transformingNewTable = null;
boolean isRegularView = (tableType == PTableType.VIEW && viewType !=
ViewType.MAPPED);
+ boolean isThisAViewIndex = false;
Review Comment:
nit: `hasViewIndex`
##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java:
##########
@@ -1587,6 +1623,56 @@ private PTable getTableFromCells(List<Cell>
tableCellList, List<List<Cell>> allC
return builder.build();
}
+ private Long scanMaxLookbackAgeFromParent(byte[] key, long
clientTimeStamp) throws IOException {
+ Scan scan = MetaDataUtil.newTableRowsScan(key, MIN_TABLE_TIMESTAMP,
clientTimeStamp);
+ Table sysCat = ServerUtil.getHTableForCoprocessorScan(this.env,
+ SchemaUtil.getPhysicalTableName(SYSTEM_CATALOG_NAME_BYTES,
env.getConfiguration()));
+ ResultScanner scanner = sysCat.getScanner(scan);
+ Result result = scanner.next();
+ boolean startCheckingForLink = false;
+ byte[] parentTableKey = null;
+ do {
+ if (result == null) {
+ return null;
+ }
+ else if (startCheckingForLink) {
+ byte[] linkTypeBytes = result.getValue(TABLE_FAMILY_BYTES,
LINK_TYPE_BYTES);
+ if (linkTypeBytes != null) {
+ LinkType linkType =
LinkType.fromSerializedValue(linkTypeBytes[0]);
+ int rowKeyColMetadataLength = 5;
+ byte[][] rowKeyMetaData = new
byte[rowKeyColMetadataLength][];
+ getVarChars(result.getRow(), rowKeyColMetadataLength,
rowKeyMetaData);
+ if (linkType == VIEW_INDEX_PARENT_TABLE) {
+ parentTableKey =
getParentTableKeyFromChildRowKeyMetaData(rowKeyMetaData);
+ return scanMaxLookbackAgeFromParent(parentTableKey,
clientTimeStamp);
+ }
+ else if (linkType == PHYSICAL_TABLE) {
Review Comment:
Does this cover "index table to data table" link also? Or does it only
covers "view index to view hierarchy's base table"?
I believe you are covering "index table to data table" case here right?
```
if (tableType == INDEX && ! isThisAViewIndex) {
byte[] tableKey = SchemaUtil.getTableKey(tenantId == null ? null
: tenantId.getBytes(),
parentSchemaName == null ? null :
parentSchemaName.getBytes(), parentTableName.getBytes());
maxLookbackAge = scanMaxLookbackAgeFromParent(tableKey,
clientTimeStamp);
}
```
##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/BaseScannerRegionObserver.java:
##########
@@ -527,6 +535,28 @@ public static boolean isMaxLookbackTimeEnabled(long
maxLookbackTime){
return maxLookbackTime > 0L;
}
+ public static long
getMaxLookbackAge(ObserverContext<RegionCoprocessorEnvironment> c) {
+ TableName tableName =
c.getEnvironment().getRegion().getRegionInfo().getTable();
+ String fullTableName = tableName.getNameAsString();
+ Configuration conf = c.getEnvironment().getConfiguration();
+ PTable table;
+ try(PhoenixConnection conn = QueryUtil.getConnectionOnServer(
+ conf).unwrap(PhoenixConnection.class)) {
+ table = conn.getTableNoCache(fullTableName);
+ }
+ catch (Exception e) {
+ if (e instanceof TableNotFoundException) {
+ LOGGER.debug("Ignoring HBase table that is not a Phoenix
table: "
+ + fullTableName);
Review Comment:
let's use parameterized version:
```
LOGGER.debug("Ignoring HBase table that is not a Phoenix table: {}",
fullTableName);
```
same for error log also
##########
phoenix-core-client/src/main/java/org/apache/phoenix/schema/PTable.java:
##########
@@ -997,6 +997,8 @@ IndexMaintainer getIndexMaintainer(PTable dataTable,
PhoenixConnection connectio
* @throws SQLException
*/
Set<ColumnReference> getIndexWhereColumns(PhoenixConnection connection)
throws SQLException;
+
+ Long getMaxLookbackAge();
Review Comment:
nit: javadoc
##########
phoenix-core-client/src/main/java/org/apache/phoenix/util/MetaDataUtil.java:
##########
@@ -1182,4 +1184,10 @@ public static void
deleteFromStatsTable(PhoenixConnection connection,
connection.setAutoCommit(isAutoCommit);
}
}
+
+ public static long getMaxLookbackAge(Configuration conf, Long
maxLookbackAge) {
Review Comment:
nit: javadoc
##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/MetaDataRegionObserver.java:
##########
@@ -61,6 +61,7 @@
import org.apache.hadoop.hbase.security.User;
import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos;
import org.apache.hadoop.hbase.util.Pair;
+import org.apache.hadoop.hbase.util.Bytes;
Review Comment:
unused
##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java:
##########
@@ -1587,6 +1623,56 @@ private PTable getTableFromCells(List<Cell>
tableCellList, List<List<Cell>> allC
return builder.build();
}
+ private Long scanMaxLookbackAgeFromParent(byte[] key, long
clientTimeStamp) throws IOException {
+ Scan scan = MetaDataUtil.newTableRowsScan(key, MIN_TABLE_TIMESTAMP,
clientTimeStamp);
+ Table sysCat = ServerUtil.getHTableForCoprocessorScan(this.env,
+ SchemaUtil.getPhysicalTableName(SYSTEM_CATALOG_NAME_BYTES,
env.getConfiguration()));
Review Comment:
With both try-with-resources, it might look something like this:
```
private Long scanMaxLookbackAgeFromParent(byte[] key, long
clientTimeStamp) throws IOException {
Scan scan = MetaDataUtil.newTableRowsScan(key, MIN_TABLE_TIMESTAMP,
clientTimeStamp);
byte[] parentTableKey = null;
try (Table sysCat = ServerUtil.getHTableForCoprocessorScan(this.env,
SchemaUtil.getPhysicalTableName(SYSTEM_CATALOG_NAME_BYTES,
env.getConfiguration()))) {
try (ResultScanner scanner = sysCat.getScanner(scan)) {
Result result = scanner.next();
boolean startCheckingForLink = false;
do {
if (result == null) {
return null;
} else if (startCheckingForLink) {
byte[] linkTypeBytes =
result.getValue(TABLE_FAMILY_BYTES, LINK_TYPE_BYTES);
if (linkTypeBytes != null) {
LinkType linkType =
LinkType.fromSerializedValue(linkTypeBytes[0]);
int rowKeyColMetadataLength = 5;
byte[][] rowKeyMetaData = new
byte[rowKeyColMetadataLength][];
getVarChars(result.getRow(),
rowKeyColMetadataLength, rowKeyMetaData);
if (linkType == VIEW_INDEX_PARENT_TABLE) {
parentTableKey =
getParentTableKeyFromChildRowKeyMetaData(rowKeyMetaData);
return
scanMaxLookbackAgeFromParent(parentTableKey,
clientTimeStamp);
} else if (linkType == PHYSICAL_TABLE) {
parentTableKey =
getParentTableKeyFromChildRowKeyMetaData(rowKeyMetaData);
}
}
} else {
byte[] maxLookbackAgeInBytes =
result.getValue(TABLE_FAMILY_BYTES,
MAX_LOOKBACK_AGE_BYTES);
if (maxLookbackAgeInBytes != null) {
return PLong.INSTANCE.getCodec()
.decodeLong(maxLookbackAgeInBytes, 0,
SortOrder.getDefault());
}
}
result = scanner.next();
startCheckingForLink = true;
} while (result != null);
}
}
return parentTableKey == null ?
null : scanMaxLookbackAgeFromParent(parentTableKey,
clientTimeStamp);
}
```
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
