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

Enforce a size limit on journaled events #33

Open
samandmoore opened this issue Nov 16, 2023 · 4 comments
Open

Enforce a size limit on journaled events #33

samandmoore opened this issue Nov 16, 2023 · 4 comments

Comments

@samandmoore
Copy link
Member

Background
In some cases, we have messages being pulled off streams, augmented with additional data, and then plopped back onto another stream. If the message we initially put on the stream is too large (at or too close to the max message size) then we run the risk of breaking stream consumers ability to minimally augment our events and add them to their own streams.

Proposal
We should introduce a size limit for the events to ensure that we don't come within some reasonable threshold of the size limit of a validate message for AWS kinesis streams.

Questions

  1. what should the limit be? perhaps 90% of the max message size?
  2. how should we enforce this limit?
  • reject events that are too large? probably bad dev ex since the size of the message is a function of external factors
  • trim fields that are too large?
@jmileham
Copy link
Member

I could buy something even more extreme/opinionatedly small max message size, given the intended use of this tool - the max kinesis message size is 1mb, which is way larger than the kinds of payloads this would be expected to use by default. I might even go down to something closer to 4kb if data analysis indicates that's appropriate?

@smudge
Copy link
Member

smudge commented Nov 16, 2023

Trimming fields could maybe work for specific use cases, but I don't think it's a good idea for the core gem (which I see as the unopinionated delivery mechanism) to mutate underlying data. Better to leave that to the next layer of abstraction. So as long as it's not a regular occurrence, better to halt delivery and leave an erroring background job to clean up.

(This is where tuning the size cutoff is important. I can pull a distribution of our production payload sizes that could inform a default value for the gem.)

@jmileham
Copy link
Member

+1 for reject, especially in the DJ background where you can maybe override the max by a bit and unwedge a job.

@samandmoore
Copy link
Member Author

yea, i'm in favor of accepting the job but rejecting the event at stream enqueue time so that exceptional events can be handled easily and we don't break the end user experience.

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

No branches or pull requests

3 participants