Merged
Conversation
Member
karya0
commented
Apr 21, 2025
karya0
commented
- DMTCP_LOG_FILE env var to specify a path for log file. This allows multiple processes to use the same log file and is important when debugging distributed computations.
- Single-line logging -- when logging to single file, one line per log message makes it easier to parse log files using external tooling.
- Bug fixes to make --enable-logging work.
- Some non-fatal asserts replaced with warnings.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR improves the logging infrastructure and fixes related bugs to simplify diagnostics during distributed computations. Key changes include:
- Enhanced logging statements with safer type casts and one-line logging for easier parsing.
- Refactored timestamp generation and environment-based log file initialization.
- Bug fixes in coordinator message and signal handling along with associated assertion/logging utilities.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/writeckpt.cpp | Added explicit casts in logging to clarify address types. |
| src/util_misc.cpp | Replaced gettimeofday with clock_gettime and updated timestamp generation logic. |
| src/util_init.cpp | Introduced DMTCP_LOG_FILE env var to allow custom log file paths. |
| src/plugin/pid/virtualpidtable.cpp | Removed debug output (printMaps) to reduce verbosity. |
| src/dmtcp_coordinator.cpp | Converted several JASSERT instances to warnings and refined signal/message handling. |
| src/dlwrappers.cpp | Commented out JTRACE calls prone to deadlock to enhance stability. |
| src/constants.h | Updated virtual PID constants and added new ENV_VAR_LOG_FILE definition. |
| jalib/jassert.h | Added overload for Print on mutable char pointers and introduced one-line assert macros. |
| jalib/jassert.cpp | Adjusted assert message formatting and newline handling in the destructor. |
Comments suppressed due to low confidence (1)
src/constants.h:60
- The new MAX_VIRTUAL_PID value is significantly lower than before; please ensure that this limit aligns with system and deployment requirements.
#define MAX_VIRTUAL_PID ((pid_t)1000000)
Co-authored-by: Copilot <[email protected]>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.