Skip to content

feat!: Add the ability to probe a data_batch#77

Draft
dhruv9vats wants to merge 1 commit intoNVIDIA:mainfrom
dhruv9vats:data-batch-probe
Draft

feat!: Add the ability to probe a data_batch#77
dhruv9vats wants to merge 1 commit intoNVIDIA:mainfrom
dhruv9vats:data-batch-probe

Conversation

@dhruv9vats
Copy link
Member

Introduces an interface idata_batch_probe that can used to probe the state of a data_batch at different points in time, currently when the batch_state of the data_batch changes. Useful to see how data_batches are working in a system.

@copy-pr-bot
Copy link

copy-pr-bot bot commented Mar 5, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@dhruv9vats dhruv9vats added breaking feature request New feature or request labels Mar 5, 2026
Copy link

@johanpel johanpel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks! Just some nits

* while this executes. Primarily intended for bookkeeping purposes.
*/
virtual void state_transitioned_to([[maybe_unused]] const batch_state& new_state,
[[maybe_unused]] const uint64_t& batch_id,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[[maybe_unused]] const uint64_t& batch_id,
[[maybe_unused]] uint64_t batch_id,

should be trivial to copy (and more of these below)

virtual void state_transitioned_to([[maybe_unused]] const batch_state& new_state,
[[maybe_unused]] const uint64_t& batch_id,
[[maybe_unused]] const idata_representation& data,
[[maybe_unused]] const size_t& processing_count,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[[maybe_unused]] const size_t& processing_count,
[[maybe_unused]] size_t processing_count,

[[maybe_unused]] const uint64_t& batch_id,
[[maybe_unused]] const idata_representation& data,
[[maybe_unused]] const size_t& processing_count,
[[maybe_unused]] const size_t& task_created_count)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[[maybe_unused]] const size_t& task_created_count)
[[maybe_unused]] size_t task_created_count)

std::unique_ptr<idata_batch_probe> probe)
: _batch_id(batch_id), _data(std::move(data)), _probe(std::move(probe))
{
std::lock_guard<std::mutex> lock(_mutex);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: shouldn't technically be necessary if update_state_to had a lockless variant

@dhruv9vats dhruv9vats changed the title feat!: Add the ability to prode a data_batch feat!: Add the ability to probe a data_batch Mar 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking feature request New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants