From eb5e22eeae87a8a49d6786519df7928b47f81174 Mon Sep 17 00:00:00 2001 From: Blaise Bourdin Date: Wed, 2 Sep 2026 10:59:47 -0400 Subject: [PATCH] ncmpio: keep zero-length collective requests in the numrecs sync 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; ivars.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) Claude-Session: https://claude.ai/code/session_01SnBfenhctkYPJbfQAohcBe --- src/drivers/ncmpio/ncmpio_getput.m4 | 23 +++-- test/testcases/Makefile.am | 1 + test/testcases/tst_zero_len_recvar.c | 147 +++++++++++++++++++++++++++ 3 files changed, 165 insertions(+), 6 deletions(-) create mode 100644 test/testcases/tst_zero_len_recvar.c diff --git a/src/drivers/ncmpio/ncmpio_getput.m4 b/src/drivers/ncmpio/ncmpio_getput.m4 index a2850ed812..2cddfaa0dd 100644 --- a/src/drivers/ncmpio/ncmpio_getput.m4 +++ b/src/drivers/ncmpio/ncmpio_getput.m4 @@ -122,7 +122,7 @@ put_varm(NC *ncp, MPI_Offset nelems=0, bnelems=0, nbytes=0; MPI_Datatype itype, imaptype; - if (varp == NULL) { /* zero-sized request */ + if (varp == NULL || fIsSet(reqMode, NC_REQ_ZERO)) { /* zero-sized request */ itype = MPI_BYTE; el_size = 0; bnelems = 0; @@ -345,7 +345,7 @@ get_varm(NC *ncp, MPI_Offset nelems=0, bnelems=0, nbytes=0; MPI_Datatype itype, imaptype=MPI_DATATYPE_NULL; - if (varp == NULL) { /* zero-sized request */ + if (varp == NULL || fIsSet(reqMode, NC_REQ_ZERO)) { /* zero-sized request */ itype = MPI_BYTE; el_size = 0; bnelems = 0; @@ -509,14 +509,16 @@ ncmpio_$1_var(void *ncdp, { NC *ncp=(NC*)ncdp; - NC_var *varp; + NC_var *varp=NULL; /* Check if this is a true zero-sized request. Note NC_REQ_ZERO is added to - * reqMode only when an error is detected at the dispatcher level. + * reqMode only when an error is detected at the dispatcher level, in which + * case varid may not be valid and must not be used to look up varp. */ if (!fIsSet(reqMode, NC_REQ_ZERO)) { int i; - for (i=0; ivars.value[varid]->ndims; i++) + varp = ncp->vars.value[varid]; + for (i=0; indims; i++) if (count[i] == 0) { reqMode |= NC_REQ_ZERO; break; @@ -529,8 +531,17 @@ ncmpio_$1_var(void *ncdp, /* In case some processes in an aggregation group have nothing to * write, they still need to participate the communication part of the * intra-node aggregation operation. + * + * varp is handed down whenever it is known to be valid, because + * put_varm() consults it to decide whether this is a record variable. + * A zero-length collective request to a record variable must still + * take part in the MPI_Allreduce() and ncmpio_write_numrecs() that + * synchronise numrecs: the processes that do have data call them + * unconditionally, so a process skipping them leaves the communicator + * out of step. varp stays NULL only when NC_REQ_ZERO came from a + * dispatcher-level error, where varid cannot be trusted. */ - return $1_varm(ncp, NULL, NULL, NULL, NULL, imap, NULL, 0, + return $1_varm(ncp, varp, NULL, NULL, NULL, imap, NULL, 0, buftype, reqMode); } diff --git a/test/testcases/Makefile.am b/test/testcases/Makefile.am index 0f3b7fd8a6..52c1d9f6b1 100644 --- a/test/testcases/Makefile.am +++ b/test/testcases/Makefile.am @@ -104,6 +104,7 @@ check_PROGRAMS = file_create_open \ put_all_kinds \ redef1 \ iput_all_kinds \ + tst_zero_len_recvar \ tst_version M4_SRCS = put_all_kinds.m4 \ diff --git a/test/testcases/tst_zero_len_recvar.c b/test/testcases/tst_zero_len_recvar.c new file mode 100644 index 0000000000..0c8e752e91 --- /dev/null +++ b/test/testcases/tst_zero_len_recvar.c @@ -0,0 +1,147 @@ +/****************************************************************************** + * + * Copyright (C) 2026, Northwestern University and Argonne National Laboratory + * See COPYRIGHT notice in top-level directory. + * + *****************************************************************************/ +/* $Id$ */ + +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * This program tests a collective write to a record variable in which one of + * the processes has nothing to contribute, i.e. it passes count == 0. Every + * process still calls the collective API, so the call is legal. + * + * Such a request must not opt out of the collective synchronization of numrecs + * that follows the I/O. If it does, the processes that do have data to write + * are left calling an MPI_Allreduce() alone and the communicator goes out of + * step, which shows up as a wrong error code, as corruption of a later + * collective call, or as a hang. + * + * Fixed-size variables take no part in the numrecs synchronization and are + * therefore not affected; the record variable is the case worth guarding. + * + * To compile: + * mpicc -O2 tst_zero_len_recvar.c -o tst_zero_len_recvar -lpnetcdf + * + * % mpiexec -n 4 ./tst_zero_len_recvar testfile.nc + * + * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ + +#include +#include +#include +#include /* basename() */ +#include + +#include + +#define NDBL 4 /* number of doubles written per process */ + +static +int test_io(const char *out_path, + const char *in_path, /* ignored */ + int format, + int coll_io, + MPI_Info info) +{ + int i, err, nerrs=0, ncid, dimid[2], varid, rank, nprocs, is_empty; + MPI_Offset start[2], count[2], numrecs; + double buf[NDBL], rbuf[NDBL]; + + MPI_Comm_rank(MPI_COMM_WORLD, &rank); + MPI_Comm_size(MPI_COMM_WORLD, &nprocs); + + /* the last process contributes nothing; with a single process there is + * nobody left to write, so keep its request non-empty + */ + is_empty = (nprocs > 1 && rank == nprocs - 1); + + for (i=0; i