mkleen commented on code in PR #23490:
URL: https://github.com/apache/datafusion/pull/23490#discussion_r3593799686
##########
datafusion/datasource/src/write/orchestration.rs:
##########
@@ -251,8 +296,32 @@ pub async fn spawn_writer_tasks_and_join(
compression_level: Option<u32>,
object_store: Arc<dyn ObjectStore>,
demux_task: SpawnedTask<Result<()>>,
+ file_stream_rx: DemuxedStreamReceiver,
+) -> Result<u64> {
+ spawn_writer_tasks_and_join_with_metrics(
+ context,
+ serializer,
+ StatelessWriterOptions::new(compression)
+ .with_compression_level(compression_level),
+ object_store,
+ demux_task,
+ file_stream_rx,
+ )
+ .await
+}
+
+/// Like [`spawn_writer_tasks_and_join`], and also records common file sink
metrics.
+pub async fn spawn_writer_tasks_and_join_with_metrics(
+ context: &Arc<TaskContext>,
+ serializer: Arc<dyn BatchSerializer>,
+ options: StatelessWriterOptions<'_>,
+ object_store: Arc<dyn ObjectStore>,
+ demux_task: SpawnedTask<Result<()>>,
mut file_stream_rx: DemuxedStreamReceiver,
) -> Result<u64> {
+ let _timer = options
Review Comment:
This timer measures the entire `spawn_writer_tasks_and_join_with_metrics`
function including object store io, channel waits, etc. The Parquet version
counts only poll time. This will overstate the metric for slow object stores.
Can we maybe only measure the serialization work inside
`serialize_rb_stream_to_object_store` e.g.:
```rust
let elapsed_compute = options.metrics.map(|m| m.elapsed_compute().clone());
```
pass that on over `spawn_writer_tasks_and_join_with_metrics` to
`serialize_rb_stream_to_object_store` and then we could do something like:
```rust
while let Some(batch) = data_rx.recv().await {
let serializer_clone = Arc::clone(&serializer);
let task = SpawnedTask::spawn(async move {
let num_rows = batch.num_rows();
let timer = elapsed_compute.as_ref().map(|t| t.timer()); //
<- measure only the poll time
let bytes = serializer_clone.serialize(batch, initial)?;
// stop measuring
Ok((num_rows, bytes))
});
```
then we would measure only poll time.
--
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]
