nzw921rx commented on PR #10954: URL: https://github.com/apache/seatunnel/pull/10954#issuecomment-4543860130
> Thanks for the contribution. I reviewed the full diff on the current head and traced where this option is consumed on the normal Kafka source path. > > # What this PR fixes > * User pain: very large reader-side poll buffers can amplify memory pressure on the Kafka source path. > * Fix approach: reduce the default `reader_cache_queue_size` from 1024 to 2 and clarify the option description. > * One-line summary: the memory concern is real, but the current patch changes a stable default contract on the main path without handling compatibility expectations. > > # Runtime chain I checked > ``` > Kafka source config > -> KafkaSourceOptions.READER_CACHE_QUEUE_SIZE [KafkaSourceOptions.java:44-51] > -> reader / fetcher queue capacity > -> affects the normal poll-buffering behavior for existing jobs that do not override the option > ``` > > # Findings > Issue 1: this changes a stable default contract for existing jobs > > * Location: `KafkaSourceOptions.java:44-51` > * Problem: the default changes from `1024` to `2` on the normal user path when the option is omitted. > * Risk: this can materially change buffering, throughput, backpressure, and poll behavior for already deployed jobs even if users never touched the config. > * Better fix: either keep the old default and introduce a new safer opt-in path, or document this as an incompatible behavior change with explicit migration guidance. > * Severity: High > > # Merge conclusion > ### Conclusion: can merge after fixes > 1. Blocking items > > * Issue 1: the default-value compatibility story needs to be handled explicitly before merge. > > 2. Suggested non-blocking follow-up > > * If the team wants a safer out-of-box profile later, a connector-level recommendation in docs would be a better first step than silently changing the existing default. > > Overall, I agree with the memory-pressure motivation, but the current patch is not ready to merge as a silent default flip. No need to add new parameters, the default behavior of this parameter will make users perceive inexplicable OOM, which is safer than throughput runtime. Users can explicitly adjust this parameter -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
