-
Notifications
You must be signed in to change notification settings - Fork 3.7k
PIP-3 : Introduce message-dispatch rate limiting #634
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -144,29 +144,31 @@ public String consumerName() { | |
| * | ||
| * @return a promise that can be use to track when all the data has been written into the socket | ||
| */ | ||
| public Pair<ChannelPromise, Integer> sendMessages(final List<Entry> entries) { | ||
| public SendMessageInfo sendMessages(final List<Entry> entries) { | ||
| final ChannelHandlerContext ctx = cnx.ctx(); | ||
| final MutablePair<ChannelPromise, Integer> sentMessages = new MutablePair<ChannelPromise, Integer>(); | ||
| final SendMessageInfo sentMessages = new SendMessageInfo(); | ||
| final ChannelPromise writePromise = ctx.newPromise(); | ||
| sentMessages.setLeft(writePromise); | ||
| sentMessages.channelPromse = writePromise; | ||
| if (entries.isEmpty()) { | ||
| if (log.isDebugEnabled()) { | ||
| log.debug("[{}] List of messages is empty, triggering write future immediately for consumerId {}", | ||
| subscription, consumerId); | ||
| } | ||
| writePromise.setSuccess(); | ||
| sentMessages.setRight(0); | ||
| sentMessages.totalSentMessages = 0; | ||
| sentMessages.totalSentMessageBytes = 0; | ||
| return sentMessages; | ||
| } | ||
|
|
||
| try { | ||
| sentMessages.setRight(updatePermitsAndPendingAcks(entries)); | ||
| updatePermitsAndPendingAcks(entries, sentMessages); | ||
| } catch (PulsarServerException pe) { | ||
| log.warn("[{}] [{}] consumer doesn't support batch-message {}", subscription, consumerId, | ||
| cnx.getRemoteEndpointProtocolVersion()); | ||
|
|
||
| subscription.markTopicWithBatchMessagePublished(); | ||
| sentMessages.setRight(0); | ||
| sentMessages.totalSentMessages = 0; | ||
| sentMessages.totalSentMessageBytes = 0; | ||
| // disconnect consumer: it will update dispatcher's availablePermits and resend pendingAck-messages of this | ||
| // consumer to other consumer | ||
| disconnect(); | ||
|
|
@@ -235,7 +237,7 @@ public static int getBatchSizeforEntry(ByteBuf metadataAndPayload, String subscr | |
| return -1; | ||
| } | ||
|
|
||
| int updatePermitsAndPendingAcks(final List<Entry> entries) throws PulsarServerException { | ||
| void updatePermitsAndPendingAcks(final List<Entry> entries, SendMessageInfo sentMessages) throws PulsarServerException { | ||
| int permitsToReduce = 0; | ||
| Iterator<Entry> iter = entries.iterator(); | ||
| boolean unsupportedVersion = false; | ||
|
|
@@ -276,7 +278,8 @@ int updatePermitsAndPendingAcks(final List<Entry> entries) throws PulsarServerEx | |
| } | ||
|
|
||
| msgOut.recordMultipleEvents(permitsToReduce, totalReadableBytes); | ||
| return permitsToReduce; | ||
| sentMessages.totalSentMessages = permitsToReduce; | ||
| sentMessages.totalSentMessageBytes = totalReadableBytes; | ||
| } | ||
|
|
||
| public boolean isWritable() { | ||
|
|
@@ -575,5 +578,31 @@ private void clearUnAckedMsgs(Consumer consumer) { | |
| subscription.addUnAckedMessages(-unaAckedMsgs); | ||
| } | ||
|
|
||
| public static class SendMessageInfo { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is this information used?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At dispatcher, we want to know how many messages/bytes we actually sent to manage the permits. Therefore, at here in consumer, we exactly know how many msgs/bytes is sent and return it to dispatcher. Earlier, |
||
| ChannelPromise channelPromse; | ||
| int totalSentMessages; | ||
| long totalSentMessageBytes; | ||
|
|
||
| public ChannelPromise getChannelPromse() { | ||
| return channelPromse; | ||
| } | ||
| public void setChannelPromse(ChannelPromise channelPromse) { | ||
| this.channelPromse = channelPromse; | ||
| } | ||
| public int getTotalSentMessages() { | ||
| return totalSentMessages; | ||
| } | ||
| public void setTotalSentMessages(int totalSentMessages) { | ||
| this.totalSentMessages = totalSentMessages; | ||
| } | ||
| public long getTotalSentMessageBytes() { | ||
| return totalSentMessageBytes; | ||
| } | ||
| public void setTotalSentMessageBytes(long totalSentMessageBytes) { | ||
| this.totalSentMessageBytes = totalSentMessageBytes; | ||
| } | ||
|
|
||
| } | ||
|
|
||
| private static final Logger log = LoggerFactory.getLogger(Consumer.class); | ||
| } | ||
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.
Is this still needed since we're not using local policies anymore?
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.
actually, this method returns
CompletableFuture<>so, we should fail the future rather throwing runtime exception.