-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed message rate out with batches to count messages/s (#466) #474
Conversation
} | ||
}); | ||
t1.start(); | ||
Thread.sleep(60000); // one minute wait for stats to be updated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't add one more minute to unit tests :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reduced the sleep to two second and manually call the updateRates function.
Reducing the sleep to one second causes the rate to be shown as 0.
@@ -185,8 +186,10 @@ public String consumerName() { | |||
readChecksum(metadataAndPayload); | |||
} | |||
|
|||
MessageMetadata messageMetaData = Commands.parseMessageMetadata(metadataAndPayload); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't have to deserialize messageMetadata again. at line #160 we already know number of messages which is going to be dispatched. can we use that number?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
pulsar.getBrokerService().updateRates(); | ||
double actualRate = admin.persistentTopics().getStats(topicName).msgRateOut; | ||
log.info("actualRate : {}, produceRate : {}", actualRate, produceRate); | ||
assertTrue((produceRate - 1) <= actualRate && actualRate <= (produceRate + 1)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jai1 Can you run this test multiple times to ensure is not intermittently failing?
@jai1 The new test is failing
|
Fix for #466
Don't Merge yet