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

feat: fix record size larger than batch size #4195

Merged
merged 6 commits into from
Oct 15, 2024

Conversation

fraidev
Copy link
Contributor

@fraidev fraidev commented Sep 26, 2024

I propose to instead return an error when a record is bigger than the batch size, create a new batch with this single record without batch size limit.

I also propose adding a config variable called max_request_size.

Also, I removed the compression_coefficient been used to calculate the batch_size, because I believe that compression should not be related with the batch_size.

Should also fix: #4161

@fraidev fraidev force-pushed the fix_batch_size_2 branch 4 times, most recently from 5ac1aa8 to 98b8ee1 Compare September 26, 2024 21:07
@fraidev fraidev changed the title Fix batch size 2 feat: fix record size bigger than batch size Sep 26, 2024
@fraidev fraidev force-pushed the fix_batch_size_2 branch 5 times, most recently from c389a59 to 86b114e Compare September 26, 2024 22:51
@fraidev fraidev requested a review from digikata September 26, 2024 23:00
@fraidev fraidev marked this pull request as ready for review September 26, 2024 23:00
@fraidev fraidev force-pushed the fix_batch_size_2 branch 3 times, most recently from caa9179 to e8479a3 Compare September 26, 2024 23:25
@fraidev fraidev force-pushed the fix_batch_size_2 branch 3 times, most recently from f1bf7b2 to f53eeee Compare September 27, 2024 14:24
@sehz
Copy link
Contributor

sehz commented Sep 27, 2024

should link PR to issue and more detail explanation how should work

@fraidev
Copy link
Contributor Author

fraidev commented Sep 27, 2024

should link PR to issue and more detail explanation how should work

Sure! Should fix #4161

@fraidev fraidev force-pushed the fix_batch_size_2 branch 2 times, most recently from 7a39f30 to 97c1bc1 Compare September 29, 2024 21:23
@fraidev fraidev requested a review from digikata October 3, 2024 14:54
@fraidev fraidev force-pushed the fix_batch_size_2 branch 5 times, most recently from be1be20 to 04e144d Compare October 14, 2024 22:09
@fraidev fraidev force-pushed the fix_batch_size_2 branch 5 times, most recently from 678a41f to c1f6e4e Compare October 15, 2024 02:44
crates/fluvio-storage/src/replica.rs Show resolved Hide resolved
crates/fluvio/src/producer/accumulator.rs Outdated Show resolved Hide resolved
crates/fluvio/src/producer/memory_batch.rs Show resolved Hide resolved
crates/fluvio/src/producer/memory_batch.rs Outdated Show resolved Hide resolved
crates/fluvio/src/producer/memory_batch.rs Outdated Show resolved Hide resolved
crates/fluvio/src/producer/memory_batch.rs Show resolved Hide resolved
@fraidev fraidev requested a review from sehz October 15, 2024 17:40
Copy link
Contributor

@sehz sehz left a comment

Choose a reason for hiding this comment

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

Nice Job. LGTM

@fraidev fraidev added this pull request to the merge queue Oct 15, 2024
Merged via the queue into infinyon:master with commit 5e1f885 Oct 15, 2024
106 checks passed
@fraidev fraidev deleted the fix_batch_size_2 branch October 15, 2024 23:49
ajhunyady pushed a commit to infinyon/fluvio-docs that referenced this pull request Oct 22, 2024
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

Successfully merging this pull request may close these issues.

Checking for max batch size ignores compression
3 participants