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

fix: kafka.errors.MessageSizeTooLargeError #19634

Closed
wants to merge 1 commit into from
Closed

Conversation

jk2K
Copy link

@jk2K jk2K commented Jan 6, 2024

Problem

fix #19633

solution

  1. kafka producer config: max.request.size set to 20971520
  2. kafka server config: message.max.bytes set to 20971520
  3. posthog kafka client, SESSION_RECORDING_KAFKA_MAX_REQUEST_SIZE_BYTES set to 20971520

related to bitnami/containers#54458

another thing

  1. put Caddy's data into caddy-data docker volume, I use Local HTTPS, Caddy will generate local CA, when recreating the container, avoid reinstalling Caddy's root CA on my host machine

@Twixes Twixes requested a review from a team January 8, 2024 10:23
@marandaneto marandaneto requested a review from a team January 8, 2024 13:42
Copy link
Contributor

@benjackwhite benjackwhite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I see no issue with it, especially given the default docker compose should support the higher message size 🤔

@pauldambra
Copy link
Member

@benjackwhite only thing I can think is that maybe it should be configurable... 2MB works for this user... 1MB has worked previously without reports... We really should have better visibility on how big these messages are and what users can do about it 🤔

@benjackwhite
Copy link
Contributor

@benjackwhite only thing I can think is that maybe it should be configurable... 2MB works for this user... 1MB has worked previously without reports... We really should have better visibility on how big these messages are and what users can do about it 🤔

Given this is the hobby deploy and the kafka config only affects this local kafka, I think it's still pretty safe.
If they have externalised kafka then none of this really applies anyways and they can still override the env var so 🤷

Copy link
Contributor

@xvello xvello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not recommend changing the defaults for the hobby deploy:

  • as Paul has mentioned, 2MiB would not fix it for everyone anyway, and increasing the Kafka message size too much comes with operational challenges on the Kafka side that I do not wish to investigate
  • the analytics pipeline assumes a 1MiB limit and does not check sizes on the producer size. It probably should, but in the meantime increasing this default introduces the risk of a PR working on local devenv and CI (with 2MiB message limit), then failing on prod where we use 1MiB. I don't want to take that risk
  • lastly, our compose stack does not use the bitnami image anymore, but https://github.com/PostHog/kafka-container, so this PR would not use the linked PR

I don't oppose making it configurable, but I strongly suggest to keep the default at 1MiB.

@posthog-bot
Copy link
Contributor

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

@posthog-bot
Copy link
Contributor

This PR was closed due to lack of activity. Feel free to reopen if it's still relevant.

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