You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by "sanjeet006py (via GitHub)" <gi...@apache.org> on 2024/09/09 11:24:18 UTC
[PR] PHOENIX-7402: Fix row getting expired partially even if updated within TTL [phoenix]
sanjeet006py opened a new pull request, #1973:
URL: https://github.com/apache/phoenix/pull/1973
The issues that were identified:
- The empty cells are not being persisted/collected during flushes and minor compactions which causes row to expire partially on next major compaction even if it was updated within TTL.
- To collect empty cells we need to be able to identify empty cells for a table and for that we need to pass PTable object to CompactionScanner. Retrieving PTable object can fail when all the RS which can hold SYSCAT are down/going down or SYSCAT itself is offline (RIT). Thus, in those cases we can persist/flush all the cells and let next minor/major compaction to do the filtering.
- To not make flushes and minor compactions heavy by trying to compute TTL per row and to keep using `TableTTLTrackerForFlushesAndMinor` we need to retain all the empty cells during flushes and minor compactions. In fact, for multi-CF case if we don't retain all empty cells during flushes and minor compactions then we need to do region level scan for empty CF store.
- Because for flushes and minor compactions we are using `TableTTLTrackerForFlushesAndMinor` which uses default TTL and not the actual TTL so, we need to avoid returning in short-circuited manner based of TTL or max lookback + TTL value. During flushes we retain delete markers and don't expire cells.
- While filling up the gap b/w the cells of a row using empty cells, we weren't taking into account that maybe the last cell in the input list (of empty cells) will be the one needed to fill the gap so, fixed that. Also, the input list of empty cells has cells in decreasing order of timestamp such that even the first cell is less than max timestamp of row but still can be needed to fill the gap.
- For multi-CF case, if we first do non-region level major compaction for empty CF then it can happen that we see some empty cells as extra not needed to be retained but then can be needed to keep cell from a different column family (non-empty CF) to not expire. So, when major compaction is being done for empty CF of a multi-CF table then we should always go for region level compaction else we can see partial row expiry.
- Introduced a separate server side connection profile: `PHOENIX_CONNECTION_FROM_COPROC` to create a HConnection with just 1 retry at HBase client level. This will be used for getting PTable during `preFlush`. The retries have been kept to 1 to avoid delaying region close operation as flush is also called during region close and additionally, retrieval of PTable is handled gracefully by flushing all the cells.
- Till now (before this PR) during flush operation, the max lookback value that was used was the one cached in last compaction right before the flush. Thus, any changes to table level max lookback weren't strongly consistent as the value of max lookback being used can vary from RS to RS based when last compaction was done. With PTable being accessible during `preFlush` that gap is also fixed. JIRA: [PHOENIX-7347](https://issues.apache.org/jira/browse/PHOENIX-7347).
- Added the test coverage for the above fixes along with test coverage for the case when PTable is not retrievable during flush and all cells are getting persisted/retained.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7402: Fix row getting expired partially even if updated within TTL [phoenix]
Posted by "kadirozde (via GitHub)" <gi...@apache.org>.
kadirozde commented on code in PR #1973:
URL: https://github.com/apache/phoenix/pull/1973#discussion_r1771825342
##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/CompactionScanner.java:
##########
@@ -154,6 +155,7 @@ public class CompactionScanner implements InternalScanner {
private long inputCellCount = 0;
private long outputCellCount = 0;
private boolean phoenixLevelOnly = false;
+ private static boolean forceMinorCompaction = false;
Review Comment:
Adding a comment stating that this is used only for testing would help here.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7402: Fix row getting expired partially even if updated within TTL [phoenix]
Posted by "sanjeet006py (via GitHub)" <gi...@apache.org>.
sanjeet006py commented on PR #1973:
URL: https://github.com/apache/phoenix/pull/1973#issuecomment-2371814169
@kadirozde I have addressed the comment. UTs/ITs which are failing are flappers. Are we good to merge the PR and can you please help with merging it. Thanks
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7402: Fix row getting expired partially even if updated within TTL [phoenix]
Posted by "sanjeet006py (via GitHub)" <gi...@apache.org>.
sanjeet006py commented on PR #1973:
URL: https://github.com/apache/phoenix/pull/1973#issuecomment-2353962235
@kadirozde @virajjasani @tkhurana @jpisaac The PR is ready for review. Please review.
I plan to follow-up with a PR to avoid doing region level scan when major compacting empty CF of a multi-CF table. I have thought of an approach but need to do its feasibility analysis and get it more concrete so, maybe first we can merge this PR and then next I can follow-up with optimization. Thanks
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7402: Fix row getting expired partially even if updated within TTL [phoenix]
Posted by "kadirozde (via GitHub)" <gi...@apache.org>.
kadirozde commented on PR #1973:
URL: https://github.com/apache/phoenix/pull/1973#issuecomment-2368963580
> @kadirozde @virajjasani @tkhurana @jpisaac The PR is ready for review. Please review.
>
> I plan to follow-up with a PR to avoid doing region level scan when major compacting empty CF of a multi-CF table. I have thought of an approach but need to do its feasibility analysis and get it more concrete so, maybe first we can merge this PR and then next I can follow-up with optimization. Thanks
Sounds good.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7402: Fix row getting expired partially even if updated within TTL [phoenix]
Posted by "kadirozde (via GitHub)" <gi...@apache.org>.
kadirozde merged PR #1973:
URL: https://github.com/apache/phoenix/pull/1973
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org