Repository navigation
Name the drains' KVDB keys per checkpoint - #541
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 51 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
xuyao00
left a comment
xuyao00
left a comment
There was a problem hiding this comment.
Looks good. Can you improve the comment for function ckpt_kvdb_name()? It explains why we need this function, but didn't explain what this function does.
The collective drain used the same KVDB names at every checkpoint, and
the coordinator keeps its KVDB until a restart. With checkpoints back
to back, each round's convergence count then included the previous
checkpoint's, so the drain could stop with ranks still behind; the P2P
drain then spun until Intel MPI aborted it ("too many outstanding
nonblocking collectives detected").
ckpt_kvdb_name() names both drains' keys by the start of the
computation and the checkpoint's generation, which every rank has from
the coordinator before DMTCP_EVENT_PRESUSPEND. It replaces the P2P
drain's own counter, and stops if a name exceeds the 63 characters that
DMTCP's KVDB keeps.
|
Done in 0d4aa84: the comment now starts with what the function does (the name it writes, and when it fails), then why. |
Fixes #540.
ckpt_kvdb_name()names the collective and P2P drains' KVDB keys by the computation's start and the checkpoint's generation, and replaces the P2P drain's own counter. It also stops if a name exceeds 63 characters, which DMTCP's KVDB truncates silently.Hercules (Intel MPI 2021.7.1, 2 nodes),
nonblocking_collectiveswith 3 checkpoints in a row: 8 of 32 runs fail on main, 32 of 32 pass with this. Suite 22/23 in both modes (attributesis #524).🤖 Generated with Claude Code