Relaxed behavior of H5Pset_page_buffer_size() when opening files (HDF…

…Group#4280)

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 HDFGroup#3382

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 */