sanjeet006py commented on code in PR #1896:
URL: https://github.com/apache/phoenix/pull/1896#discussion_r1615843022
##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java:
##########
@@ -581,92 +601,135 @@ private boolean areMutationsInSameTable(Table
targetHTable, Region region) {
region.getTableDescriptor().getTableName().getName()) == 0);
}
+ @Override
+ public InternalScanner
preFlush(ObserverContext<RegionCoprocessorEnvironment> c, Store store,
+ InternalScanner scanner, FlushLifeCycleTracker tracker) throws
IOException {
+ if (!isPhoenixTableTTLEnabled(c.getEnvironment().getConfiguration())) {
+ return scanner;
+ } else {
+ return User.runAsLoginUser(new
PrivilegedExceptionAction<InternalScanner>() {
+ @Override public InternalScanner run() throws Exception {
+ String tableName =
c.getEnvironment().getRegion().getRegionInfo().getTable()
+ .getNameAsString();
+ long maxLookbackInMillis =
+
UngroupedAggregateRegionObserver.getMaxLookbackInMillis(
Review Comment:
This guarantees eventual consistency and the static map is filled up by
pre-compact hook. So that means once DDL to change max lookback for a table is
run, new value will start getting used on region server (updated in static map)
when pre-compact hook on that table is called. So, I have following doubts:
- Is there a set frequency within which pre-compact will get called at least
once for each table on each region server hosting regions of that table?
- For short duration within the max lookback map just after changing max
lookback, value of max lookback for table and index/view and view index might
not match, is that fine?
##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java:
##########
@@ -581,92 +601,135 @@ private boolean areMutationsInSameTable(Table
targetHTable, Region region) {
region.getTableDescriptor().getTableName().getName()) == 0);
}
+ @Override
+ public InternalScanner
preFlush(ObserverContext<RegionCoprocessorEnvironment> c, Store store,
+ InternalScanner scanner, FlushLifeCycleTracker tracker) throws
IOException {
+ if (!isPhoenixTableTTLEnabled(c.getEnvironment().getConfiguration())) {
+ return scanner;
+ } else {
+ return User.runAsLoginUser(new
PrivilegedExceptionAction<InternalScanner>() {
+ @Override public InternalScanner run() throws Exception {
+ String tableName =
c.getEnvironment().getRegion().getRegionInfo().getTable()
+ .getNameAsString();
+ long maxLookbackInMillis =
+
UngroupedAggregateRegionObserver.getMaxLookbackInMillis(
+ tableName, store.getColumnFamilyName(),
+
BaseScannerRegionObserverConstants.getMaxLookbackInMillis(
+
c.getEnvironment().getConfiguration()));
+ maxLookbackInMillis =
CompactionScanner.getMaxLookbackInMillis(tableName,
Review Comment:
Why are we maintaining two maps of max lookback age: one in this class and
other in Compaction Scanner?
##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java:
##########
@@ -581,92 +601,135 @@ private boolean areMutationsInSameTable(Table
targetHTable, Region region) {
region.getTableDescriptor().getTableName().getName()) == 0);
}
+ @Override
+ public InternalScanner
preFlush(ObserverContext<RegionCoprocessorEnvironment> c, Store store,
+ InternalScanner scanner, FlushLifeCycleTracker tracker) throws
IOException {
+ if (!isPhoenixTableTTLEnabled(c.getEnvironment().getConfiguration())) {
+ return scanner;
+ } else {
+ return User.runAsLoginUser(new
PrivilegedExceptionAction<InternalScanner>() {
+ @Override public InternalScanner run() throws Exception {
+ String tableName =
c.getEnvironment().getRegion().getRegionInfo().getTable()
+ .getNameAsString();
+ long maxLookbackInMillis =
+
UngroupedAggregateRegionObserver.getMaxLookbackInMillis(
+ tableName, store.getColumnFamilyName(),
+
BaseScannerRegionObserverConstants.getMaxLookbackInMillis(
+
c.getEnvironment().getConfiguration()));
+ maxLookbackInMillis =
CompactionScanner.getMaxLookbackInMillis(tableName,
+ store.getColumnFamilyName(), maxLookbackInMillis);
+ return new CompactionScanner(c.getEnvironment(), store,
scanner,
+ maxLookbackInMillis, null, null, false, true);
+ }
+ });
+ }
+ }
+
@Override
public InternalScanner
preCompact(ObserverContext<RegionCoprocessorEnvironment> c, Store store,
InternalScanner scanner, ScanType
scanType, CompactionLifeCycleTracker tracker,
CompactionRequest request) throws
IOException {
- if (scanType.equals(ScanType.COMPACT_DROP_DELETES)) {
- final TableName tableName =
c.getEnvironment().getRegion().getRegionInfo().getTable();
- // Compaction and split upcalls run with the effective user
context of the requesting user.
- // This will lead to failure of cross cluster RPC if the effective
user is not
- // the login user. Switch to the login user context to ensure we
have the expected
- // security context.
- return User.runAsLoginUser(new
PrivilegedExceptionAction<InternalScanner>() {
- @Override
- public InternalScanner run() throws Exception {
- InternalScanner internalScanner = scanner;
- if (request.isMajor()) {
- boolean isDisabled = false;
- boolean isMultiTenantIndexTable = false;
- if
(tableName.getNameAsString().startsWith(MetaDataUtil.VIEW_INDEX_TABLE_PREFIX)) {
- isMultiTenantIndexTable = true;
- }
- final String fullTableName = isMultiTenantIndexTable ?
-
SchemaUtil.getParentTableNameFromIndexTable(tableName.getNameAsString(),
- MetaDataUtil.VIEW_INDEX_TABLE_PREFIX) :
- tableName.getNameAsString();
- PTable table = null;
- try (PhoenixConnection conn =
QueryUtil.getConnectionOnServer(
-
compactionConfig).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);
- // non-Phoenix HBase tables won't be found, do
nothing
- } else {
- LOGGER.error(
- "Unable to modify compaction scanner
to retain deleted "
- + "cells for a table with
disabled Index; "
- + fullTableName, e);
- }
- }
- // The previous indexing design needs to retain delete
markers and deleted
- // cells to rebuild disabled indexes. Thus, we skip
major compaction for
- // them. GlobalIndexChecker is the coprocessor
introduced by the current
- // indexing design.
- if (table != null &&
-
!PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME.equals(fullTableName) &&
- !ServerUtil.hasCoprocessor(c.getEnvironment(),
- GlobalIndexChecker.class.getName())) {
- List<PTable>
- indexes =
- PTableType.INDEX.equals(table.getType()) ?
- Lists.newArrayList(table) :
- table.getIndexes();
- // FIXME need to handle views and indexes on views
as well
- for (PTable index : indexes) {
- if (index.getIndexDisableTimestamp() != 0) {
- LOGGER.info("Modifying major compaction
scanner to retain "
- + "deleted cells for a table with
disabled index: "
- + fullTableName);
- isDisabled = true;
- break;
- }
- }
- }
- if (table != null && !isDisabled &&
isPhoenixTableTTLEnabled) {
- internalScanner =
- new CompactionScanner(c.getEnvironment(),
store, scanner,
- MetaDataUtil.getMaxLookbackAge(
-
c.getEnvironment().getConfiguration(), table.getMaxLookbackAge()),
-
SchemaUtil.getEmptyColumnFamily(table),
- table.getEncodingScheme()
- ==
PTable.QualifierEncodingScheme.NON_ENCODED_QUALIFIERS ?
-
QueryConstants.EMPTY_COLUMN_BYTES :
-
table.getEncodingScheme().encode(QueryConstants.ENCODED_EMPTY_COLUMN_NAME)
- );
+
+ final TableName tableName =
c.getEnvironment().getRegion().getRegionInfo().getTable();
+ // Compaction and split upcalls run with the effective user context of
the requesting user.
+ // This will lead to failure of cross cluster RPC if the effective
user is not
+ // the login user. Switch to the login user context to ensure we have
the expected
+ // security context.
+ return User.runAsLoginUser(new
PrivilegedExceptionAction<InternalScanner>() {
+ @Override
+ public InternalScanner run() throws Exception {
+ InternalScanner internalScanner = scanner;
+ boolean keepDeleted = false;
+ boolean isMultiTenantIndexTable = false;
+ if
(tableName.getNameAsString().startsWith(MetaDataUtil.VIEW_INDEX_TABLE_PREFIX)) {
+ isMultiTenantIndexTable = true;
+ }
+ final String fullTableName = isMultiTenantIndexTable ?
+
SchemaUtil.getParentTableNameFromIndexTable(tableName.getNameAsString(),
+ MetaDataUtil.VIEW_INDEX_TABLE_PREFIX) :
+ tableName.getNameAsString();
+ PTable table = null;
+ Integer maxLookbackAge = null;
+ try (PhoenixConnection conn = QueryUtil.getConnectionOnServer(
+ compactionConfig).unwrap(PhoenixConnection.class)) {
+ table = conn.getTableNoCache(fullTableName);
+ maxLookbackAge = table.getMaxLookbackAge();
+ if (table.getType() == PTableType.INDEX) {
Review Comment:
I think we don't need this `if` check now as Indexes inherit max lookback of
tables and view indexes inherit max lookback of parent view?
--
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]
