You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/06/30 09:09:49 UTC
[GitHub] [pulsar] Technoboy- opened a new pull request, #16299: [Branch-2.7] [Cherry-pick] Fix incorrect returned last message ID while the lastConfirmedEntry with negative entry ID #12277
Technoboy- opened a new pull request, #16299:
URL: https://github.com/apache/pulsar/pull/16299
Cherry-pick #12277, #12237, #12161
### Motivation
When recovering a ManagedLedger, if the ManagedLedgers does not contain any ledgers,
the ManagedLedger will use the current Ledger ID and -1 to generate the `lastConfirmedEntry`.
For more details to see: https://github.com/apache/pulsar/blob/4bc3c405a565b1c756b9b70ff02a63ea06c32c0d/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java#L477
But for the compacted topic, all the data might be compacted and move to the compacted Ledger,
In this case, the broker will return X:-1 as the last message ID of the topic to the consumer,
the consumer will treat the negative entry ID as no data in this topic,
so `hasMoreMessages` method will return false, but there is compacted data in the topic.
The fix is as #12161 does to return the last message ID from the compacted Ledger if `lastConfirmedEntry` of the ManagedLedger with negative entry ID.
Improved the `testLastMessageIdForCompactedLedger` test to cover the new changes.
--
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
[GitHub] [pulsar] Technoboy- commented on a diff in pull request #16299: [Branch-2.7] [Cherry-pick] Fix incorrect returned last message ID while the lastConfirmedEntry with negative entry ID
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #16299:
URL: https://github.com/apache/pulsar/pull/16299#discussion_r913321225
##########
pulsar-broker/src/test/java/org/apache/pulsar/compaction/CompactedTopicTest.java:
##########
@@ -40,10 +40,13 @@
import org.apache.bookkeeper.client.BKException;
import org.apache.bookkeeper.client.BookKeeper;
import org.apache.bookkeeper.client.LedgerHandle;
+import org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl;
Review Comment:
removed.
--
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
[GitHub] [pulsar] Technoboy- commented on pull request #16299: [Branch-2.7] [Cherry-pick] Fix incorrect returned last message ID while the lastConfirmedEntry with negative entry ID
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on PR #16299:
URL: https://github.com/apache/pulsar/pull/16299#issuecomment-1174503387
> Do we have any place to track the test you ignored?
Yes, https://github.com/apache/pulsar/issues/16314
--
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
[GitHub] [pulsar] mattisonchao commented on a diff in pull request #16299: [Branch-2.7] [Cherry-pick] Fix incorrect returned last message ID while the lastConfirmedEntry with negative entry ID
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on code in PR #16299:
URL: https://github.com/apache/pulsar/pull/16299#discussion_r913320316
##########
pulsar-broker/src/test/java/org/apache/pulsar/compaction/CompactedTopicTest.java:
##########
@@ -40,10 +40,13 @@
import org.apache.bookkeeper.client.BKException;
import org.apache.bookkeeper.client.BookKeeper;
import org.apache.bookkeeper.client.LedgerHandle;
+import org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl;
Review Comment:
Why add these `import`?
--
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
[GitHub] [pulsar] mattisonchao commented on pull request #16299: [Branch-2.7] [Cherry-pick] Fix incorrect returned last message ID while the lastConfirmedEntry with negative entry ID
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on PR #16299:
URL: https://github.com/apache/pulsar/pull/16299#issuecomment-1174506941
When I read the code you changed, I found some weird things, and I re-confirm to the master branch.
Maybe this PR you also need to cherry-pick to branch-2.7. #13533
--
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
[GitHub] [pulsar] Technoboy- commented on pull request #16299: [Branch-2.7] [Cherry-pick] Fix incorrect returned last message ID while the lastConfirmedEntry with negative entry ID
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on PR #16299:
URL: https://github.com/apache/pulsar/pull/16299#issuecomment-1174512987
> When I read the code you changed, I found some weird things and re-confirmed it to the master branch. Maybe the PR #13533 you also need to cherry-pick to branch-2.7 that looks related to this PR.
yes, right. We need to cherry-pick all the these prs : https://github.com/apache/pulsar/pulls?q=is%3Apr+label%3Arelease%2F2.7.6+is%3Aclosed+label%3Acomponent%2Fcompaction+-label%3Acherry-picked%2Fbranch-2.7+
--
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
[GitHub] [pulsar] mattisonchao commented on pull request #16299: [Branch-2.7] [Cherry-pick] Fix incorrect returned last message ID while the lastConfirmedEntry with negative entry ID
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on PR #16299:
URL: https://github.com/apache/pulsar/pull/16299#issuecomment-1174504842
> > Do we have any place to track the test you ignored?
>
> Yes, #16314
Oh, I see. 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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- merged pull request #16299: [Branch-2.7] [Cherry-pick] Fix incorrect returned last message ID while the lastConfirmedEntry with negative entry ID
Posted by GitBox <gi...@apache.org>.
Technoboy- merged PR #16299:
URL: https://github.com/apache/pulsar/pull/16299
--
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