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 2021/11/11 15:55:48 UTC
[GitHub] [pulsar] congbobo184 opened a new pull request #12758: [Transaction] Fix topicTransactionBuffer handle null snapshot
congbobo184 opened a new pull request #12758:
URL: https://github.com/apache/pulsar/pull/12758
fix https://github.com/apache/pulsar/issues/12754
### Motivation
Now when delete topic, we will write a null value to Transaction buffer snapshot topic, other topic recover by this transaction buffer snapshot system topic, will produce NPE
### Modifications
judge NPE logic
### Verifying this change
add some test for it
--
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 merged pull request #12758: [Transaction] Fix topicTransactionBuffer handle null snapshot
Posted by GitBox <gi...@apache.org>.
congbobo184 merged pull request #12758:
URL: https://github.com/apache/pulsar/pull/12758
--
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 #12758: [Transaction] Fix topicTransactionBuffer handle null snapshot
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12758:
URL: https://github.com/apache/pulsar/pull/12758#issuecomment-966417028
@congbobo184:Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? 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] github-actions[bot] commented on pull request #12758: [Transaction] Fix topicTransactionBuffer handle null snapshot
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12758:
URL: https://github.com/apache/pulsar/pull/12758#issuecomment-966416729
@congbobo184:Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? 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] gaoran10 commented on a change in pull request #12758: [Transaction] Fix topicTransactionBuffer handle null snapshot
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on a change in pull request #12758:
URL: https://github.com/apache/pulsar/pull/12758#discussion_r747943032
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/transaction/buffer/impl/TopicTransactionBuffer.java
##########
@@ -537,10 +537,11 @@ public void run() {
try {
boolean hasSnapshot = false;
while (reader.hasMoreEvents()) {
- hasSnapshot = true;
Message<TransactionBufferSnapshot> message = reader.readNext();
TransactionBufferSnapshot transactionBufferSnapshot = message.getValue();
- if (topic.getName().equals(transactionBufferSnapshot.getTopicName())) {
+ if (transactionBufferSnapshot != null
Review comment:
Could we use the key of the message to verify the message belong to this topic? Then we could reduce some deserialization 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] gaoran10 commented on a change in pull request #12758: [Transaction] Fix topicTransactionBuffer handle null snapshot
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on a change in pull request #12758:
URL: https://github.com/apache/pulsar/pull/12758#discussion_r747943032
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/transaction/buffer/impl/TopicTransactionBuffer.java
##########
@@ -537,10 +537,11 @@ public void run() {
try {
boolean hasSnapshot = false;
while (reader.hasMoreEvents()) {
- hasSnapshot = true;
Message<TransactionBufferSnapshot> message = reader.readNext();
TransactionBufferSnapshot transactionBufferSnapshot = message.getValue();
- if (topic.getName().equals(transactionBufferSnapshot.getTopicName())) {
+ if (transactionBufferSnapshot != null
Review comment:
Could we use the key of the message to verify the message belong to this topic? This may reduce some deserialization 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] eolivelli commented on pull request #12758: [Transaction] Fix topicTransactionBuffer handle null snapshot
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #12758:
URL: https://github.com/apache/pulsar/pull/12758#issuecomment-966910254
This patch actually fixes my issue, thank you very much !
--
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