Return multiple errors from Writer.WriteMessages and MessageTooLargeError handling improvements #1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ct error channel to be retried.
Add id field so that a given message can be tracked back to its index when it was originally sent from Writer.WriteMessages. id allows WriteMessages to keep track of which messages have been successfully sent therefore making it easier to do more sophisticated error handling.
Change res channel type in writerMessage to new writerResponse struct type so ids can be returned back to writer.WriteMessages. Refactor affected receivers.
Implement WriterErrors type which implements the error interface to allow for the returning of multiple WriterErrors that may occur during a write. Provide basic Error() method which takes inspiration from https://github.com/hashicorp/go-multierror/blob/72917a1559e17f38638ade54020ab372ba848d67/format.go#L14 so the slice of WriterError is formatted in a nice fashion.
error is a bit too general for dependent code. All err values are WriterError anyway so make it the more specific type.
… before returning MessageTooLarge errors Utilize WriterErrors and WriterError types to return all errors that occur when sending a batch of messages using Writer.WriteMessages while still preserving error return type. Try to send all eligible messages in msgs before returning any MessageTooLarge errors.
Evanjt1
changed the title
|
Putting this here for visibility. Copy of upstream segment repo PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses issues 363 and 364
This PR revolves around 2 main improvements for the Writer type:
Return information about all errors that occurred during a write of multiple messages while still preserving interface of Writer.WriteMessages
Try to send all acceptable messages in a batch before returning the ones that are too large
I introduced two new public types, WriterError and WriterErrors to implement the first improvement. Both implement the error interface. Under the hood Writer.WriteMessages returns WriterErrors. The caller can cast the error returned as WriterErrors to use the new functionality if they so choose.
The second improvement is addressed by skipping messages that are too large as the Writer comes across them while sending to the msgs channel. They are instead stored as errors that will be returned after all the other messages have been tried.
New tests have been added to writer_test.go to ensure these features behave as expected. Additionally, some of the preexisting tests have been updated so they also check the WriterErrors returned.
There would have been a bunch of merge conflicts and wasted work if I tried to made both improvements individually so I decided to knock out two birds with one stone.