chore: replace max_io_request

[JackTan25]

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

Summary about this PR
we need to use thread_nums to limit the permit_nums , in some cases, if the permit num is too large, we will get oom.

• Closes #issue

---

This change is 

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
databend	⬜️ Ignored (Inspect)	Visit Preview		Sep 25, 2023 8:36am

[JackTan25]

use thread_nums as permit_nums to control the concurrency. Do we need to use try_join_all directly without permits? or use thread_nums * 4 as the permit_nums? cc @dantengsky @zhyass

[github-actions]

Docker Image for PR

• tag: pr-12997-5e39b00

note: this image tag is only available for internal use,
please check the internal doc for more details.

[github-actions]

Docker Image for PR

• tag: pr-12997-cb1815e

note: this image tag is only available for internal use,
please check the internal doc for more details.

[zhang2014]

maybe we should add new settings is better, with default values same to max_threads

[JackTan25]

maybe we should add new settings is better, with default values same to max_threads

I'm not sure whether we need the factor * thread_nums or just use try_join_all()

[github-actions]

ClickBench Report

• hits: https://benchmark.databend.rs/clickbench/pr/12997/6295413174/hits.html
• tpch: https://benchmark.databend.rs/clickbench/pr/12997/6295413174/tpch.html

[zhang2014]

maybe we should add new settings is better, with default values same to max_threads

I'm not sure whether we need the factor * thread_nums or just use try_join_all()

it may be necessary when we need to adjust its parallel？

[BohuTANG]

maybe we should add new settings is better, with default values same to max_threads

Looks we don't need the new setting, only adjust a factor with CPU is fine.

[dantengsky]

ci-benchmark

#12997 (comment)

shows no performance degradations, which is expected, since all the modifications are table mutation-related only (If I get it right).

but scenarios the ci-benchmark not covered, e.g. deletions/compactions/replace-into, etc, we do not know the impacts yet.

Maybe too conservative, but I am with @zhang2014 , that we may need to introduce a new setting(let's say, max_mutation_io_request or a better name), with a default value that equals the values of max_stroage_io_request, but can be tweaked individually.

[JackTan25]

ci-benchmark

#12997 (comment)

shows no performance degradations, which is expected, since all the modifications are table mutation-related only (If I get it right).

but scenarios the ci-benchmark not covered, e.g. deletions/compactions/replace-into, etc, we do not know the impacts yet.

Maybe too conservative, but I am with @zhang2014 , that we may need to introduce a new setting(let's say, max_mutation_io_request or a better name), with a default value that equals the values of max_stroage_io_request, but can be tweaked individually.

I agree with that. When we set max_stroage_io_request, we will also set max_mutation_io_request, but we can also set max_mutation_io_request individually. And no need factor, just use max_mutation_io_request as the permit_nums in execute_futures_in_parallel. cc @dantengsky @zhang2014 @BohuTANG @zhyass can we be consistent with this?

[BohuTANG]

ci-benchmark
#12997 (comment)
shows no performance degradations, which is expected, since all the modifications are table mutation-related only (If I get it right).
but scenarios the ci-benchmark not covered, e.g. deletions/compactions/replace-into, etc, we do not know the impacts yet.
Maybe too conservative, but I am with @zhang2014 , that we may need to introduce a new setting(let's say, max_mutation_io_request or a better name), with a default value that equals the values of max_stroage_io_request, but can be tweaked individually.

I agree with that. When we set max_stroage_io_request, we will also set max_mutation_io_request, but we can also set max_mutation_io_request individually. And no need factor, just use max_mutation_io_request as the permit_nums in execute_futures_in_parallel. cc @dantengsky @zhang2014 @BohuTANG @zhyass can we be consistent with this?

Hmm...
This means that we do not do the CPU*fatctor right, depends another setting max_mutation_io_request .
This PR aims to make less settings.

[JackTan25]

@zhyass Any concerns? shall we let the permit_nums eq scale_factor * threads_nums, where scale_factor = 4, in this case (reclustering/compaction) for example?

Set scale_factor * threads_nums, if use execute_futures_in_parallel. Otherwise use threads_nums directly.
use @zhyass 's suggestion.