Add close method in the class of NegativeAcksTracker #12469
Conversation
|
@lordcheng10:Thanks for your contribution. For this PR, do we need to update docs? |
lordcheng10
changed the title
No need to add documentation |
codelipenghui
added
doc-not-needed
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java
Show resolved
Hide resolved
codelipenghui
requested review from
eolivelli,
merlimat,
315157973,
hangc0276,
congbobo184 and
gaoran10
|
ConsumerImplTest.testClose method has been added. Please review again，thanks! @codelipenghui |
| Exception checkException = null; | ||
| try { | ||
| if (consumer != null) { | ||
| consumer.close(); | ||
| } | ||
| } catch (Exception e) { | ||
| checkException = e; | ||
| } | ||
| Assert.assertNull(checkException); |
There was a problem hiding this comment.
Please confirm the timeout==null and the displayedMessages== null in the Tracker
There was a problem hiding this comment.
By default timeout is null and the nackedMessages is null. By calling consumer.negativeAcknowledge(new MessageIdImpl(-1, -1, -1)) to make timeout != null and nackedMessages != null .
Please review again,thanks! @315157973
There was a problem hiding this comment.
Then why not close Consumer after calling consumer.negativeAcknowledge?
Your unit test has no way to confirm that the Tracker is closed, and we can't even confirm that the close method has been called.
There was a problem hiding this comment.
you are right! I resubmitted the modification: close Consumer after calling consumer.negativeAcknowledge：
public void testClose() {
Exception checkException = null;
try {
if (consumer != null) {
consumer.negativeAcknowledge(new MessageIdImpl(-1, -1, -1));
consumer.close();
}
} catch (Exception e) {
checkException = e;
}
Assert.assertNull(checkException);
}
Please review again,thanks! @315157973
…egativeAcksTracker. By default timeout is null and the nackedMessages is null.
|
/pulsarbot run-failure-checks |
4 similar comments
|
/pulsarbot run-failure-checks |
|
/pulsarbot run-failure-checks |
|
/pulsarbot run-failure-checks |
|
/pulsarbot run-failure-checks |
|
/pulsarbot run-failure-checks |
2 similar comments
|
/pulsarbot run-failure-checks |
|
/pulsarbot run-failure-checks |
|
/pulsarbot run-failure-checks |
1 similar comment
|
/pulsarbot run-failure-checks |
|
/pulsarbot run-failure-checks |
3 similar comments
|
/pulsarbot run-failure-checks |
|
/pulsarbot run-failure-checks |
|
/pulsarbot run-failure-checks |
|
@codelipenghui PTAL,thanks! |
|
@codelipenghui 's comment has been addressed. Merged to master branch |
(cherry picked from commit 3694aa1)
(cherry picked from commit 3694aa1)
Add close method in the class of NegativeAcksTracker