-
Notifications
You must be signed in to change notification settings - Fork 246
chore: Add memory reservation debug logging and visualization #2521
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
Conversation
native/core/src/execution/jni_api.rs
Outdated
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.
rather than adding yet another flag to this API call, I am now using the already available spark config map in native code.
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.
+1. The config map should be the preferred method
Codecov Report
✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.93%. Comparing base (f09f8af) to head (2884ed3).
Additional details and impacted files
@@ Coverage Diff @@ ## main #2521 +/- ## ============================================ + Coverage 56.12% 58.93% +2.80% - Complexity 976 1449 +473 ============================================ Files 119 147 +28 Lines 11743 13649 +1906 Branches 2251 2369 +118 ============================================ + Hits 6591 8044 +1453 - Misses 4012 4382 +370 - Partials 1140 1223 +83
☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.
🚀 New features to boost your workflow:
- ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
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.
should be println as info! or trace! ?
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.
I guess info! would be ok. I pushed that change. If we use trace! then we would have to set spark.comet.debug.memory=true and also configure trace logging for this one file, which seem like overkill for a debug feature
moving to draft while I work on the Python scripts
Still experimenting...
mem_chart
Chart now shows when try_grow failed:
Uh oh!
There was an error while loading. Please reload this page.
Which issue does this PR close?
Closes #.
Rationale for this change
Debugging.
From this, we can make pretty charts to help with comprehension:
imageWhat changes are included in this PR?
spark.comet.debug.memoryLoggingPoolthat is enabled when the new config is setHow are these changes tested?