Skip to content
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

Any reason to set as minimum batch publishing delay 50msec? #332

Closed
chicco785 opened this issue Jul 18, 2024 · 7 comments
Closed

Any reason to set as minimum batch publishing delay 50msec? #332

chicco785 opened this issue Jul 18, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@chicco785
Copy link

Is your feature request related to a problem? Please describe.

minBatchPublishingDelay = 50

in our use case the application work at 20msec frequency, so the idea batch delay should be around that. we can play with batch size, but when some sensors are unreliable, this pops up again, and we need to reconfigure the batch size. setting to 20msec or (16 in the us market), would decrease a lot the lanency for real time processing.

Describe the solution you'd like

if possible, set a batch delay below 50msec.

Describe alternatives you've considered

No response

Additional context

No response

@chicco785 chicco785 added the enhancement New feature or request label Jul 18, 2024
@Gsantomaggio
Copy link
Member

@chicco785 there is not a specific reason. Feel free to prepare a PR to reduce the value. I'd say that we can drop down to 10 ms . Thank you.

btw there is always the BatchSend to give you more control, see: https://github.com/rabbitmq/rabbitmq-stream-go-client?tab=readme-ov-file#send-vs-batchsend

@chicco785
Copy link
Author

@hiimjako I don't recall if there is a reason why we picked send over batchsend

@Gsantomaggio
Copy link
Member

if you work with @hiimjako I know why, because HA part does not support (yet) the bach send, but only send. :)

https://github.com/rabbitmq/rabbitmq-stream-go-client/blob/main/pkg/ha/ha_publisher.go#L112

@hiimjako
Copy link
Collaborator

if you work with @hiimjako I know why, because HA part does not support (yet) the bach send, but only send. :)

https://github.com/rabbitmq/rabbitmq-stream-go-client/blob/main/pkg/ha/ha_publisher.go#L112

Exactly for this. I think it would be great to have both:

  1. Reduced trashold, since the Send function has all the logic needed for batch handling.
  2. Expose the BatchSend to give more control to those who want to implement different batch handling.

For the first point, why not just do the check that the minBatchPublishingDelay is not less than 1 ms?
It will obviously stress the client with lower values, but everyone will have the freedom to set it to the value that works best for them. What do you think?

@Gsantomaggio
Copy link
Member

Reduced trashold, since the Send function has all the logic needed for batch handling.
Expose the BatchSend to give more control to those who want to implement different batch handling.

I don't have space now but feel free to propose a PR.

For the first point, why not just do the check that the minBatchPublishingDelay is not less than 1 ms?

Ok.

@hiimjako
Copy link
Collaborator

I don't have space now but feel free to propose a PR.

I will do it these days 👍

@chicco785
Copy link
Author

fixed by #333

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants