Revert "feat: Change send to not block on waiting receipt (#298)"

[mattisonchao]

This reverts commit 5c245cd.

Motivation

Pr #298 made a change to the sending process so that we don't need to wait for the receipt to send the next message. that's a good idea to improve the performance. but I have to revert it because it is a breaking change for existing users. suppose they were using it without a rate limiter for large messages. it will let many messages stuck in the Tokio task queue and some tasks get timed.

[mattisonchao]

@cirias @tisonkun please take a look. thanks!

[cirias]

suppose they were using it without a rate limiter for large messages. it will let many messages stuck in the Tokio task queue and some tasks get timed.

We've been seeing this issue as well. But instead of reverting the change, we should probably change to use bounded channels instead of unbounded ones. The root cause of messages accumulating in the channel is because the throughput isn't enough for the high rate of the messages. The bottleneck could be either server or network, with network being the most likely case. Revert the change disables the ability to fully utilize the network bandwidth, which is probably not a right direction. Pulsar client should tell user to slow down when the bandwidth is not enough. If we change to use bounded channel, and return an error for send when the channel is full, then user can handle this error by simply retry after some delay. It will be a breaking change but should be better than revert. I could take the effort creating a PR, if people agree.

[mattisonchao]

We've been seeing this issue as well. But instead of reverting the change, we should probably change to use bounded channels instead of unbounded ones. The root cause of messages accumulating in the channel is because the throughput isn't enough for the high rate of the messages. The bottleneck could be either server or network, with network being the most likely case. Revert the change disables the ability to fully utilize the network bandwidth, which is probably not a right direction. Pulsar client should tell user to slow down when the bandwidth is not enough. If we change to use bounded channel, and return an error for send when the channel is full, then user can handle this error by simply retry after some delay. It will be a breaking change but should be better than revert. I could take the effort creating a PR, if people agree.

maybe we can introduce a new API or option for this improvement. the default should be the blocking call. :)

Plus, we should consider a health check here. I saw they are using the same channel to send messages to the pulsar server. the client health check will fail if the channel is crowded.

[cirias]

We've been seeing this issue as well. But instead of reverting the change, we should probably change to use bounded channels instead of unbounded ones. The root cause of messages accumulating in the channel is because the throughput isn't enough for the high rate of the messages. The bottleneck could be either server or network, with network being the most likely case. Revert the change disables the ability to fully utilize the network bandwidth, which is probably not a right direction. Pulsar client should tell user to slow down when the bandwidth is not enough. If we change to use bounded channel, and return an error for send when the channel is full, then user can handle this error by simply retry after some delay. It will be a breaking change but should be better than revert. I could take the effort creating a PR, if people agree.

maybe we can introduce a new API or option for this improvement. the default should be the blocking call. :)

Plus, we should consider a health check here. I saw they are using the same channel to send messages to the pulsar server. the client health check will fail if the server is crowded.

Makes sense. It would be easier to have the blocking function based on the non-blocking one. I will make a PR.