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/04/28 07:00:08 UTC
[GitHub] [pulsar] poorbarcode opened a new pull request, #15363: [fix] [broker] Disabled deduplication, but ledger-store will not be d…
poorbarcode opened a new pull request, #15363:
URL: https://github.com/apache/pulsar/pull/15363
### Motivation
When execute command `ulsar-admin topicPolicies get-deduplication persistent://{tenant}/{ns}/{topic}` failure.
Then the `deduplication task` will be disabled, and cursor 'pulsar.dedup' will not move still restart `broker`.
But zk-node: `/managed-ledgers/{tenant}/{ns}/persistent/{topic}/pulsar.dedup` still exists.
Errors occur: this topic's ledger-store will not be deleted anymore, even if all message has been acknowledged.
### Modifications
When `checkDeduplicationStatus()` failure, stop this topic.
### Documentation
Need to update docs?
- [ ] `doc-required`
- [x] `no-need-doc`
- [ ] `doc`
- [ ] `doc-added`
--
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] poorbarcode commented on pull request #15363: [fix] [broker] Disabled deduplication, but ledger-store will not be d…
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #15363:
URL: https://github.com/apache/pulsar/pull/15363#issuecomment-1111965992
> Little question: I'm not sure if the issue is unrecoverable, but it feels a little bad to close the topic. Please let me know what you think.
> I'm not sure if the issue is unrecoverable, but it feels a little bad to close the topic.
If `deduplication` can't work properly, means `topic` can't work properly.
When user changes `deduplication status`, it should be clear that works or not.
Do `close topic` will rerty `start deduplication task` or told user `topic was done`, both of these results are useful to user
--
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] github-actions[bot] commented on pull request #15363: [fix] [broker] Disabled deduplication, but ledger-store will not be d…
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15363:
URL: https://github.com/apache/pulsar/pull/15363#issuecomment-1146498004
The pr had no activity for 30 days, mark with Stale label.
--
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 #15363: [fix] [broker] Disabled deduplication, but ledger-store will not be d…
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on PR #15363:
URL: https://github.com/apache/pulsar/pull/15363#issuecomment-1111835563
Another problem:
Can we add tests to verify this issue?
--
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] poorbarcode commented on pull request #15363: [fix] [broker] Disabled deduplication, but ledger-store will not be d…
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #15363:
URL: https://github.com/apache/pulsar/pull/15363#issuecomment-1113928841
/pulsarbot rerun-failure-checks
--
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] michaeljmarshall commented on pull request #15363: [fix] [broker] Disabled deduplication, but ledger-store will not be d…
Posted by "michaeljmarshall (via GitHub)" <gi...@apache.org>.
michaeljmarshall commented on PR #15363:
URL: https://github.com/apache/pulsar/pull/15363#issuecomment-1610272184
As discussed on the mailing list https://lists.apache.org/thread/w4jzk27qhtosgsz7l9bmhf1t7o9mxjhp, there is no plan to release 2.9.6, so I am going to remove the release/2.9.6 label
--
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] github-actions[bot] commented on pull request #15363: [fix] [broker] Disabled deduplication, but ledger-store will not be d…
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15363:
URL: https://github.com/apache/pulsar/pull/15363#issuecomment-1258878045
The pr had no activity for 30 days, mark with Stale label.
--
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] congbobo184 commented on pull request #15363: [fix] [broker] Disabled deduplication, but ledger-store will not be d…
Posted by GitBox <gi...@apache.org>.
congbobo184 commented on PR #15363:
URL: https://github.com/apache/pulsar/pull/15363#issuecomment-1318538778
@poorbarcode hi, I move this PR to release/2.9.5, if you have any questions, please ping me. 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] poorbarcode commented on a diff in pull request #15363: [fix] [broker] Disabled deduplication, but ledger-store will not be d…
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #15363:
URL: https://github.com/apache/pulsar/pull/15363#discussion_r860667133
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java:
##########
@@ -3060,7 +3060,21 @@ public void onUpdate(TopicPolicies policies) {
checkReplicationAndRetryOnFailure();
}
- checkDeduplicationStatus();
+ /**
+ * When execute command "pulsar-admin topicPolicies get-deduplication persistent://{tenant}/{ns}/{topic}"
+ * but "store.asyncRemoveCursor('pulsar.dedup')" failure, the 'pulsar-dedup' cursor will be disabled,
+ * zk-node: "/managed-ledgers/{tenant}/{ns}/persistent/{topic}/pulsar.dedup" still exists.
+ * Then this topic's ledger-store will not be deleted anymore, even if all message has been acknowledged.
+ * So when `checkDeduplicationStatus()` failure, stop this topic.
+ */
+ try {
+ checkDeduplicationStatus().get();
Review Comment:
Why ? I tried it , it works.
--
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 #15363: [fix] [broker] Disabled deduplication, but ledger-store will not be d…
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on code in PR #15363:
URL: https://github.com/apache/pulsar/pull/15363#discussion_r860556076
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java:
##########
@@ -3060,7 +3060,21 @@ public void onUpdate(TopicPolicies policies) {
checkReplicationAndRetryOnFailure();
}
- checkDeduplicationStatus();
+ /**
+ * When execute command "pulsar-admin topicPolicies get-deduplication persistent://{tenant}/{ns}/{topic}"
+ * but "store.asyncRemoveCursor('pulsar.dedup')" failure, the 'pulsar-dedup' cursor will be disabled,
+ * zk-node: "/managed-ledgers/{tenant}/{ns}/persistent/{topic}/pulsar.dedup" still exists.
+ * Then this topic's ledger-store will not be deleted anymore, even if all message has been acknowledged.
+ * So when `checkDeduplicationStatus()` failure, stop this topic.
+ */
+ try {
+ checkDeduplicationStatus().get();
Review Comment:
At first glance, we cannot call sync method inside async method.
--
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] github-actions[bot] commented on pull request #15363: [fix] [broker] Disabled deduplication, but ledger-store will not be d…
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15363:
URL: https://github.com/apache/pulsar/pull/15363#issuecomment-1170787028
@poorbarcode Please provide a correct documentation label for your PR.
Instructions see [Pulsar Documentation Label Guide](https://docs.google.com/document/d/1Qw7LHQdXWBW9t2-r-A7QdFDBwmZh6ytB4guwMoXHqc0).
--
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] github-actions[bot] commented on pull request #15363: [fix] [broker] Disabled deduplication, but ledger-store will not be d…
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15363:
URL: https://github.com/apache/pulsar/pull/15363#issuecomment-1170785572
@poorbarcode Please provide a correct documentation label for your PR.
Instructions see [Pulsar Documentation Label Guide](https://docs.google.com/document/d/1Qw7LHQdXWBW9t2-r-A7QdFDBwmZh6ytB4guwMoXHqc0).
--
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 #15363: [fix] [broker] Disabled deduplication, but ledger-store will not be d…
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on code in PR #15363:
URL: https://github.com/apache/pulsar/pull/15363#discussion_r860556076
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java:
##########
@@ -3060,7 +3060,21 @@ public void onUpdate(TopicPolicies policies) {
checkReplicationAndRetryOnFailure();
}
- checkDeduplicationStatus();
+ /**
+ * When execute command "pulsar-admin topicPolicies get-deduplication persistent://{tenant}/{ns}/{topic}"
+ * but "store.asyncRemoveCursor('pulsar.dedup')" failure, the 'pulsar-dedup' cursor will be disabled,
+ * zk-node: "/managed-ledgers/{tenant}/{ns}/persistent/{topic}/pulsar.dedup" still exists.
+ * Then this topic's ledger-store will not be deleted anymore, even if all message has been acknowledged.
+ * So when `checkDeduplicationStatus()` failure, stop this topic.
+ */
+ try {
+ checkDeduplicationStatus().get();
Review Comment:
At first glance, we cannot call the sync method inside the async method.
--
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] poorbarcode commented on pull request #15363: [fix] [broker] Disabled deduplication, but ledger-store will not be d…
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #15363:
URL: https://github.com/apache/pulsar/pull/15363#issuecomment-1111959261
> Little question: I'm not sure if the issue is unrecoverable, but it feels a little bad to close the topic. Please let me know what you think.
> Another question: Can we add tests to verify this issue?
OK. Thank you for reminding me. liang zai
--
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] github-actions[bot] commented on pull request #15363: [fix] [broker] Disabled deduplication, but ledger-store will not be d…
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15363:
URL: https://github.com/apache/pulsar/pull/15363#issuecomment-1170786066
@poorbarcode Please provide a correct documentation label for your PR.
Instructions see [Pulsar Documentation Label Guide](https://docs.google.com/document/d/1Qw7LHQdXWBW9t2-r-A7QdFDBwmZh6ytB4guwMoXHqc0).
--
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] poorbarcode commented on pull request #15363: [fix] [broker] Disabled deduplication, but ledger-store will not be d…
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #15363:
URL: https://github.com/apache/pulsar/pull/15363#issuecomment-1154268113
/pulsarbot rerun-failure-checks
--
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] poorbarcode commented on pull request #15363: [fix] [broker] Disabled deduplication, but ledger-store will not be d…
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #15363:
URL: https://github.com/apache/pulsar/pull/15363#issuecomment-1153856061
/pulsarbot rerun-failure-checks
--
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