Skip to content

ncmpio: keep zero-length collective requests in the numrecs sync - #238

Open
bourdin wants to merge 1 commit into
Parallel-NetCDF:masterfrom
bourdin:fix-zero-length-collective-numrecs
Open

ncmpio: keep zero-length collective requests in the numrecs sync#238
bourdin wants to merge 1 commit into
Parallel-NetCDF:masterfrom
bourdin:fix-zero-length-collective-numrecs

Conversation

@bourdin

@bourdin bourdin commented Sep 2, 2026

Copy link
Copy Markdown

Background:

I have been investigating a deadlock in my fracture mechanics code (mef90), which depends on pnetcdf-1.14.1 through the exodusII libraries. claude linked the issue to something that was fixed in 1.15, but we are still having problems. The attached patch (which I have not reviewed as I really don't understand the pnetcdf code base) seem to fix it.

Would you mind having a look at the reproducer and the proposed fix?

Regards,
Blaise

==============
A collective put/get of a record variable in which one process contributes a zero-length request leaves that process out of the collective synchronisation of numrecs, so the communicator goes out of step.

GETPUT_API() promotes a genuine count[i] == 0 request to NC_REQ_ZERO and routes NC_REQ_ZERO + NC_REQ_COLL into put_varm()/get_varm() with varp == NULL:

if (!fIsSet(reqMode, NC_REQ_ZERO)) {
    for (i=0; i<ncp->vars.value[varid]->ndims; i++)
        if (count[i] == 0) { reqMode |= NC_REQ_ZERO; break; }
}
if (fIsSet(reqMode, NC_REQ_ZERO) && fIsSet(reqMode, NC_REQ_COLL))
    return $1_varm(ncp, NULL, ..., reqMode);

but the tail of put_varm() guards the numrecs synchronisation with varp:

if (varp != NULL && IS_RECVAR(varp)) {
    ...
    if (fIsSet(reqMode, NC_REQ_COLL)) {
        MPI_Allreduce(&new_numrecs, &max_numrecs, 1, MPI_OFFSET, MPI_MAX,
                      ncp->comm);
        if (ncp->numrecs < max_numrecs)
            ncmpio_write_numrecs(ncp, max_numrecs);
    }
}

so a process with nothing to write skips an MPI_Allreduce() that the processes with data call unconditionally.

Two processes writing disjoint halves of one record, one of them empty (mvapich 4.0, CDF-2):

[1] -> put_vara start 0 count 0
[0] -> put_vara start 0 count 4
[1] <- put_vara_all err 0 (No error)
[0] <- put_vara_all err -221 (Integer type casting overflow.)
Abort on node 1: Fatal error in internal_Allreduce: Message truncated

Depending on what the caller does next this appears as a bogus error, as corruption of a later collective, or as a hang. It makes parallel exodus writers deadlock, since exodus stores time-dependent fields as record variables and ex_put_partial_var() passes count = 0 for a process owning no element in a block.

Hand varp down whenever it is known to be valid, and use NC_REQ_ZERO rather than varp == NULL as the "zero-sized request" signal inside put_varm() and get_varm(). The buffer-side work is still skipped and ncmpio_ina_req() already ignores varp/start/count for a zero-length request, so the only behavioural change is that the record-variable branch is now reached by every process in the collective. varp is left NULL only when NC_REQ_ZERO came from a dispatcher-level error, where varid cannot be trusted.

Add test/testcases/tst_zero_len_recvar.c, which writes one record with the last process passing count == 0 and then checks numrecs and the data read back, over all classic formats and both data modes.

Claude-Session: https://claude.ai/code/session_01SnBfenhctkYPJbfQAohcBe

A collective put/get of a record variable in which one process contributes a
zero-length request leaves that process out of the collective synchronisation
of numrecs, so the communicator goes out of step.

GETPUT_API() promotes a genuine count[i] == 0 request to NC_REQ_ZERO and routes
NC_REQ_ZERO + NC_REQ_COLL into put_varm()/get_varm() with varp == NULL:

    if (!fIsSet(reqMode, NC_REQ_ZERO)) {
        for (i=0; i<ncp->vars.value[varid]->ndims; i++)
            if (count[i] == 0) { reqMode |= NC_REQ_ZERO; break; }
    }
    if (fIsSet(reqMode, NC_REQ_ZERO) && fIsSet(reqMode, NC_REQ_COLL))
        return $1_varm(ncp, NULL, ..., reqMode);

but the tail of put_varm() guards the numrecs synchronisation with varp:

    if (varp != NULL && IS_RECVAR(varp)) {
        ...
        if (fIsSet(reqMode, NC_REQ_COLL)) {
            MPI_Allreduce(&new_numrecs, &max_numrecs, 1, MPI_OFFSET, MPI_MAX,
                          ncp->comm);
            if (ncp->numrecs < max_numrecs)
                ncmpio_write_numrecs(ncp, max_numrecs);
        }
    }

so a process with nothing to write skips an MPI_Allreduce() that the processes
with data call unconditionally.

Two processes writing disjoint halves of one record, one of them empty
(mvapich 4.0, CDF-2):

    [1] -> put_vara start 0 count 0
    [0] -> put_vara start 0 count 4
    [1] <- put_vara_all err 0 (No error)
    [0] <- put_vara_all err -221 (Integer type casting overflow.)
    Abort on node 1: Fatal error in internal_Allreduce: Message truncated

Depending on what the caller does next this appears as a bogus error, as
corruption of a later collective, or as a hang. It makes parallel exodus
writers deadlock, since exodus stores time-dependent fields as record variables
and ex_put_partial_var() passes count = 0 for a process owning no element in a
block.

Hand varp down whenever it is known to be valid, and use NC_REQ_ZERO rather
than varp == NULL as the "zero-sized request" signal inside put_varm() and
get_varm(). The buffer-side work is still skipped and ncmpio_ina_req() already
ignores varp/start/count for a zero-length request, so the only behavioural
change is that the record-variable branch is now reached by every process in
the collective. varp is left NULL only when NC_REQ_ZERO came from a
dispatcher-level error, where varid cannot be trusted.

Add test/testcases/tst_zero_len_recvar.c, which writes one record with the last
process passing count == 0 and then checks numrecs and the data read back, over
all classic formats and both data modes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SnBfenhctkYPJbfQAohcBe
@wkliao

wkliao commented Sep 3, 2026

Copy link
Copy Markdown
Member

Thanks for reporting this issue and providing a reproducer.
I was able to reproduce the problem and will soon create a PR to fix this issue.

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.

2 participants