Re: [PR] [fix][broker] Fix readCompacted skipping tombstones within compaction horizon [pulsar]

["merlimat (via GitHub)" <gi...@apache.org>]

merlimat commented on PR #26190:
URL: https://github.com/apache/pulsar/pull/26190#issuecomment-6244445566

   @lhotari, on your review findings:
   
   1. Semantics: with the rework, only a reader that has already moved past its snapshot reads the raw topic, from that point on, which is what it would have read had the later compaction not run. Fresh readers are unchanged. The description now spells this out.
   2. Retention: when the topic's ledger at the recorded position has been deleted, reads fall back to the compacted ledger, which is the only copy of that data. So this narrows the window and doesn't close it. That's documented in the description and covered by `testTrimmedOriginalLedgerFallsBackToCompactedLedger`.
   3. Pluggable `TopicCompactionService`: an empty result no longer means "read the raw topic". The `readCompactedEntries` javadoc now describes when the service is consulted.
   4. Tests: renamed, a trailing `assertNull` added, the horizon is awaited explicitly instead of the compaction status (which can be stale from the first run), and unit tests now cover the max read position, the trimmed ledger and rewinds.
   5. The description follows the template now. I didn't find an existing issue for this.
   


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org