Skip to content

Try to send entire batch before returning any MessageTooLargeErrors in Writer.WriteMessages #364

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

Closed
Evanjt1 opened this issue Oct 16, 2019 · 4 comments
Assignees

Comments

@Evanjt1
Copy link

Evanjt1 commented Oct 16, 2019

Currently, Writer.WriteMessages returns early if it encounters a message that is too large:

err := MessageTooLargeError{

Since messages may have already been added to w.msgs before encountering the MessageTooLargeError, wouldn't it be better to try and submit all messages in the batch before returning this error? I have a feeling that the early return is due to only one error being returned from a WriteMessages call so ensuring this error gets returned allows the caller to handle appropriately.

Is there some other edge case that I'm missing here for why the receiver returns early when encountering this issue?

Linking #363 since it's related to the singular return from WriteMessages

Obligating myself to implement this, if needed.

@achille-roussel
Copy link
Contributor

Hey @Evanjt1, thanks for raising the question.

You're right that the current behavior isn't ideal, I think we should either attempt to send as many messages as possible (all messages that aren't too large), or do a check prior to sending the messages and either send them all or send nothing.

@stevevls you may have an opinion on this as well.

@achille-roussel achille-roussel self-assigned this Oct 16, 2019
@Evanjt1
Copy link
Author

Evanjt1 commented Oct 16, 2019

@achille-roussel I typically lean towards processing as much as possible and rejecting only the malformed/too large messages rather than failing out the entire batch. I perused a few clients for other systems that have similar batch send, and that seems to be the preferred way to go.

@achille-roussel
Copy link
Contributor

This issue has been stale for a while. I'll go ahead and close it but feel free to reopen if this is still a problem.

@stevevls
Copy link
Contributor

stevevls commented Oct 8, 2021

Oops...I didn't realize I was flagged on this issue because I was on paternity leave last October. 😅 Anyhow, I think that some clients may be sensitive to message order within a partition, so the safest option is the current behavior. If we wanted to enable sending all the messages in a single batch, then I would opt for a configuration option to govern the behavior. Just putting my thoughts down here in case we reopen the issue down the line. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants