Skip to content

fix(loader): show throughput even when errors occur#670

Merged
luisremis merged 7 commits into
developfrom
fix/244-loader-stats-nan-error
May 19, 2026
Merged

fix(loader): show throughput even when errors occur#670
luisremis merged 7 commits into
developfrom
fix/244-loader-stats-nan-error

Conversation

@ad-claw000
Copy link
Copy Markdown
Contributor

Closes #244

Show the Overall insertion throughput instead of NaN when the error counter is > 0.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes loader/parallel execution stats output so “Overall insertion throughput” is reported as a numeric value even when errors occurred (instead of the literal NaN string), addressing issue #244 where throughput was hidden whenever error_counter > 0.

Changes:

  • Always print the computed overall throughput value in print_stats() for ParallelLoader, ParallelQuery, and ParallelQuerySet (no longer conditional on error_counter == 0).
  • Update the stats test expectation to validate a numeric throughput output when errors occur.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
aperturedb/ParallelQuerySet.py Removes the error_counter-based conditional that forced throughput to print as NaN.
aperturedb/ParallelQuery.py Removes the error_counter-based conditional that forced throughput to print as NaN.
aperturedb/ParallelLoader.py Removes the error_counter-based conditional that forced throughput to print as NaN.
test/test_Stats.py Updates the assertion for overall throughput output when errors occur.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/test_Stats.py Outdated
Comment thread aperturedb/ParallelLoader.py Outdated
Comment thread aperturedb/ParallelQuery.py Outdated
Comment thread aperturedb/ParallelQuerySet.py Outdated
@ad-claw000 ad-claw000 force-pushed the fix/244-loader-stats-nan-error branch from 579a023 to c82424e Compare May 19, 2026 02:35
Addresses review feedback from Copilot regarding throughput calculation
@ad-claw000
Copy link
Copy Markdown
Contributor Author

Addressed feedback from @Copilot:

  • Updated i_tp throughput calculation in ParallelLoader, ParallelQuery, and ParallelQuerySet to use successfully inserted elements instead of planned actions.
  • Adjusted test_Stats.py assertion to expect a float of 0 when all queries fail, instead of throwing an isinstance error.

Updated in commit aa5c638

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 6 changed files in this pull request and generated no new comments.

@ad-claw000 ad-claw000 requested a review from luisremis May 19, 2026 04:08
luisremis
luisremis previously approved these changes May 19, 2026
@ad-claw000
Copy link
Copy Markdown
Contributor Author

I noticed the CI failure related to succeeded_queries in dry_run mode (which caused test_varying_commands_blobs to fail). I pushed a fix for that to correctly populate actual_stats.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 6 changed files in this pull request and generated no new comments.

luisremis
luisremis previously approved these changes May 19, 2026
Copilot AI review requested due to automatic review settings May 19, 2026 20:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 7 changed files in this pull request and generated no new comments.

@luisremis luisremis merged commit 92f1997 into develop May 19, 2026
3 checks passed
@luisremis luisremis deleted the fix/244-loader-stats-nan-error branch May 19, 2026 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Loader stats showing NaN when error occur

3 participants