From e3d420084c9bcc3e8f2a4e68bc01e39a1a5f48d1 Mon Sep 17 00:00:00 2001 From: Dana Robinson <43805+derobins@users.noreply.github.com> Date: Fri, 1 Sep 2023 07:31:38 -0700 Subject: [PATCH] Revert "[1.10 Merge] Fix serial to parallel chunked dataset file space allocation bug (#3394) (#3475)" (#3479) This reverts commit dd4c6c707370c32eb2722ea18509485e43ecef4f. --- release_docs/RELEASE.txt | 11 -- src/H5Dint.c | 34 ++--- testpar/t_chunk_alloc.c | 295 --------------------------------------- testpar/testphdf5.c | 2 - testpar/testphdf5.h | 1 - 5 files changed, 10 insertions(+), 333 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 046d1f1c61a..9bf9fe0ea30 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -135,17 +135,6 @@ Bug Fixes since HDF5-1.10.10 release =================================== Library ------- - - Fixed a file space allocation bug in the parallel library for chunked - datasets - - With the addition of support for incremental file space allocation for - chunked datasets with filters applied to them that are created/accessed - in parallel, a bug was introduced to the library's parallel file space - allocation code. This could cause file space to not be allocated correctly - for datasets without filters applied to them that are created with serial - file access and later opened with parallel file access. In turn, this could - cause parallel writes to those datasets to place incorrect data in the file. - - Fixed an assertion failure in Parallel HDF5 when a file can't be created due to an invalid library version bounds setting diff --git a/src/H5Dint.c b/src/H5Dint.c index 02154d4f4c3..3d5d06742d2 100644 --- a/src/H5Dint.c +++ b/src/H5Dint.c @@ -1647,13 +1647,12 @@ H5D__append_flush_setup(H5D_t *dset, hid_t dapl_id) static herr_t H5D__open_oid(H5D_t *dataset, hid_t dapl_id) { - H5P_genplist_t *plist; /* Property list */ - H5O_fill_t *fill_prop; /* Pointer to dataset's fill value info */ - unsigned alloc_time_state; /* Allocation time state */ - htri_t msg_exists; /* Whether a particular type of message exists */ - hbool_t layout_init = FALSE; /* Flag to indicate that chunk information was initialized */ - hbool_t must_init_storage = FALSE; - herr_t ret_value = SUCCEED; /* Return value */ + H5P_genplist_t *plist; /* Property list */ + H5O_fill_t *fill_prop; /* Pointer to dataset's fill value info */ + unsigned alloc_time_state; /* Allocation time state */ + htri_t msg_exists; /* Whether a particular type of message exists */ + hbool_t layout_init = FALSE; /* Flag to indicate that chunk information was initialized */ + herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_STATIC_TAG(dataset->oloc.addr) @@ -1795,30 +1794,17 @@ H5D__open_oid(H5D_t *dataset, hid_t dapl_id) * Make sure all storage is properly initialized. * This is important only for parallel I/O where the space must * be fully allocated before I/O can happen. - * - * Storage will be initialized here if either the VFD being used - * has set the H5FD_FEAT_ALLOCATE_EARLY flag to indicate that it - * wishes to force early space allocation OR a parallel VFD is - * being used and the dataset in question doesn't have any filters - * applied to it. If filters are applied to the dataset, collective - * I/O will be required when writing to the dataset, so we don't - * need to initialize storage here, as the collective I/O process - * will coordinate that. */ - must_init_storage = (H5F_INTENT(dataset->oloc.file) & H5F_ACC_RDWR) && - !(*dataset->shared->layout.ops->is_space_alloc)(&dataset->shared->layout.storage); - must_init_storage = must_init_storage && (H5F_HAS_FEATURE(dataset->oloc.file, H5FD_FEAT_ALLOCATE_EARLY) || - (H5F_HAS_FEATURE(dataset->oloc.file, H5FD_FEAT_HAS_MPI) && - dataset->shared->dcpl_cache.pline.nused == 0)); - - if (must_init_storage) { + if ((H5F_INTENT(dataset->oloc.file) & H5F_ACC_RDWR) && + !(*dataset->shared->layout.ops->is_space_alloc)(&dataset->shared->layout.storage) && + H5F_HAS_FEATURE(dataset->oloc.file, H5FD_FEAT_ALLOCATE_EARLY)) { H5D_io_info_t io_info; io_info.dset = dataset; if (H5D__alloc_storage(&io_info, H5D_ALLOC_OPEN, FALSE, NULL) < 0) HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "unable to initialize file storage") - } + } /* end if */ done: if (ret_value < 0) { diff --git a/testpar/t_chunk_alloc.c b/testpar/t_chunk_alloc.c index c38d373b654..ac5b90b5a3c 100644 --- a/testpar/t_chunk_alloc.c +++ b/testpar/t_chunk_alloc.c @@ -483,298 +483,3 @@ test_chunk_alloc(void) /* reopen dataset in parallel, read and verify the data */ verify_data(filename, CHUNK_FACTOR, all, CLOSE, &file_id, &dataset); } - -/* - * A test to verify the following: - * - * - That the library forces allocation of all space in the file - * for a chunked dataset opened with parallel file access when - * that dataset: - * - * - was created with serial file access - * - was created with the default incremental file space - * allocation time - * - has no filters applied to it - * - * In this case, the library has to ensure that all the file - * space for the dataset is allocated so that the MPI processes - * can write to chunks independently of each other and still have - * a consistent view of the file. - * - * - That the library DOES NOT force allocation of all space in - * the file for a chunked dataset opened with parallel file access - * when that dataset: - * - * - was created with serial file access - * - was created with the default incremental file space - * allocation time - * - has filters applied to it - * - * In this case, writes to the dataset are required to be collective, - * so file space can be allocated incrementally in a coordinated - * fashion. - */ -void -test_chunk_alloc_incr_ser_to_par(void) -{ - H5D_space_status_t space_status; - const char *filename; - hsize_t dset_dims[1]; - hsize_t mem_dims[1]; - hsize_t start[1]; - hsize_t stride[1]; - hsize_t count[1]; - hsize_t block[1]; - hsize_t alloc_size; - size_t nchunks; - herr_t ret; - hid_t fid = H5I_INVALID_HID; - hid_t fapl_id = H5I_INVALID_HID; - hid_t dset_id = H5I_INVALID_HID; - hid_t fspace_id = H5I_INVALID_HID; - hid_t mspace_id = H5I_INVALID_HID; - hid_t dxpl_id = H5I_INVALID_HID; - int *data = NULL; - int *correct_data = NULL; - int *read_data = NULL; - - MPI_Comm_size(MPI_COMM_WORLD, &mpi_size); - MPI_Comm_rank(MPI_COMM_WORLD, &mpi_rank); - - filename = (const char *)GetTestParameters(); - if (MAINPROCESS && VERBOSE_MED) - printf("Chunked dataset incremental file space allocation serial to parallel test on file %s\n", - filename); - - nchunks = (size_t)(CHUNK_FACTOR * mpi_size); - dset_dims[0] = (hsize_t)(nchunks * CHUNK_SIZE); - - if (mpi_rank == 0) { - hsize_t chunk_dims[1] = {CHUNK_SIZE}; - hid_t space_id = H5I_INVALID_HID; - hid_t dcpl_id = H5I_INVALID_HID; - - fid = H5Fcreate(filename, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT); - VRFY((fid >= 0), "H5Fcreate"); - - dcpl_id = H5Pcreate(H5P_DATASET_CREATE); - VRFY((dcpl_id >= 0), "H5Pcreate"); - - ret = H5Pset_chunk(dcpl_id, 1, chunk_dims); - VRFY((ret == SUCCEED), "H5Pset_chunk"); - - ret = H5Pset_alloc_time(dcpl_id, H5D_ALLOC_TIME_INCR); - VRFY((ret == SUCCEED), "H5Pset_alloc_time"); - - space_id = H5Screate_simple(1, dset_dims, NULL); - VRFY((space_id >= 0), "H5Screate_simple"); - - /* Create a chunked dataset without a filter applied to it */ - dset_id = - H5Dcreate2(fid, "dset_no_filter", H5T_NATIVE_INT, space_id, H5P_DEFAULT, dcpl_id, H5P_DEFAULT); - VRFY((dset_id >= 0), "H5Dcreate2"); - - ret = H5Dclose(dset_id); - VRFY((ret == SUCCEED), "H5Dclose"); - - /* Create a chunked dataset with a filter applied to it */ - ret = H5Pset_shuffle(dcpl_id); - VRFY((ret == SUCCEED), "H5Pset_shuffle"); - - dset_id = H5Dcreate2(fid, "dset_filter", H5T_NATIVE_INT, space_id, H5P_DEFAULT, dcpl_id, H5P_DEFAULT); - VRFY((dset_id >= 0), "H5Dcreate2"); - - ret = H5Dclose(dset_id); - VRFY((ret == SUCCEED), "H5Dclose"); - ret = H5Pclose(dcpl_id); - VRFY((ret == SUCCEED), "H5Pclose"); - ret = H5Sclose(space_id); - VRFY((ret == SUCCEED), "H5Sclose"); - ret = H5Fclose(fid); - VRFY((ret == SUCCEED), "H5Fclose"); - } - - MPI_Barrier(MPI_COMM_WORLD); - - fapl_id = H5Pcreate(H5P_FILE_ACCESS); - VRFY((fapl_id >= 0), "H5Pcreate"); - - ret = H5Pset_fapl_mpio(fapl_id, MPI_COMM_WORLD, MPI_INFO_NULL); - VRFY((ret == SUCCEED), "H5Pset_fapl_mpio"); - - fid = H5Fopen(filename, H5F_ACC_RDWR, fapl_id); - VRFY((fid >= 0), "H5Fopen"); - - data = malloc((dset_dims[0] / (hsize_t)mpi_size) * sizeof(int)); - VRFY(data, "malloc"); - read_data = malloc(dset_dims[0] * sizeof(int)); - VRFY(read_data, "malloc"); - correct_data = malloc(dset_dims[0] * sizeof(int)); - VRFY(correct_data, "malloc"); - - /* - * Check the file space allocation status/size and dataset - * data before and after writing to the dataset without a - * filter - */ - dset_id = H5Dopen2(fid, "dset_no_filter", H5P_DEFAULT); - VRFY((dset_id >= 0), "H5Dopen2"); - - ret = H5Dget_space_status(dset_id, &space_status); - VRFY((ret == SUCCEED), "H5Dread"); - - VRFY((space_status == H5D_SPACE_STATUS_ALLOCATED), "file space allocation status verification succeeded"); - - alloc_size = H5Dget_storage_size(dset_id); - VRFY(((dset_dims[0] * sizeof(int)) == alloc_size), "file space allocation size verification succeeded"); - - memset(read_data, 255, dset_dims[0] * sizeof(int)); - memset(correct_data, 0, dset_dims[0] * sizeof(int)); - - ret = H5Dread(dset_id, H5T_NATIVE_INT, H5S_ALL, H5S_ALL, H5P_DEFAULT, read_data); - VRFY((ret == SUCCEED), "H5Dread"); - - MPI_Barrier(MPI_COMM_WORLD); - - VRFY((0 == memcmp(read_data, correct_data, dset_dims[0] * sizeof(int))), "data verification succeeded"); - - fspace_id = H5Dget_space(dset_id); - VRFY((ret == SUCCEED), "H5Dget_space"); - - start[0] = ((hsize_t)mpi_rank * (dset_dims[0] / (hsize_t)mpi_size)); - stride[0] = 1; - count[0] = (dset_dims[0] / (hsize_t)mpi_size); - block[0] = 1; - - ret = H5Sselect_hyperslab(fspace_id, H5S_SELECT_SET, start, stride, count, block); - VRFY((ret == SUCCEED), "H5Sselect_hyperslab"); - - mem_dims[0] = count[0] * block[0]; - - mspace_id = H5Screate_simple(1, mem_dims, NULL); - VRFY((mspace_id >= 0), "H5Screate_simple"); - - memset(data, 255, (dset_dims[0] / (hsize_t)mpi_size) * sizeof(int)); - - ret = H5Dwrite(dset_id, H5T_NATIVE_INT, mspace_id, fspace_id, H5P_DEFAULT, data); - VRFY((ret == SUCCEED), "H5Dwrite"); - - ret = H5Sclose(mspace_id); - VRFY((ret == SUCCEED), "H5Sclose"); - - MPI_Barrier(MPI_COMM_WORLD); - - ret = H5Dget_space_status(dset_id, &space_status); - VRFY((ret == SUCCEED), "H5Dread"); - - VRFY((space_status == H5D_SPACE_STATUS_ALLOCATED), "file space allocation status verification succeeded"); - - alloc_size = H5Dget_storage_size(dset_id); - VRFY(((dset_dims[0] * sizeof(int)) == alloc_size), "file space allocation size verification succeeded"); - - memset(read_data, 0, dset_dims[0] * sizeof(int)); - memset(correct_data, 255, dset_dims[0] * sizeof(int)); - - ret = H5Dread(dset_id, H5T_NATIVE_INT, H5S_ALL, H5S_ALL, H5P_DEFAULT, read_data); - VRFY((ret == SUCCEED), "H5Dread"); - - MPI_Barrier(MPI_COMM_WORLD); - - VRFY((0 == memcmp(read_data, correct_data, dset_dims[0] * sizeof(int))), "data verification succeeded"); - - ret = H5Sclose(fspace_id); - VRFY((ret == SUCCEED), "H5Sclose"); - ret = H5Dclose(dset_id); - VRFY((ret == SUCCEED), "H5Dclose"); - - /* - * Check the file space allocation status/size and dataset - * data before and after writing to the dataset with a - * filter - */ - dset_id = H5Dopen2(fid, "dset_filter", H5P_DEFAULT); - VRFY((dset_id >= 0), "H5Dopen2"); - - ret = H5Dget_space_status(dset_id, &space_status); - VRFY((ret == SUCCEED), "H5Dread"); - - VRFY((space_status == H5D_SPACE_STATUS_NOT_ALLOCATED), - "file space allocation status verification succeeded"); - - alloc_size = H5Dget_storage_size(dset_id); - VRFY((0 == alloc_size), "file space allocation size verification succeeded"); - - memset(read_data, 255, dset_dims[0] * sizeof(int)); - memset(correct_data, 0, dset_dims[0] * sizeof(int)); - - ret = H5Dread(dset_id, H5T_NATIVE_INT, H5S_ALL, H5S_ALL, H5P_DEFAULT, read_data); - VRFY((ret == SUCCEED), "H5Dread"); - - MPI_Barrier(MPI_COMM_WORLD); - - VRFY((0 == memcmp(read_data, correct_data, dset_dims[0] * sizeof(int))), "data verification succeeded"); - - fspace_id = H5Dget_space(dset_id); - VRFY((ret == SUCCEED), "H5Dget_space"); - - start[0] = ((hsize_t)mpi_rank * (dset_dims[0] / (hsize_t)mpi_size)); - stride[0] = 1; - count[0] = (dset_dims[0] / (hsize_t)mpi_size); - block[0] = 1; - - ret = H5Sselect_hyperslab(fspace_id, H5S_SELECT_SET, start, stride, count, block); - VRFY((ret == SUCCEED), "H5Sselect_hyperslab"); - - mem_dims[0] = count[0] * block[0]; - - mspace_id = H5Screate_simple(1, mem_dims, NULL); - VRFY((mspace_id >= 0), "H5Screate_simple"); - - memset(data, 255, (dset_dims[0] / (hsize_t)mpi_size) * sizeof(int)); - - dxpl_id = H5Pcreate(H5P_DATASET_XFER); - VRFY((dxpl_id >= 0), "H5Pcreate"); - - ret = H5Pset_dxpl_mpio(dxpl_id, H5FD_MPIO_COLLECTIVE); - VRFY((ret == SUCCEED), "H5Pset_dxpl_mpio"); - - ret = H5Dwrite(dset_id, H5T_NATIVE_INT, mspace_id, fspace_id, dxpl_id, data); - VRFY((ret == SUCCEED), "H5Dwrite"); - - ret = H5Sclose(mspace_id); - VRFY((ret == SUCCEED), "H5Sclose"); - - MPI_Barrier(MPI_COMM_WORLD); - - ret = H5Dget_space_status(dset_id, &space_status); - VRFY((ret == SUCCEED), "H5Dread"); - - VRFY((space_status == H5D_SPACE_STATUS_ALLOCATED), "file space allocation status verification succeeded"); - - alloc_size = H5Dget_storage_size(dset_id); - VRFY(((dset_dims[0] * sizeof(int)) == alloc_size), "file space allocation size verification succeeded"); - - memset(read_data, 0, dset_dims[0] * sizeof(int)); - memset(correct_data, 255, dset_dims[0] * sizeof(int)); - - ret = H5Dread(dset_id, H5T_NATIVE_INT, H5S_ALL, H5S_ALL, H5P_DEFAULT, read_data); - VRFY((ret == SUCCEED), "H5Dread"); - - MPI_Barrier(MPI_COMM_WORLD); - - VRFY((0 == memcmp(read_data, correct_data, dset_dims[0] * sizeof(int))), "data verification succeeded"); - - ret = H5Pclose(dxpl_id); - VRFY((ret == SUCCEED), "H5Pclose"); - ret = H5Sclose(fspace_id); - VRFY((ret == SUCCEED), "H5Sclose"); - ret = H5Dclose(dset_id); - VRFY((ret == SUCCEED), "H5Dclose"); - - free(correct_data); - free(read_data); - free(data); - - H5Pclose(fapl_id); - H5Fclose(fid); -} diff --git a/testpar/testphdf5.c b/testpar/testphdf5.c index 8b6b43e0145..be6a8ea604f 100644 --- a/testpar/testphdf5.c +++ b/testpar/testphdf5.c @@ -374,8 +374,6 @@ main(int argc, char **argv) AddTest("selnone", none_selection_chunk, NULL, "chunked dataset with none-selection", PARATESTFILE); AddTest("calloc", test_chunk_alloc, NULL, "parallel extend Chunked allocation on serial file", PARATESTFILE); - AddTest("chkallocser2par", test_chunk_alloc_incr_ser_to_par, NULL, - "chunk allocation from serial to parallel file access", PARATESTFILE); AddTest("fltread", test_filter_read, NULL, "parallel read of dataset written serially with filters", PARATESTFILE); diff --git a/testpar/testphdf5.h b/testpar/testphdf5.h index f7cec0a050a..fac29aaee5d 100644 --- a/testpar/testphdf5.h +++ b/testpar/testphdf5.h @@ -254,7 +254,6 @@ void none_selection_chunk(void); void actual_io_mode_tests(void); void no_collective_cause_tests(void); void test_chunk_alloc(void); -void test_chunk_alloc_incr_ser_to_par(void); void test_filter_read(void); void compact_dataset(void); void null_dataset(void);