[ 
https://issues.apache.org/jira/browse/PHOENIX-8000?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Shubham Roy updated PHOENIX-8000:
---------------------------------
    Description: 
h2. Summary

A SYSTEM table configured with a *conditional TTL* never has that TTL applied 
during compaction. Expired rows are hidden at read time (masking) but are 
*never physically removed* from the HFiles, so they accumulate indefinitely. 
\{{SYSTEM.CDC_STREAM}} — created with a conditional TTL to expire 
closed-partition rows after a minimum age — is the concrete instance.

h2. Root cause

In \{{CompactionScanner.NonPartitionedTableTTLTracker}}, SYSTEM tables are 
unconditionally routed to the HBase column-family *descriptor* TTL instead of 
the table's compiled *conditional* TTL expression:

{code:java}
boolean isSystemTable = pTable.getType() == PTableType.SYSTEM;
...
if (isSystemTable
|| 
pTable.getTTLExpression().equals(TTL_EXPRESSION_DEFINED_IN_TABLE_DESCRIPTOR)) {
ColumnFamilyDescriptor cfd = store.getColumnFamilyDescriptor();
ttlExpr = TTLExpressionFactory.create(cfd.getTimeToLive()); // descriptor TTL, 
NOT the conditional expr
} else {
ttlExpr = !pTable.getTTLExpression().equals(TTL_EXPRESSION_NOT_DEFINED)
? pTable.getCompiledTTLExpression(pConn) // conditional path - skipped for 
SYSTEM
: TTL_EXPRESSION_FOREVER;
}
{code}

For a table whose TTL is a *conditional* expression, the CF-descriptor TTL is 
\{{FOREVER}} (Phoenix stores a numeric TTL in the descriptor only for _literal_ 
expressions; conditional TTLs are stored in SYSTEM.CATALOG and the descriptor 
is left \{{FOREVER}}). Consequently, for a SYSTEM table:

* \{{ttlExprForRow}} resolves to a literal \{{FOREVER}}, so 
\{{RowContext.hasConditionalTTL()}} is always \{{false}};
* therefore \{{postProcessForConditionalTTL(...)}} — the only place the 
conditional expression is evaluated and an expired row is dropped 
(\{{result.clear()}}) — is *never reached*;
* so expiry is enforced *only* as read-time masking (\{{TTLRegionScanner}}), 
never at compaction.

Minor compaction / flush also never purge (they use \{{TTL_EXPRESSION_FOREVER}} 
by design), so major compaction is the only place this _should_ happen — and it 
does not for SYSTEM tables.

h2. Steps to reproduce

# Create/use a SYSTEM table with a conditional TTL (e.g. 
\{{SYSTEM.CDC_STREAM}}, whose TTL is {{PARTITION_END_TIME IS NOT NULL AND
TO_NUMBER(CURRENT_TIME()) - TO_NUMBER(PHOENIX_ROW_TIMESTAMP()) >= <min-age>}}, 
default ~30h).
# Insert rows that satisfy the expiry condition and advance the clock past the 
min age.
# Trigger a major compaction on the table.

*Observed:* the rows are still physically present in the HFiles (a raw / 
all-versions scan still returns them); they are only hidden from normal Phoenix 
reads.
*Expected:* the expired rows are physically removed by the major compaction.

h2. Expected behavior

SYSTEM tables should honor their conditional TTL at (major) compaction — 
evaluating the compiled conditional expression and physically purging matching 
rows — the same way non-SYSTEM tables do. The \{{isSystemTable}} short-circuit 
should not force a conditional-TTL table onto the (\{{FOREVER}}) descriptor TTL.

h2. Proposed fix

In \{{NonPartitionedTableTTLTracker}}, do not route a SYSTEM table to the 
descriptor TTL when it has a *conditional* TTL expression; use the table's 
compiled conditional TTL (\{{pTable.getCompiledTTLExpression(...)}}) so 
\{{hasConditionalTTL()}} becomes true and 
\{{postProcessForConditionalTTL(...)}} runs at major compaction. Keep the 
descriptor-TTL path only for tables that define TTL in the descriptor / literal 
TTLs.

h2. Test

Add an integration test that:

* uses a SYSTEM table with a conditional TTL (\{{SYSTEM.CDC_STREAM}}),
* writes rows satisfying the condition and ages them past the configured 
min-age (~30h),
* runs a *major compaction*, and
* asserts the expired rows are *physically removed* (e.g. a raw / all-versions 
scan no longer returns them), not merely masked.

h2. Impact

Any SYSTEM table with a conditional TTL retains expired rows on disk forever, 
causing unbounded growth of the system table and defeating the intent of the 
conditional TTL (rows are hidden but never reclaimed).

  was:
Description

Summary

A SYSTEM table configured with a conditional TTL never has that TTL applied 
during compaction. Expired rows are hidden at read time (masking) but are never 
physically removed from the HFiles, so they accumulate indefinitely. 
SYSTEM.CDC_STREAM — which is created with a conditional TTL to expire 
closed-partition rows after a minimum age — is the concrete instance where this 
shows up.

Root cause

In CompactionScanner.NonPartitionedTableTTLTracker, SYSTEM tables are 
unconditionally routed to the HBase column-family-descriptor TTL instead of the 
table's compiled conditional TTL expression:

```
boolean isSystemTable = pTable.getType() == PTableType.SYSTEM;
...
if (isSystemTable
|| 
pTable.getTTLExpression().equals(TTL_EXPRESSION_DEFINED_IN_TABLE_DESCRIPTOR)) {
ColumnFamilyDescriptor cfd = store.getColumnFamilyDescriptor();
ttlExpr = TTLExpressionFactory.create(cfd.getTimeToLive()); // descriptor TTL, 
NOT the conditional expr
} else {
ttlExpr = !pTable.getTTLExpression().equals(TTL_EXPRESSION_NOT_DEFINED)
? pTable.getCompiledTTLExpression(pConn) // conditional path — skipped for 
SYSTEM
: TTL_EXPRESSION_FOREVER;
}
```

For a table whose TTL is a conditional expression, the CF-descriptor TTL is 
FOREVER (Phoenix stores a numeric TTL in the descriptor only for literal 
expressions; conditional TTLs are stored in SYSTEM.CATALOG and the descriptor 
is left FOREVER). Consequently, for a SYSTEM table:

- ttlExprForRow resolves to a literal FOREVER, so 
RowContext.hasConditionalTTL() is always false;
- therefore postProcessForConditionalTTL(...) — the only place the conditional 
expression is evaluated and an expired row is dropped (result.clear()) — is 
never reached;
- so expiry is enforced only as read-time masking (TTLRegionScanner), never at 
compaction.

Minor compaction / flush also never purge (they use TTL_EXPRESSION_FOREVER by 
design), so major compaction is the only place this should happen — and it 
doesn't for SYSTEM tables.

Steps to reproduce

1. Create/use a SYSTEM table with a conditional TTL (e.g. SYSTEM.CDC_STREAM, 
whose TTL is PARTITION_END_TIME IS NOT NULL AND TO_NUMBER(CURRENT_TIME()) -
TO_NUMBER(PHOENIX_ROW_TIMESTAMP()) >= <min-age>, default 
PHOENIX_CDC_STREAM_PARTITION_EXPIRY_MIN_AGE_MS ≈ 30h).
2. Insert rows that satisfy the expiry condition and advance the clock past the 
min age.
3. Trigger a major compaction on the table.
4. Observed: the rows are still physically present in the HFiles (a raw/setRaw 
scan still returns them); they are only hidden from normal Phoenix reads.
5. Expected: the expired rows are physically removed by the major compaction.

Expected behavior

SYSTEM tables should honor their conditional TTL at (major) compaction — 
evaluating the compiled conditional expression and physically purging matching 
rows — the same way non-SYSTEM tables do. The isSystemTable short-circuit 
should not force a conditional-TTL table onto the (FOREVER) descriptor TTL.

Proposed fix

In NonPartitionedTableTTLTracker, do not route a SYSTEM table to the descriptor 
TTL when it has a conditional TTL expression; use the table's compiled 
conditional TTL (pTable.getCompiledTTLExpression(...)) so hasConditionalTTL() 
becomes true and postProcessForConditionalTTL(...) runs at major compaction.
(Keep the descriptor-TTL path only for tables that genuinely define TTL in the 
descriptor / literal TTLs.)

Test

Add an integration test that:
- uses a SYSTEM table with a conditional TTL (SYSTEM.CDC_STREAM),
- writes rows satisfying the condition and ages them past the configured 
min-age (~30h),
- runs a major compaction, and
- asserts the expired rows are physically removed (e.g. a raw scan / 
all-versions scan no longer returns them), not merely masked.

Impact

Any SYSTEM table with a conditional TTL retains expired rows on disk forever, 
causing unbounded growth of the system table and defeating the intent of the 
conditional TTL (rows are hidden but never reclaimed).


> Conditional TTL is not enforced at compaction for SYSTEM tables — expired 
> rows are never physically purged (e.g. SYSTEM.CDC_STREAM)
> -----------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: PHOENIX-8000
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-8000
>             Project: Phoenix
>          Issue Type: Bug
>          Components: phoenix
>    Affects Versions: 5.3.0, 5.3.1, 5.3.2
>            Reporter: Shubham Roy
>            Assignee: Shubham Roy
>            Priority: Minor
>             Fix For: 5.3.3
>
>
> h2. Summary
> A SYSTEM table configured with a *conditional TTL* never has that TTL applied 
> during compaction. Expired rows are hidden at read time (masking) but are 
> *never physically removed* from the HFiles, so they accumulate indefinitely. 
> \{{SYSTEM.CDC_STREAM}} — created with a conditional TTL to expire 
> closed-partition rows after a minimum age — is the concrete instance.
> h2. Root cause
> In \{{CompactionScanner.NonPartitionedTableTTLTracker}}, SYSTEM tables are 
> unconditionally routed to the HBase column-family *descriptor* TTL instead of 
> the table's compiled *conditional* TTL expression:
> {code:java}
> boolean isSystemTable = pTable.getType() == PTableType.SYSTEM;
> ...
> if (isSystemTable
> || 
> pTable.getTTLExpression().equals(TTL_EXPRESSION_DEFINED_IN_TABLE_DESCRIPTOR)) 
> {
> ColumnFamilyDescriptor cfd = store.getColumnFamilyDescriptor();
> ttlExpr = TTLExpressionFactory.create(cfd.getTimeToLive()); // descriptor 
> TTL, NOT the conditional expr
> } else {
> ttlExpr = !pTable.getTTLExpression().equals(TTL_EXPRESSION_NOT_DEFINED)
> ? pTable.getCompiledTTLExpression(pConn) // conditional path - skipped for 
> SYSTEM
> : TTL_EXPRESSION_FOREVER;
> }
> {code}
> For a table whose TTL is a *conditional* expression, the CF-descriptor TTL is 
> \{{FOREVER}} (Phoenix stores a numeric TTL in the descriptor only for 
> _literal_ expressions; conditional TTLs are stored in SYSTEM.CATALOG and the 
> descriptor is left \{{FOREVER}}). Consequently, for a SYSTEM table:
> * \{{ttlExprForRow}} resolves to a literal \{{FOREVER}}, so 
> \{{RowContext.hasConditionalTTL()}} is always \{{false}};
> * therefore \{{postProcessForConditionalTTL(...)}} — the only place the 
> conditional expression is evaluated and an expired row is dropped 
> (\{{result.clear()}}) — is *never reached*;
> * so expiry is enforced *only* as read-time masking (\{{TTLRegionScanner}}), 
> never at compaction.
> Minor compaction / flush also never purge (they use 
> \{{TTL_EXPRESSION_FOREVER}} by design), so major compaction is the only place 
> this _should_ happen — and it does not for SYSTEM tables.
> h2. Steps to reproduce
> # Create/use a SYSTEM table with a conditional TTL (e.g. 
> \{{SYSTEM.CDC_STREAM}}, whose TTL is {{PARTITION_END_TIME IS NOT NULL AND
> TO_NUMBER(CURRENT_TIME()) - TO_NUMBER(PHOENIX_ROW_TIMESTAMP()) >= 
> <min-age>}}, default ~30h).
> # Insert rows that satisfy the expiry condition and advance the clock past 
> the min age.
> # Trigger a major compaction on the table.
> *Observed:* the rows are still physically present in the HFiles (a raw / 
> all-versions scan still returns them); they are only hidden from normal 
> Phoenix reads.
> *Expected:* the expired rows are physically removed by the major compaction.
> h2. Expected behavior
> SYSTEM tables should honor their conditional TTL at (major) compaction — 
> evaluating the compiled conditional expression and physically purging 
> matching rows — the same way non-SYSTEM tables do. The \{{isSystemTable}} 
> short-circuit should not force a conditional-TTL table onto the 
> (\{{FOREVER}}) descriptor TTL.
> h2. Proposed fix
> In \{{NonPartitionedTableTTLTracker}}, do not route a SYSTEM table to the 
> descriptor TTL when it has a *conditional* TTL expression; use the table's 
> compiled conditional TTL (\{{pTable.getCompiledTTLExpression(...)}}) so 
> \{{hasConditionalTTL()}} becomes true and 
> \{{postProcessForConditionalTTL(...)}} runs at major compaction. Keep the 
> descriptor-TTL path only for tables that define TTL in the descriptor / 
> literal TTLs.
> h2. Test
> Add an integration test that:
> * uses a SYSTEM table with a conditional TTL (\{{SYSTEM.CDC_STREAM}}),
> * writes rows satisfying the condition and ages them past the configured 
> min-age (~30h),
> * runs a *major compaction*, and
> * asserts the expired rows are *physically removed* (e.g. a raw / 
> all-versions scan no longer returns them), not merely masked.
> h2. Impact
> Any SYSTEM table with a conditional TTL retains expired rows on disk forever, 
> causing unbounded growth of the system table and defeating the intent of the 
> conditional TTL (rows are hidden but never reclaimed).



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to