Merge pull request #1717 from NOAA-GSD/ejh_cache

now properly setting HDF5 file cache for files created/opened sequentially on parallel I/O builds

RELEASE_NOTES.md

@@ -7,6 +7,7 @@ This file contains a high-level description of this package's evolution. Release
 
 ## 4.8.0 - TBD
 
+* [Bug Fix] Now larger cache settings are used for sequential HDF5 file creates/opens on parallel I/O capable builds; see [Github #1716](https://github.com/Unidata/netcdf-c/issues/1716) for more information.
 * [Bug Fix] Add functions to libdispatch/dnotnc4.c to support
 dispatch table operations that should work for any dispatch
 table, even if they do not do anything; functions such as

include/hdf5internal.h

@@ -177,6 +177,9 @@ int nc4_HDF5_close_att(NC_ATT_INFO_T *att);
 /* Perform lazy read of the rest of the metadata for a var. */
 int nc4_get_var_meta(NC_VAR_INFO_T *var);
 
+/* Get the file chunk cache settings from HDF5. */
+int nc4_hdf5_get_chunk_cache(int ncid, size_t *sizep, size_t *nelemsp,
+			     float *preemptionp);
 
 /* Define Filter API Function */
 int nc4_global_filter_action(int action, unsigned int id, struct NC_FILTER_OBJ_HDF5* infop);

libhdf5/hdf5cache.c

@@ -80,8 +80,10 @@ nc_set_chunk_cache(size_t size, size_t nelems, float preemption)
 }
 
 /**
- * Get current chunk cache settings. These settings may be changed
- * with nc_set_chunk_cache().
+ * Get current netCDF chunk cache settings. These settings may be
+ * changed with nc_set_chunk_cache(). This function does not get the
+ * settings from HDF5, it simply reports the current netCDF chunk
+ * cache settings.
  *
  * @param sizep Pointer that gets size in bytes to set cache. Ignored
  * if NULL.

libhdf5/hdf5create.c

@@ -147,14 +147,18 @@ nc4_create_file(const char *path, int cmode, size_t initialsz,
             nc4_info->info = info;
         }
     }
-#else /* only set cache for non-parallel... */
+#endif /* USE_PARALLEL4 */
+
+    /* Only set cache for non-parallel creates. */
+    if (!nc4_info->parallel)
+    {
 	if (H5Pset_cache(fapl_id, 0, nc4_chunk_cache_nelems, nc4_chunk_cache_size,
 			 nc4_chunk_cache_preemption) < 0)
 	    BAIL(NC_EHDFERR);
 	LOG((4, "%s: set HDF raw chunk cache to size %d nelems %d preemption %f",
 	     __func__, nc4_chunk_cache_size, nc4_chunk_cache_nelems,
 	     nc4_chunk_cache_preemption));
-#endif /* USE_PARALLEL4 */
+    }
 
 #ifdef HAVE_H5PSET_LIBVER_BOUNDS
 #if H5_VERSION_GE(1,10,2)

libhdf5/hdf5internal.c

@@ -948,6 +948,49 @@ nc4_hdf5_find_grp_var_att(int ncid, int varid, const char *name, int attnum,
     return NC_NOERR;
 }
 
+/**
+ * @internal Get the file chunk cache settings from HDF5.
+ *
+ * @param ncid File ID of a NetCDF/HDF5 file.
+ * @param sizep Pointer that gets size in bytes to set cache. Ignored
+ * if NULL.
+ * @param nelemsp Pointer that gets number of elements to hold in
+ * cache. Ignored if NULL.
+ * @param preemptionp Pointer that gets preemption stragety (between 0
+ * and 1). Ignored if NULL.
+ *
+ * @return ::NC_NOERR No error.
+ * @author Ed Hartnett
+ */
+int
+nc4_hdf5_get_chunk_cache(int ncid, size_t *sizep, size_t *nelemsp,
+		     float *preemptionp)
+{
+    NC_FILE_INFO_T *h5;
+    NC_HDF5_FILE_INFO_T *hdf5_info;
+    hid_t plistid;
+    double dpreemption;
+    int retval;
+    
+    /* Find info for this file, group, and h5 info. */
+    if ((retval = nc4_find_nc_grp_h5(ncid, NULL, NULL, &h5)))
+        return retval;
+    assert(h5 && h5->format_file_info);
+    hdf5_info = (NC_HDF5_FILE_INFO_T *)h5->format_file_info;
+
+    /* Get the file access property list. */
+    if ((plistid = H5Fget_access_plist(hdf5_info->hdfid)) < 0)
+	return NC_EHDFERR;
+
+    /* Get the chunk cache values from HDF5 for this property list. */
+    if (H5Pget_cache(plistid, NULL, nelemsp, sizep, &dpreemption) < 0)
+	return NC_EHDFERR;
+    if (preemptionp)
+	*preemptionp = dpreemption;
+
+    return NC_NOERR;
+}
+
 #ifdef LOGGING
 /* We will need to check against nc log level from nc4internal.c. */
 extern int nc_log_level;

libhdf5/hdf5open.c

@@ -745,7 +745,7 @@ nc4_open_file(const char *path, int mode, void* parameters, int ncid)
         BAIL(NC_EINTERNAL);
 
 #ifdef USE_PARALLEL4
-    mpiinfo = (NC_MPI_INFO*)parameters; /* assume, may be changed if inmemory is true */
+    mpiinfo = (NC_MPI_INFO *)parameters; /* assume, may be changed if inmemory is true */
 #endif /* !USE_PARALLEL4 */
 
     /* Need this access plist to control how HDF5 handles open objects
@@ -760,20 +760,19 @@ nc4_open_file(const char *path, int mode, void* parameters, int ncid)
 #ifdef USE_PARALLEL4
     if (!(mode & (NC_INMEMORY | NC_DISKLESS)) && mpiinfo != NULL) {
         /* If this is a parallel file create, set up the file creation
-         * property list.
-         */
+         * property list. */
         nc4_info->parallel = NC_TRUE;
         LOG((4, "opening parallel file with MPI/IO"));
         if (H5Pset_fapl_mpio(fapl_id, mpiinfo->comm, mpiinfo->info) < 0)
             BAIL(NC_EPARINIT);
 
         /* Keep copies of the MPI Comm & Info objects */
-        if (MPI_SUCCESS != MPI_Comm_dup(mpiinfo->comm, &nc4_info->comm))
+        if (MPI_Comm_dup(mpiinfo->comm, &nc4_info->comm) != MPI_SUCCESS)
             BAIL(NC_EMPI);
         comm_duped++;
-        if (MPI_INFO_NULL != mpiinfo->info)
+        if (mpiinfo->info != MPI_INFO_NULL)
         {
-            if (MPI_SUCCESS != MPI_Info_dup(mpiinfo->info, &nc4_info->info))
+            if (MPI_Info_dup(mpiinfo->info, &nc4_info->info) != MPI_SUCCESS)
                 BAIL(NC_EMPI);
             info_duped++;
         }
@@ -785,18 +784,23 @@ nc4_open_file(const char *path, int mode, void* parameters, int ncid)
     }
 
 #ifdef HDF5_HAS_COLL_METADATA_OPS
+    /* If collective metadata operations are available in HDF5, turn
+     * them on. */
     if (H5Pset_all_coll_metadata_ops(fapl_id, 1) < 0)
         BAIL(NC_EPARINIT);
-#endif
+#endif /* HDF5_HAS_COLL_METADATA_OPS */
+#endif /* USE_PARALLEL4 */
 
-#else /* only set cache for non-parallel. */
+    /* Only set cache for non-parallel opens. */
+    if (!nc4_info->parallel)
+    {
 	if (H5Pset_cache(fapl_id, 0, nc4_chunk_cache_nelems, nc4_chunk_cache_size,
 			 nc4_chunk_cache_preemption) < 0)
 	    BAIL(NC_EHDFERR);
 	LOG((4, "%s: set HDF raw chunk cache to size %d nelems %d preemption %f",
 	     __func__, nc4_chunk_cache_size, nc4_chunk_cache_nelems,
 	     nc4_chunk_cache_preemption));
-#endif /* USE_PARALLEL4 */
+    }
 
     /* Process  NC_INMEMORY */
     if(nc4_info->mem.inmemory) {

nc_test4/tst_parallel5.c

@@ -19,6 +19,13 @@
 #define NUM_PROC 4
 #define NUM_SLABS 10
 #define NUM_ACCESS_TESTS 2
+#define HDF5_DEFAULT_CACHE_SIZE 1048576
+#define HDF5_DEFAULT_NELEMS 521
+#define HDF5_DEFAULT_PREEMPTION 0.75
+
+int
+nc4_hdf5_get_chunk_cache(int ncid, size_t *sizep, size_t *nelemsp,
+                         float *preemptionp);
 
 int
 main(int argc, char **argv)
@@ -297,7 +304,7 @@ main(int argc, char **argv)
         /* Crate a file with a scalar NC_BYTE value. */
         if (nc_create_par(FILE, NC_NETCDF4, MPI_COMM_WORLD, MPI_INFO_NULL,
                           &ncid)) ERR;
-        if (nc_def_var(ncid, "fred", NC_BYTE, 0, NULL, &varid)) ERR;
+        if (nc_def_var(ncid, VAR_NAME, NC_BYTE, 0, NULL, &varid)) ERR;
         if (nc_enddef(ncid)) ERR;
         if (nc_put_var_schar(ncid, varid, &test_data)) ERR;
         if (nc_close(ncid)) ERR;
@@ -310,6 +317,62 @@ main(int argc, char **argv)
     }
     if (!mpi_rank)
         SUMMARIZE_ERR;
+    if (!mpi_rank)
+        printf("*** testing cache settings for sequentially-opened files...");
+    {
+        /* This test is related to
+         * https://github.com/Unidata/netcdf-c/issues/1715. */
+        int ncid, varid;
+        int test_data_in, test_data = 42;
+        size_t size, nelems;
+        float preemption;
+
+        /* Create a file with parallel I/O and check cache settings. */
+        if (nc_create_par(FILE, NC_NETCDF4|NC_CLOBBER, MPI_COMM_WORLD, MPI_INFO_NULL,
+                          &ncid)) ERR;
+        if (nc4_hdf5_get_chunk_cache(ncid, &size, &nelems, &preemption)) ERR;
+        if (size != HDF5_DEFAULT_CACHE_SIZE || nelems != HDF5_DEFAULT_NELEMS ||
+            preemption != HDF5_DEFAULT_PREEMPTION) ERR;
+        /* printf("%ld %ld %g\n", size, nelems, preemption); */
+        if (nc_close(ncid)) ERR;
+
+        /* Create a file with sequential I/O and check cache settings
+         * on processor 0. Now, instead of being set to the HDF5
+         * defaults, the chunk settings are set to the netCDF
+         * defaults. */
+        if (!mpi_rank)
+        {
+            if (nc_create(FILE, NC_NETCDF4|NC_CLOBBER, &ncid)) ERR;
+            if (nc4_hdf5_get_chunk_cache(ncid, &size, &nelems, &preemption)) ERR;
+            /* printf("%ld %ld %g\n", size, nelems, preemption); */
+            if (size != CHUNK_CACHE_SIZE || nelems != CHUNK_CACHE_NELEMS ||
+                preemption != CHUNK_CACHE_PREEMPTION) ERR;
+            if (nc_close(ncid)) ERR;
+        }
+
+        /* Reopen the file and check. */
+        if (nc_open_par(FILE, 0, comm, info, &ncid)) ERR;
+        if (nc4_hdf5_get_chunk_cache(ncid, &size, &nelems, &preemption)) ERR;
+        if (size != HDF5_DEFAULT_CACHE_SIZE || nelems != HDF5_DEFAULT_NELEMS ||
+            preemption != HDF5_DEFAULT_PREEMPTION) ERR;
+        if (nc_close(ncid)) ERR;
+
+        /* Open the file with sequential I/O and check cache settings
+         * on processor 0. Now, instead of being set to the HDF5
+         * defaults, the chunk settings are set to the netCDF
+         * defaults. */
+        if (!mpi_rank)
+        {
+            if (nc_open(FILE, 0, &ncid)) ERR;
+            if (nc4_hdf5_get_chunk_cache(ncid, &size, &nelems, &preemption)) ERR;
+            if (size != CHUNK_CACHE_SIZE || nelems != CHUNK_CACHE_NELEMS ||
+                preemption != CHUNK_CACHE_PREEMPTION) ERR;
+            if (nc_close(ncid)) ERR;
+        }
+        nc_set_log_level(0);
+    }
+    if (!mpi_rank)
+        SUMMARIZE_ERR;
 #ifdef USE_SZIP
 #ifdef HDF5_SUPPORTS_PAR_FILTERS
 #define SZIP_DIM_LEN 256