poorbarcode commented on code in PR #20984:
URL: https://github.com/apache/pulsar/pull/20984#discussion_r1293085970
##########
pulsar-broker/src/main/java/org/apache/pulsar/compaction/CompactedTopicImpl.java:
##########
@@ -70,14 +70,14 @@ public CompactedTopicImpl(BookKeeper bk) {
@Override
public CompletableFuture<CompactedTopicContext>
newCompactedLedger(Position p, long compactedLedgerId) {
synchronized (this) {
- compactionHorizon = (PositionImpl) p;
-
CompletableFuture<CompactedTopicContext> previousContext =
compactedTopicContext;
compactedTopicContext = openCompactedLedger(bk, compactedLedgerId);
+ compactionHorizon = (PositionImpl) p;
Review Comment:
We can not guarantee that the reading of `compactionHorizon` and
`compactedTopicContext` safety even if this patch is merged. We should add some
code-comment on the method `getCompactedTopicContextFuture` and
`getCompactedTopicContext` like this:
```
/**
* If you want to guarantee that `compactionHorizon` and
`compactedTopicContext` match, you need to call this method in
"synchronized(compartor){ ... }" lock block
*/
```
--
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]
