-
Notifications
You must be signed in to change notification settings - Fork 4k
GH-33880: [C++] Improve I/O tracing #34168
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
50 commits
Select commit
Hold shift + click to select a range
3f24850
Added low-level file I/O spans
joosthooz d106c42
Added spans to datasetwriter for popstagedbatches and the actual IO w…
joosthooz 8aba916
Added a span for writing parquet.
joosthooz f9da7ad
Removed ReadBatch spans because they were orphans and didn't represen…
joosthooz 10a80d6
File I/O spans now have actual read bytes as property
joosthooz ab34c08
Update span name
joosthooz 4e48581
Added spans to ipc compression functions because they are called by a…
joosthooz 1a1307a
Added a span to a parallelFor lambda in parquet: GetRecordBatchReader…
joosthooz 56b8535
Creating a single span for a projection instead of 1 for each expression
joosthooz 56c52af
Added span type attributes to task submission spans.
joosthooz 815d317
No longer creating a span when task submission is queued (throttled)
joosthooz 481bbe4
Creating a string_view from the task name basic_string_view
joosthooz 1f3adee
Added the size of the output to some spans.
joosthooz 43a7fba
Added (presumably missing) START_SPAN to OrderBySinkNode
joosthooz af56a27
Removed unintended line
joosthooz 24b5ff8
Do not create toplevel task span at task submission
joosthooz 37e84e4
Removed arrow::dataset::CsvFileFormat::ScanBatchesAsync::Next span in…
joosthooz ed397fc
Added the size of the batch outputted by CSV reader to the span.
joosthooz 2f7fa97
Added span for processing the first block.
joosthooz 38c4639
Creating explicit ProcessMorsel and DatasetScan spans
joosthooz 8764d34
Added span for keeping track of re-chunking during scanning
joosthooz 58c9db4
Reverting back to PeekAndMakeAsync,
joosthooz ee28b21
Posting datasetwriter backpressure events to the asyncscheduler span
joosthooz 1163ef1
Added push and pop spans to the datasetwriter that allow tracking sta…
joosthooz 07343d7
Formatting
joosthooz a3a126d
Fixed typo in span attribute
joosthooz e1a0a0f
Added a surrogate arrow::csv::ReadNextAsync span for the first block …
joosthooz 6a9b635
WIP writing some more extensive documentation about tracing Acero
joosthooz ecd80c4
Merge remote-tracking branch 'origin/main' into GH-33880-improve-trac…
joosthooz a28a1db
Added helper function ATTRIBUTE_ON_CURRENT_SPAN
joosthooz a665bb6
Merge remote-tracking branch 'origin/main' into GH-33880-improve-tracing
joosthooz 2129678
Reverted commenting out task submission tracing
joosthooz 06bcc1e
Fix typo
joosthooz c8fa9f6
Using helper function in some more places
joosthooz d315264
Removed some unused code from the datasetwriter
joosthooz 407bcb2
Propagating span context to SimpleTask submission functions
joosthooz ccdf2ec
Removed TODO, some typos
joosthooz 45f03fa
Formatting
joosthooz 4e0d497
Merge branch 'main' into GH-33880-improve-tracing
joosthooz 42fc7b6
Added missing empty macro for ATTRIBUTE_ON_CURRENT_SPAN()
joosthooz b64ca22
Revert "Propagating span context to SimpleTask submission functions"
joosthooz 898b093
Propagating parent span through task submission
joosthooz 78ea497
Increased queue size for tracing to avoid dropping spans
joosthooz 4028cd6
Removed task submission trace functions
joosthooz 19522e5
Formatting
joosthooz b9df0be
Updated some namespace usages
joosthooz 07d5012
Wrote some documentation about the structure of traces
joosthooz 14874ca
Merge remote-tracking branch 'origin/main' into GH-33880-improve-tracing
joosthooz 50e808c
Removed some of the more intrusive Otel spans
joosthooz fc9c659
Removed the low-level File I/O spans, there's just too many of them a…
joosthooz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,7 @@ | |
| #include "arrow/record_batch.h" | ||
| #include "arrow/result.h" | ||
| #include "arrow/table.h" | ||
| #include "arrow/util/byte_size.h" | ||
| #include "arrow/util/future.h" | ||
| #include "arrow/util/logging.h" | ||
| #include "arrow/util/map.h" | ||
|
|
@@ -191,9 +192,13 @@ class DatasetWriterFileQueue { | |
| } | ||
|
|
||
| Result<int64_t> PopAndDeliverStagedBatch() { | ||
| util::tracing::Span span; | ||
| START_SPAN(span, "DatasetWriter::Pop"); | ||
| ARROW_ASSIGN_OR_RAISE(std::shared_ptr<RecordBatch> next_batch, PopStagedBatch()); | ||
| int64_t rows_popped = next_batch->num_rows(); | ||
| rows_currently_staged_ -= next_batch->num_rows(); | ||
| ATTRIBUTE_ON_CURRENT_SPAN("batch.size_rows", next_batch->num_rows()); | ||
| ATTRIBUTE_ON_CURRENT_SPAN("rows_currently_staged", rows_currently_staged_); | ||
| ScheduleBatch(std::move(next_batch)); | ||
| return rows_popped; | ||
| } | ||
|
|
@@ -202,7 +207,15 @@ class DatasetWriterFileQueue { | |
| Status Push(std::shared_ptr<RecordBatch> batch) { | ||
| uint64_t delta_staged = batch->num_rows(); | ||
| rows_currently_staged_ += delta_staged; | ||
| staged_batches_.push_back(std::move(batch)); | ||
| { | ||
| util::tracing::Span span; | ||
| START_SPAN(span, "DatasetWriter::Push", | ||
| {{"batch.size_rows", batch->num_rows()}, | ||
| {"rows_currently_staged", rows_currently_staged_}, | ||
| {"options_.min_rows_per_group", options_.min_rows_per_group}, | ||
| {"max_rows_staged", writer_state_->max_rows_staged}}); | ||
| staged_batches_.push_back(std::move(batch)); | ||
| } | ||
| while (!staged_batches_.empty() && | ||
| (writer_state_->StagingFull() || | ||
| rows_currently_staged_ >= options_.min_rows_per_group)) { | ||
|
|
@@ -233,6 +246,18 @@ class DatasetWriterFileQueue { | |
| return DeferNotOk(options_.filesystem->io_context().executor()->Submit( | ||
| [self = this, batch = std::move(next)]() { | ||
| int64_t rows_to_release = batch->num_rows(); | ||
| #ifdef ARROW_WITH_OPENTELEMETRY | ||
| uint64_t size_bytes = util::TotalBufferSize(*batch); | ||
| uint64_t num_buffers = 0; | ||
| for (auto column : batch->columns()) { | ||
| num_buffers += column->data()->buffers.size(); | ||
| } | ||
| util::tracing::Span span; | ||
| START_SPAN(span, "DatasetWriter::WriteNext", | ||
| {{"threadpool", "IO"}, | ||
| {"batch.size_bytes", size_bytes}, | ||
| {"batch.num_buffers", num_buffers}}); | ||
| #endif | ||
| Status status = self->writer_->Write(batch); | ||
| self->writer_state_->rows_in_flight_throttle.Release(rows_to_release); | ||
| return status; | ||
|
|
@@ -261,11 +286,6 @@ class DatasetWriterFileQueue { | |
| util::AsyncTaskScheduler* file_tasks_ = nullptr; | ||
| }; | ||
|
|
||
| struct WriteTask { | ||
| std::string filename; | ||
| uint64_t num_rows; | ||
| }; | ||
|
|
||
| class DatasetWriterDirectoryQueue { | ||
| public: | ||
| DatasetWriterDirectoryQueue(util::AsyncTaskScheduler* scheduler, std::string directory, | ||
|
|
@@ -301,7 +321,6 @@ class DatasetWriterDirectoryQueue { | |
|
|
||
| Status StartWrite(const std::shared_ptr<RecordBatch>& batch) { | ||
| rows_written_ += batch->num_rows(); | ||
| WriteTask task{current_filename_, static_cast<uint64_t>(batch->num_rows())}; | ||
| if (!latest_open_file_) { | ||
| ARROW_RETURN_NOT_OK(OpenFileQueue(current_filename_)); | ||
| } | ||
|
|
@@ -351,6 +370,8 @@ class DatasetWriterDirectoryQueue { | |
| latest_open_file_tasks_ = util::MakeThrottledAsyncTaskGroup( | ||
| scheduler_, 1, /*queue=*/nullptr, std::move(file_finish_task)); | ||
| if (init_future_.is_valid()) { | ||
| util::tracing::Span span; | ||
| START_SPAN(span, "arrow::dataset::WaitForDirectoryInit"); | ||
| latest_open_file_tasks_->AddSimpleTask( | ||
| [init_future = init_future_]() { return init_future; }, | ||
| "DatasetWriter::WaitForDirectoryInit"sv); | ||
|
|
@@ -362,6 +383,8 @@ class DatasetWriterDirectoryQueue { | |
| uint64_t rows_written() const { return rows_written_; } | ||
|
|
||
| void PrepareDirectory() { | ||
| util::tracing::Span span; | ||
| START_SPAN(span, "arrow::dataset::SubmitPrepareDirectoryTask"); | ||
| if (directory_.empty() || !write_options_.create_dir) { | ||
| return; | ||
| } | ||
|
|
@@ -383,6 +406,8 @@ class DatasetWriterDirectoryQueue { | |
| if (write_options_.existing_data_behavior == | ||
| ExistingDataBehavior::kDeleteMatchingPartitions) { | ||
| init_task = [this, create_dir_cb, notify_waiters_cb, notify_waiters_on_err_cb] { | ||
| util::tracing::Span span; | ||
| START_SPAN(span, "arrow::dataset::PrepareDirectory"); | ||
| return write_options_.filesystem | ||
| ->DeleteDirContentsAsync(directory_, | ||
| /*missing_dir_ok=*/true) | ||
|
|
@@ -614,12 +639,14 @@ class DatasetWriter::DatasetWriterImpl { | |
| backpressure = | ||
| writer_state_.rows_in_flight_throttle.Acquire(next_chunk->num_rows()); | ||
| if (!backpressure.is_finished()) { | ||
| EVENT(scheduler_->span(), "DatasetWriter::Backpressure::TooManyRowsQueued"); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why both |
||
| EVENT_ON_CURRENT_SPAN("DatasetWriter::Backpressure::TooManyRowsQueued"); | ||
| break; | ||
| } | ||
| if (will_open_file) { | ||
| backpressure = writer_state_.open_files_throttle.Acquire(1); | ||
| if (!backpressure.is_finished()) { | ||
| EVENT(scheduler_->span(), "DatasetWriter::Backpressure::TooManyOpenFiles"); | ||
| EVENT_ON_CURRENT_SPAN("DatasetWriter::Backpressure::TooManyOpenFiles"); | ||
| RETURN_NOT_OK(TryCloseLargestFile()); | ||
| break; | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do I need to know
num_buffers?