PHOENIX-7677 TTL_DELETE CDC event to use batch mutation #2247
Conversation
| for (int retryCount = 0; retryCount < cdcTtlMutationMaxRetries; retryCount++) { | ||
| try (Table cdcIndexTable = | ||
| env.getConnection().getTable(TableName.valueOf(cdcIndex.getPhysicalName().getBytes()))) { | ||
| cdcIndexTable.put(new ArrayList<>(pendingMutations.values())); |
There was a problem hiding this comment.
For my understanding, what are the semantics here for partial failures when trying to apply batch mutations? We will log failure even if some mutations from the batch were applied after all retries?
There was a problem hiding this comment.
Since we are using batchMutate() on the table where all the rowkeys are most likely expected to go to same region, either all mutations will be successful or none
| * that all 82 rows have CDC TTL_DELETE events recorded with correct pre-image data. | ||
| */ | ||
| @Test | ||
| public void testCDCBatchMutationsForTTLExpiredRows() throws Exception { |
There was a problem hiding this comment.
Is there a way to verify the actual batching and retry logic from this IT? Do you think a metric for this operation would be helpful?
There was a problem hiding this comment.
Metric would be helpful but initiating retry is not that simple, certainly should be kept separately
There was a problem hiding this comment.
@palashc i have added bunch of logs, do you think we still need metrics?
| */ | ||
| public final class CDCCompactionUtil { | ||
|
|
||
| private static final Logger LOGGER = LoggerFactory.getLogger(CDCCompactionUtil.class); | ||
|
|
||
| // Shared cache for row images across all CompactionScanner instances in the JVM. | ||
| // Entries expire after 1200 seconds (20 minutes) by default. | ||
| // The JVM level cache helps merge the pre-image for the row with multiple CFs. |
There was a problem hiding this comment.
Is it guaranteed that for multiple CFs all compactionscanner instances will run in the same JVM ? What if the RS aborts between compaction on two CFs ?
There was a problem hiding this comment.
Yes, this is best case scenario only. Let me add a note here. Generating TTL_DELETE event for multi-CF is not guaranteed to be accurate due to the edge case with server crash.
There was a problem hiding this comment.
Rather than comment in the code, let me clarify this in Jira
There was a problem hiding this comment.
Let me also evaluate doing batch writes only during CompactionScanner close(). This way, we might be able to guarantee no incorrect values for the CDC event. Let me think about this.
Jira: PHOENIX-7677
Inserting TTL_DELETE CDC event from CompactionScanner for single expired row at a time can cause slight degradation to the write workload to the CDC index table.
To improve the performance of inserting TTL_DELETE events, CompactionScanner can accumulate 50 mutations by default and perform the batchMutate(). Any mutations that are not inserted into the CDC index should be inserted with batchMutate() while closing the CompactionScanner.