2010YOUY01 commented on code in PR #24204: URL: https://github.com/apache/datafusion/pull/24204#discussion_r3746002094
##########
datafusion/common/src/config.rs:
##########
@@ -663,6 +663,91 @@ impl Display for ConfigNonZeroUsize {
}
}
+/// A `usize` configuration value that rejects 0 and 1 when set from strings.
+///
+/// Use this for options whose consumer divides the value in half to size an
+/// internal buffer (e.g. a bounded channel capacity): values below 2 would
+/// round down to a zero-capacity buffer and panic. Invalid values return a
+/// configuration error through [`ConfigField`] instead.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub struct ConfigMinTwoUsize(usize);
Review Comment:
Perhaps we can make the min value a configurable `k`, but it's not necessary
now, we can reuse it when there is a need in other configurations.
##########
docs/source/user-guide/configs.md:
##########
@@ -130,7 +130,7 @@ The following configuration settings are available:
| datafusion.execution.meta_fetch_concurrency | 32
| Number of files to read in parallel when inferring
schema and statistics
|
| datafusion.execution.minimum_parallel_output_files | 4
| Guarantees a minimum level of output files running in
parallel. RecordBatches will be distributed in round robin fashion to each
parallel writer. Each writer is closed and a new file opened once
soft_max_rows_per_output_file is reached.
|
| datafusion.execution.soft_max_rows_per_output_file |
50000000 | Target number of rows in output files when writing
multiple. This is a soft max, so it can be exceeded slightly. There also will
be one file smaller than the limit if the total number of rows written is not
roughly divisible by the soft max
|
-| datafusion.execution.max_buffered_batches_per_output_file | 2
| This is the maximum number of RecordBatches buffered
for each output file being worked. Higher values can potentially give faster
write performance at the cost of higher peak memory consumption
|
+| datafusion.execution.max_buffered_batches_per_output_file | 2
| This is the maximum number of RecordBatches buffered
for each output file being worked. Higher values can potentially give faster
write performance at the cost of higher peak memory consumption. Must be at
least 2: this value is halved to size internal buffering channels, and a value
of 0 or 1 would create a zero-capacity channel and panic at write time.
|
Review Comment:
I don't fully understand why it need to be halved, in the implementation it
seems to be shared evenly between input and output file buffering:
https://github.com/apache/datafusion/blob/bc48a4f7acd87a9779cb5a88d9b1a3f4ce506e45/datafusion/datasource/src/write/demux.rs#L287
Would be nice if we can explain it better.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
