Relaxed behavior of H5Pset_page_buffer_size() when opening files

release_docs/RELEASE.txt

@@ -368,6 +368,29 @@ New Features
 
     Library:
     --------
+    - Relaxed behavior of H5Pset_page_buffer_size() when opening files
+
+      This API call sets the size of a file's page buffer cache. This call
+      was extremely strict about matching its parameters to the file strategy
+      and page size used to create the file, requiring a separate open of the
+      file to obtain these parameters.
+
+      These requirements have been relaxed when using the fapl to open
+      a previously-created file:
+
+      * When opening a file that does not use the H5F_FSPACE_STRATEGY_PAGE
+        strategy, the setting is ignored and the file will be opened, but
+        without a page buffer cache. This was previously an error.
+
+      * When opening a file that has a page size larger than the desired
+        page buffer cache size, the page buffer cache size will be increased
+        to the file's page size. This was previously an error.
+
+      The behavior when creating a file using H5Pset_page_buffer_size() is
+      unchanged.
+
+      Fixes GitHub issue #3382
+
     - Added support for _Float16 16-bit half-precision floating-point datatype
 
       Support for the _Float16 C datatype has been added on platforms where:

src/H5Fint.c

@@ -2073,7 +2073,21 @@ H5F_open(const char *name, unsigned flags, hid_t fcpl_id, hid_t fapl_id)
         if (H5F__super_read(file, a_plist, true) < 0)
             HGOTO_ERROR(H5E_FILE, H5E_READERROR, NULL, "unable to read superblock");
 
-        /* Create the page buffer before initializing the superblock */
+        /* Skip trying to create a page buffer if the file space strategy
+         * stored in the superblock isn't paged.
+         */
+        if (shared->fs_strategy != H5F_FSPACE_STRATEGY_PAGE)
+            page_buf_size = 0;
+
+        /* If the page buffer is enabled, the strategy is paged, and the size in
+         * the fapl is smaller than the file's page size, bump the page buffer
+         * size up to the file's page size.
+         */
+        if (page_buf_size > 0 && shared->fs_strategy == H5F_FSPACE_STRATEGY_PAGE &&
+            shared->fs_page_size > page_buf_size)
+            page_buf_size = shared->fs_page_size;
+
+        /* Create the page buffer *after* reading the superblock */
         if (page_buf_size)
             if (H5PB_create(shared, page_buf_size, page_buf_min_meta_perc, page_buf_min_raw_perc) < 0)
                 HGOTO_ERROR(H5E_FILE, H5E_CANTINIT, NULL, "unable to create page buffer");

src/H5Ppublic.h

@@ -5748,7 +5748,16 @@ H5_DLL herr_t H5Pset_mdc_image_config(hid_t plist_id, H5AC_cache_image_config_t
  *          If a non-zero page buffer size is set, and the file space strategy
  *          is not set to paged or the page size for the file space strategy is
  *          larger than the page buffer size, the subsequent call to H5Fcreate()
- *          or H5Fopen() using the \p plist_id will fail.
+ *          using the \p plist_id will fail.
+ *
+ * \note    As of HDF5 1.14.4, this property will be ignored when an existing
+ *          file is being opened and the file space strategy stored in the
+ *          file isn't paged. This was previously a failure.
+ *
+ * \note    As of HDF5 1.14.4, if a file with a paged file space strategy is
+ *          opened with a page size that is smaller than the file's page size,
+ *          the page cache size will be rounded up to the file's page size.
+ *          This was previously a failure.
  *
  *          The function also allows setting the minimum percentage of pages for
  *          metadata and raw data to prevent a certain type of data to evict hot

test/page_buffer.c

@@ -1650,6 +1650,114 @@ test_min_threshold(hid_t orig_fapl, const char *driver_name)
 
 } /* test_min_threshold */
 
+/*-------------------------------------------------------------------------
+ * Function:    test_pb_fapl_tolerance_at_open()
+ *
+ * Purpose:     Tests if the library tolerates setting fapl page buffer
+ *              values via H5Pset_page_buffer_size() when opening a file
+ *              that does not use page buffering or has a size smaller
+ *              than the file's page size.
+ *
+ *              As of HDF5 1.14.4, these should succeed.
+ *
+ * Return:      0 if test is successful
+ *              1 if test fails
+ *
+ *-------------------------------------------------------------------------
+ */
+static unsigned
+test_pb_fapl_tolerance_at_open(void)
+{
+    const char *filename = "pb_fapl_tolerance.h5";
+    hid_t       fapl     = H5I_INVALID_HID;
+    hid_t       fcpl     = H5I_INVALID_HID;
+    hid_t       fid      = H5I_INVALID_HID;
+    H5F_t      *f        = NULL;
+
+    TESTING("if opening non-page-buffered files works w/ H5Pset_page_buffer_size()");
+
+    /* Create a file WITHOUT page buffering */
+    if ((fid = H5Fcreate(filename, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT)) < 0)
+        TEST_ERROR;
+    if (H5Fclose(fid) < 0)
+        TEST_ERROR;
+
+    /* Set up page buffering values on a fapl */
+    if ((fapl = H5Pcreate(H5P_FILE_ACCESS)) < 0)
+        TEST_ERROR;
+    if (H5Pset_page_buffer_size(fapl, 512, 0, 0) < 0)
+        TEST_ERROR;
+
+    /* Attempt to open non-page-buf file w/ page buf fapl. Should succeed,
+     * but without a page buffer.
+     */
+    if ((fid = H5Fopen(filename, H5F_ACC_RDWR, fapl)) < 0)
+        TEST_ERROR;
+    if (NULL == (f = (H5F_t *)H5VL_object(fid)))
+        TEST_ERROR;
+    if (f->shared->fs_strategy == H5F_FSPACE_STRATEGY_PAGE)
+        TEST_ERROR;
+    if (f->shared->page_buf != NULL)
+        TEST_ERROR;
+    if (H5Fclose(fid) < 0)
+        TEST_ERROR;
+
+    /* Set up a fcpl with a page size that is larger than the fapl size */
+    if ((fcpl = H5Pcreate(H5P_FILE_CREATE)) < 0)
+        TEST_ERROR;
+    if (H5Pset_file_space_strategy(fcpl, H5F_FSPACE_STRATEGY_PAGE, false, 1) < 0)
+        TEST_ERROR;
+    if (H5Pset_file_space_page_size(fcpl, 4096) < 0)
+        TEST_ERROR;
+
+    /* Create a file that uses page buffering with a larger page size */
+    if ((fid = H5Fcreate(filename, H5F_ACC_TRUNC, fcpl, H5P_DEFAULT)) < 0)
+        TEST_ERROR;
+    if (H5Fclose(fid) < 0)
+        TEST_ERROR;
+
+    /* Attempt to open page-buf file w/ fapl page buf size that is too small.
+     * Should succeed with a page buffer size that matches the file's page size.
+     */
+    if ((fid = H5Fopen(filename, H5F_ACC_RDWR, fapl)) < 0)
+        TEST_ERROR;
+    if (NULL == (f = (H5F_t *)H5VL_object(fid)))
+        TEST_ERROR;
+    if (f->shared->fs_strategy != H5F_FSPACE_STRATEGY_PAGE)
+        TEST_ERROR;
+    if (f->shared->page_buf == NULL)
+        TEST_ERROR;
+    if (f->shared->fs_page_size != 4096)
+        TEST_ERROR;
+    if (H5Fclose(fid) < 0)
+        TEST_ERROR;
+
+    /* Shut down */
+    if (H5Pclose(fcpl) < 0)
+        TEST_ERROR;
+    if (H5Pclose(fapl) < 0)
+        TEST_ERROR;
+
+    HDremove(filename);
+
+    PASSED();
+
+    return 0;
+
+error:
+
+    H5E_BEGIN_TRY
+    {
+        H5Pclose(fapl);
+        H5Pclose(fcpl);
+        H5Fclose(fid);
+    }
+    H5E_END_TRY
+
+    return 1;
+
+} /* test_pb_fapl_tolerance_at_open */
+
 /*-------------------------------------------------------------------------
  * Function:    test_stats_collection()
  *
@@ -2083,12 +2191,12 @@ main(void)
         SKIPPED();
         puts("Skip page buffering test because paged aggregation is disabled for multi/split drivers");
         exit(EXIT_SUCCESS);
-    } /* end if */
+    }
 
     if ((fapl = h5_fileaccess()) < 0) {
         nerrors++;
         PUTS_ERROR("Can't get VFD-dependent fapl");
-    } /* end if */
+    }
 
     /* Push API context */
     if (H5CX_push() < 0)
@@ -2107,6 +2215,7 @@ main(void)
     nerrors += test_lru_processing(fapl, driver_name);
     nerrors += test_min_threshold(fapl, driver_name);
     nerrors += test_stats_collection(fapl, driver_name);
+    nerrors += test_pb_fapl_tolerance_at_open();
 
 #endif /* H5_HAVE_PARALLEL */