[Enhancement] limit tablet write request size #50302

silverbullet233 · 2024-08-27T05:32:12Z

Why I'm doing:

When there are large chunks during the load process, the following error may be returned due to exceeding the size limit of protobuf.

NodeChannel currently only aggregates batches based on the function of the chunk, but does not take into account the size of the chunk.

What I'm doing:

In this PR, I have added chunk mem usage check to avoid generating large pb requests for load tasks

Fixes #issue

What type of PR is this:

Does this PR entail a change in behavior?

Yes, this PR will result in a change in behavior.
No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

Interface/UI changes: syntax, type conversion, expression evaluation, display information
Parameter changes: default values, similar parameters but with different default values
Policy changes: use new policy to replace old one, functionality automatically enabled
Feature removed
Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

I have added test cases for my bug fix or my new feature
This pr needs user documentation (for new or modified features or behaviors)
- I have added documentation for my new feature or new function
This is a backport pr

Bugfix cherry-pick branch check:

be/src/common/config.h

be/src/runtime/current_thread.h

be/src/exec/tablet_sink_index_channel.cpp

Signed-off-by: silverbullet233 <[email protected]>

trueeyu · 2024-09-18T11:53:56Z

be/src/exec/tablet_sink_index_channel.cpp

        auto req = _rpc_request.mutable_requests(0);
        for (size_t i = 0; i < filter_size; ++i) {
            req->add_tablet_ids(tablet_ids[filtered_indexes[from + i]]);
        }
    }

-    if (_cur_chunk->num_rows() < _runtime_state->chunk_size()) {
+    if (_cur_chunk->num_rows() < _runtime_state->chunk_size() &&
+        _cur_chunk_mem_usage < config::max_tablet_write_chunk_bytes) {


_cur_chunk->num_rows() <= 0 || (_cur_chunk->num_rows() < _runtime_state->chunk_size() && _cur_chunk_mem_usage < config::max_tablet_write_chunk_bytes) maybe better

clone_empty_with_slot maybe reserve some memory, there maybe no one row in chunk, but memory exceed the limit.

Signed-off-by: silverbullet233 <[email protected]>

be/src/exec/tablet_sink_index_channel.cpp

Signed-off-by: silverbullet233 <[email protected]>

chaoyli · 2024-09-19T03:00:43Z

be/src/common/config.h

+// NOTE: If there are a large number of columns when loading,
+// a too small max_tablet_write_chunk_bytes may cause more frequent RPCs, which may affect performance.
+// In this case, we can try to increase the value to avoid the problem.
+CONF_mInt64(max_tablet_write_chunk_bytes, "536870912");


Why not keep the same size of protobuf limit?

Why not keep the same size of protobuf limit?

Because this is not a hard limit, the chunk may exceed it. If it is set to 2GB, serialization will still fail once it exceeds the limit. 512MB is large enough for most scenarios.

github-actions · 2024-09-19T11:12:08Z

[Java-Extensions Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

github-actions · 2024-09-19T11:12:15Z

[FE Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

github-actions · 2024-09-19T11:13:02Z

[BE Incremental Coverage Report]

❌ fail : 0 / 24 (00.00%)

file detail

	path	covered_line	new_line	coverage	not_covered_line_detail
🔵	be/src/exec/tablet_sink_index_channel.cpp	0	24	00.00%	[418, 419, 420, 421, 422, 423, 425, 426, 427, 428, 429, 430, 440, 453, 462, 470, 471, 481, 502, 514, 523, 524, 534, 589]

github-actions · 2024-09-19T11:18:30Z

@Mergifyio backport branch-3.3

github-actions · 2024-09-19T11:18:31Z

@Mergifyio backport branch-3.2

mergify · 2024-09-19T11:18:37Z

backport branch-3.3

✅ Backports have been created

#51172 [Enhancement] limit tablet write request size (backport #50302) has been created for branch branch-3.3

mergify · 2024-09-19T11:18:40Z

backport branch-3.2

✅ Backports have been created

#51173 [Enhancement] limit tablet write request size (backport #50302) has been created for branch branch-3.2

Signed-off-by: silverbullet233 <[email protected]> (cherry picked from commit ce25287)

Co-authored-by: eyes_on_me <[email protected]>

dengliu · 2024-09-19T23:13:11Z

Can we backport it to 3.2?
Thank you!

silverbullet233 · 2024-09-20T10:34:38Z

Can we backport it to 3.2? Thank you!

I think so, you can cherry-pick #51173 @dengliu

Signed-off-by: silverbullet233 <[email protected]> Signed-off-by: zhiminr.ren <[email protected]>

silverbullet233 requested review from a team as code owners August 27, 2024 05:32

mergify bot assigned silverbullet233 Aug 27, 2024

silverbullet233 requested a review from a team as a code owner August 27, 2024 05:41

github-actions bot added the 3.3 label Aug 27, 2024

silverbullet233 requested review from meegoo and luohaha August 27, 2024 06:52

silverbullet233 commented Aug 27, 2024

View reviewed changes

be/src/common/config.h Outdated Show resolved Hide resolved

trueeyu reviewed Aug 27, 2024

View reviewed changes

be/src/runtime/current_thread.h Outdated Show resolved Hide resolved

luohaha previously approved these changes Sep 13, 2024

View reviewed changes

luohaha reviewed Sep 13, 2024

View reviewed changes

be/src/exec/tablet_sink_index_channel.cpp Outdated Show resolved Hide resolved

luohaha reviewed Sep 13, 2024

View reviewed changes

be/src/exec/tablet_sink_index_channel.cpp Outdated Show resolved Hide resolved

silverbullet233 added 3 commits September 18, 2024 09:11

limit tablet write request size

a8fd544

Signed-off-by: silverbullet233 <[email protected]>

fix format

aa0740a

Signed-off-by: silverbullet233 <[email protected]>

change default max_tablet_write_chunk_bytes to 128MB

5717597

Signed-off-by: silverbullet233 <[email protected]>

silverbullet233 force-pushed the limit_tablet_sink_request_size branch from 9862259 to 5717597 Compare September 18, 2024 01:11

fix comments

3ca485f

Signed-off-by: silverbullet233 <[email protected]>

silverbullet233 dismissed luohaha’s stale review via 3ca485f September 18, 2024 10:47

trueeyu reviewed Sep 18, 2024

View reviewed changes

fix comments

580fa07

Signed-off-by: silverbullet233 <[email protected]>

silverbullet233 force-pushed the limit_tablet_sink_request_size branch from 097add0 to 580fa07 Compare September 18, 2024 13:29

luohaha previously approved these changes Sep 18, 2024

View reviewed changes

github-actions bot added the 3.2 label Sep 18, 2024

trueeyu reviewed Sep 19, 2024

View reviewed changes

be/src/exec/tablet_sink_index_channel.cpp Show resolved Hide resolved

trueeyu previously approved these changes Sep 19, 2024

View reviewed changes

silverbullet233 enabled auto-merge (squash) September 19, 2024 02:08

add comments

a53729f

Signed-off-by: silverbullet233 <[email protected]>

silverbullet233 dismissed stale reviews from trueeyu and luohaha via a53729f September 19, 2024 02:27

trueeyu approved these changes Sep 19, 2024

View reviewed changes

chaoyli reviewed Sep 19, 2024

View reviewed changes

luohaha approved these changes Sep 19, 2024

View reviewed changes

wyb approved these changes Sep 19, 2024

View reviewed changes

dirtysalt approved these changes Sep 19, 2024

View reviewed changes

silverbullet233 merged commit ce25287 into StarRocks:main Sep 19, 2024
55 of 56 checks passed

silverbullet233 deleted the limit_tablet_sink_request_size branch September 19, 2024 11:18

github-actions bot removed 3.3 3.2 labels Sep 19, 2024

mergify bot pushed a commit that referenced this pull request Sep 19, 2024

[Enhancement] limit tablet write request size (#50302)

739852a

Signed-off-by: silverbullet233 <[email protected]> (cherry picked from commit ce25287)

mergify bot mentioned this pull request Sep 19, 2024

[Enhancement] limit tablet write request size (backport #50302) #51172

Merged

42 tasks

mergify bot pushed a commit that referenced this pull request Sep 19, 2024

[Enhancement] limit tablet write request size (#50302)

684f15e

Signed-off-by: silverbullet233 <[email protected]> (cherry picked from commit ce25287)

mergify bot mentioned this pull request Sep 19, 2024

[Enhancement] limit tablet write request size (backport #50302) #51173

Merged

42 tasks

wanpengfei-git pushed a commit that referenced this pull request Sep 19, 2024

[Enhancement] limit tablet write request size (backport #50302) (#51173)

2ad8771

Co-authored-by: eyes_on_me <[email protected]>

github-actions bot added the 3.2-merged label Sep 19, 2024

wanpengfei-git pushed a commit that referenced this pull request Sep 19, 2024

[Enhancement] limit tablet write request size (backport #50302) (#51172)

38e2009

Co-authored-by: eyes_on_me <[email protected]>

github-actions bot added the 3.3-merged label Sep 19, 2024

renzhimin7 pushed a commit to renzhimin7/starrocks that referenced this pull request Nov 7, 2024

[Enhancement] limit tablet write request size (StarRocks#50302)

9a665c6

Signed-off-by: silverbullet233 <[email protected]> Signed-off-by: zhiminr.ren <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Enhancement] limit tablet write request size #50302

[Enhancement] limit tablet write request size #50302

silverbullet233 commented Aug 27, 2024 •

edited by luohaha

Loading

trueeyu Sep 18, 2024

trueeyu Sep 18, 2024

chaoyli Sep 19, 2024

silverbullet233 Sep 19, 2024

github-actions bot commented Sep 19, 2024

github-actions bot commented Sep 19, 2024

github-actions bot commented Sep 19, 2024

github-actions bot commented Sep 19, 2024

github-actions bot commented Sep 19, 2024

mergify bot commented Sep 19, 2024 •

edited

Loading

mergify bot commented Sep 19, 2024 •

edited

Loading

dengliu commented Sep 19, 2024

silverbullet233 commented Sep 20, 2024

[Enhancement] limit tablet write request size #50302

[Enhancement] limit tablet write request size #50302

Conversation

silverbullet233 commented Aug 27, 2024 • edited by luohaha Loading

Why I'm doing:

What I'm doing:

What type of PR is this:

Checklist:

Bugfix cherry-pick branch check:

trueeyu Sep 18, 2024

Choose a reason for hiding this comment

trueeyu Sep 18, 2024

Choose a reason for hiding this comment

chaoyli Sep 19, 2024

Choose a reason for hiding this comment

silverbullet233 Sep 19, 2024

Choose a reason for hiding this comment

github-actions bot commented Sep 19, 2024

[Java-Extensions Incremental Coverage Report]

github-actions bot commented Sep 19, 2024

[FE Incremental Coverage Report]

github-actions bot commented Sep 19, 2024

[BE Incremental Coverage Report]

file detail

github-actions bot commented Sep 19, 2024

github-actions bot commented Sep 19, 2024

mergify bot commented Sep 19, 2024 • edited Loading

✅ Backports have been created

mergify bot commented Sep 19, 2024 • edited Loading

✅ Backports have been created

dengliu commented Sep 19, 2024

silverbullet233 commented Sep 20, 2024

silverbullet233 commented Aug 27, 2024 •

edited by luohaha

Loading

mergify bot commented Sep 19, 2024 •

edited

Loading

mergify bot commented Sep 19, 2024 •

edited

Loading