You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "hengfeiyang (via GitHub)" <gi...@apache.org> on 2023/09/19 05:00:07 UTC
[GitHub] [arrow-datafusion] hengfeiyang opened a new pull request, #7595: feat: Parallel collecting parquet files statistics #7573
hengfeiyang opened a new pull request, #7595:
URL: https://github.com/apache/arrow-datafusion/pull/7595
## Which issue does this PR close?
Implement #7573
## Rationale for this change
## What changes are included in this PR?
- [ ] Add option `execution.meta_fetch_concurrency` default is `CPU::num()`
- [ ] Replace `SCHEMA_INFERENCE_CONCURRENCY` with option `meta_fetch_concurrency`
- [ ] Implement parallel collecting parquet files statistics
## Are these changes tested?
In my local to search for 60 parquet files from s3. Because of parallel collecting statistics, the search speed improved 30%, of course, my local network request s3 has high latency.
## Are there any user-facing changes?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] hengfeiyang commented on a diff in pull request #7595: feat: Parallel collecting parquet files statistics #7573
Posted by "hengfeiyang (via GitHub)" <gi...@apache.org>.
hengfeiyang commented on code in PR #7595:
URL: https://github.com/apache/arrow-datafusion/pull/7595#discussion_r1332441371
##########
datafusion/common/src/config.rs:
##########
@@ -251,6 +251,11 @@ config_namespace! {
/// and sorted in a single RecordBatch rather than sorted in
/// batches and merged.
pub sort_in_place_threshold_bytes: usize, default = 1024 * 1024
+
+ /// Number of files to read in parallel when inferring schema and statistics
+ ///
+ /// Defaults to the number of CPU cores on the system
+ pub meta_fetch_concurrency: usize, default = num_cpus::get()
Review Comment:
@alamb Done.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] hengfeiyang commented on a diff in pull request #7595: feat: Parallel collecting parquet files statistics #7573
Posted by "hengfeiyang (via GitHub)" <gi...@apache.org>.
hengfeiyang commented on code in PR #7595:
URL: https://github.com/apache/arrow-datafusion/pull/7595#discussion_r1332382763
##########
datafusion/common/src/config.rs:
##########
@@ -251,6 +251,11 @@ config_namespace! {
/// and sorted in a single RecordBatch rather than sorted in
/// batches and merged.
pub sort_in_place_threshold_bytes: usize, default = 1024 * 1024
+
+ /// Number of files to read in parallel when inferring schema and statistics
+ ///
+ /// Defaults to the number of CPU cores on the system
+ pub meta_fetch_concurrency: usize, default = num_cpus::get()
Review Comment:
i will change it to `32`. this should be better. let me do that.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #7595: feat: Parallel collecting parquet files statistics #7573
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #7595:
URL: https://github.com/apache/arrow-datafusion/pull/7595#discussion_r1329962590
##########
docs/source/user-guide/configs.md:
##########
@@ -76,6 +76,7 @@ Environment variables are read during `SessionConfig` initialisation so they mus
| datafusion.execution.planning_concurrency | 0 | Fan-out during initial physical planning. This is mostly use to plan `UNION` children in parallel. Defaults to the number of CPU cores on the system |
| datafusion.execution.sort_spill_reservation_bytes | 10485760 | Specifies the reserved memory for each spillable sort operation to facilitate an in-memory merge. When a sort operation spills to disk, the in-memory data must be sorted and merged before being written to a file. This setting reserves a specific amount of memory for that in-memory sort/merge process. Note: This setting is irrelevant if the sort operation cannot spill (i.e., if there's no `DiskManager` configured). |
| datafusion.execution.sort_in_place_threshold_bytes | 1048576 | When sorting, below what size should data be concatenated and sorted in a single RecordBatch rather than sorted in batches and merged. |
+| datafusion.execution.meta_fetch_concurrency | 0 | Number of files to read in parallel when inferring schema and statistics Defaults to the number of CPU cores on the system |
Review Comment:
The default value of `0` seems strange here, but it looks like a bug with the script that makes this page, given the same issue appears there:
```
| datafusion.execution.planning_concurrency | 0 | Fan-out during initial physical planning. This is mostly use to plan `UNION` children in parallel. Defaults to the number of CPU cores on the system
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] hengfeiyang commented on a diff in pull request #7595: feat: Parallel collecting parquet files statistics #7573
Posted by "hengfeiyang (via GitHub)" <gi...@apache.org>.
hengfeiyang commented on code in PR #7595:
URL: https://github.com/apache/arrow-datafusion/pull/7595#discussion_r1329975067
##########
docs/source/user-guide/configs.md:
##########
@@ -76,6 +76,7 @@ Environment variables are read during `SessionConfig` initialisation so they mus
| datafusion.execution.planning_concurrency | 0 | Fan-out during initial physical planning. This is mostly use to plan `UNION` children in parallel. Defaults to the number of CPU cores on the system |
| datafusion.execution.sort_spill_reservation_bytes | 10485760 | Specifies the reserved memory for each spillable sort operation to facilitate an in-memory merge. When a sort operation spills to disk, the in-memory data must be sorted and merged before being written to a file. This setting reserves a specific amount of memory for that in-memory sort/merge process. Note: This setting is irrelevant if the sort operation cannot spill (i.e., if there's no `DiskManager` configured). |
| datafusion.execution.sort_in_place_threshold_bytes | 1048576 | When sorting, below what size should data be concatenated and sorted in a single RecordBatch rather than sorted in batches and merged. |
+| datafusion.execution.meta_fetch_concurrency | 0 | Number of files to read in parallel when inferring schema and statistics Defaults to the number of CPU cores on the system |
Review Comment:
Yes, the default is `cpu::num()`, so it has no fixed value.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] Dandandan commented on pull request #7595: feat: Parallel collecting parquet files statistics #7573
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on PR #7595:
URL: https://github.com/apache/arrow-datafusion/pull/7595#issuecomment-1728913591
Thanks @hengfeiyang
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] Dandandan merged pull request #7595: feat: Parallel collecting parquet files statistics #7573
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan merged PR #7595:
URL: https://github.com/apache/arrow-datafusion/pull/7595
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] hengfeiyang commented on a diff in pull request #7595: feat: Parallel collecting parquet files statistics #7573
Posted by "hengfeiyang (via GitHub)" <gi...@apache.org>.
hengfeiyang commented on code in PR #7595:
URL: https://github.com/apache/arrow-datafusion/pull/7595#discussion_r1330038681
##########
datafusion/common/src/config.rs:
##########
@@ -251,6 +251,11 @@ config_namespace! {
/// and sorted in a single RecordBatch rather than sorted in
/// batches and merged.
pub sort_in_place_threshold_bytes: usize, default = 1024 * 1024
+
+ /// Number of files to read in parallel when inferring schema and statistics
+ ///
+ /// Defaults to the number of CPU cores on the system
+ pub meta_fetch_concurrency: usize, default = num_cpus::get()
Review Comment:
@Dandandan I agree set it higher than `cpu::num`, actually I use `cpu::num() * 4` in my project. but for fixed `32`, the user may have `4` CPU cores or `64` cores.
But this is a default value, users can override it. I think we can use `32`.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] Dandandan commented on a diff in pull request #7595: feat: Parallel collecting parquet files statistics #7573
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on code in PR #7595:
URL: https://github.com/apache/arrow-datafusion/pull/7595#discussion_r1330028713
##########
datafusion/common/src/config.rs:
##########
@@ -251,6 +251,11 @@ config_namespace! {
/// and sorted in a single RecordBatch rather than sorted in
/// batches and merged.
pub sort_in_place_threshold_bytes: usize, default = 1024 * 1024
+
+ /// Number of files to read in parallel when inferring schema and statistics
+ ///
+ /// Defaults to the number of CPU cores on the system
+ pub meta_fetch_concurrency: usize, default = num_cpus::get()
Review Comment:
I think it might be weird to use the number of cpu's as value here, as to benefit from it could be higher than the number of cpu's.
What about using the fixed value `32` here by default used previously?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #7595: feat: Parallel collecting parquet files statistics #7573
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #7595:
URL: https://github.com/apache/arrow-datafusion/pull/7595#discussion_r1332160683
##########
datafusion/common/src/config.rs:
##########
@@ -251,6 +251,11 @@ config_namespace! {
/// and sorted in a single RecordBatch rather than sorted in
/// batches and merged.
pub sort_in_place_threshold_bytes: usize, default = 1024 * 1024
+
+ /// Number of files to read in parallel when inferring schema and statistics
+ ///
+ /// Defaults to the number of CPU cores on the system
+ pub meta_fetch_concurrency: usize, default = num_cpus::get()
Review Comment:
What is the final plan for this PR? Will we set the default to `32` ? Or shall we leave it at the number of cores?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org