Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip vlen IO API test for cache VOL #4135

Merged
merged 4 commits into from
Mar 20, 2024
Merged

Skip vlen IO API test for cache VOL #4135

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a78ebac
Fix cache VOL segfault in vlen io test
mattjala Mar 14, 2024
ef5be54
Skip vlen IO API test
mattjala Mar 15, 2024
c10fdef
Move skip to correct test
mattjala Mar 18, 2024
67fd159
Committing clang-format changes
github-actions[bot] Mar 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 57 additions & 52 deletions test/API/H5_api_dataset_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9868,6 +9868,10 @@ test_dataset_vlen_io(void)
return 0;
}

/* Skipped for now due to segfault with the Cache VOL */
SKIPPED();
return 0;

TESTING_2("test setup");

if ((file_id = H5Fopen(H5_api_test_filename, H5F_ACC_RDWR, H5P_DEFAULT)) < 0) {
Expand Down Expand Up @@ -9955,9 +9959,6 @@ test_dataset_vlen_io(void)

dset_string = H5I_INVALID_HID;

memset(wbuf, 0, sizeof(hvl_t) * DATASET_VLEN_IO_DSET_DIMS);
memset(rbuf, 0, sizeof(hvl_t) * DATASET_VLEN_IO_DSET_DIMS);

PASSED();

BEGIN_MULTIPART
Expand Down Expand Up @@ -9985,6 +9986,9 @@ test_dataset_vlen_io(void)
if ((H5Dwrite(dset_int, vlen_int, space_id, H5S_ALL, H5P_DEFAULT, (const void *)wbuf)) < 0)
PART_TEST_ERROR(rw_all_int);

if (H5Dflush(dset_int) < 0)
PART_TEST_ERROR(rw_hyperslab_selection);

/* Close and reopen file objects to flush cache */
if (H5Dclose(dset_int) < 0)
PART_TEST_ERROR(rw_all_int);
Expand Down Expand Up @@ -10014,6 +10018,11 @@ test_dataset_vlen_io(void)
if ((H5Dread(dset_int, vlen_int, space_id, H5S_ALL, H5P_DEFAULT, (void *)rbuf)) < 0)
PART_TEST_ERROR(rw_all_int);

/* Close to finalize read */
if (H5Dclose(dset_int) < 0)
PART_TEST_ERROR(rw_all_int);
dset_int = H5I_INVALID_HID;

/* Verify data */
for (size_t i = 0; i < DATASET_VLEN_IO_DSET_DIMS; i++) {
if (!rbuf[i].p)
Expand All @@ -10027,12 +10036,6 @@ test_dataset_vlen_io(void)
PART_TEST_ERROR(rw_all_int);
}

/* Close before use in selection tests */
if (H5Dclose(dset_int) < 0)
PART_TEST_ERROR(rw_all_int);

dset_int = H5I_INVALID_HID;

/* Reset buffers */
if (H5Treclaim(vlen_int, space_id, H5P_DEFAULT, rbuf) < 0)
PART_TEST_ERROR(rw_all_int);
Expand Down Expand Up @@ -10069,6 +10072,9 @@ test_dataset_vlen_io(void)
if ((H5Dwrite(dset_float, vlen_float, space_id, H5S_ALL, H5P_DEFAULT, (const void *)wbuf)) < 0)
PART_TEST_ERROR(rw_all_float);

if (H5Dflush(dset_float) < 0)
PART_TEST_ERROR(rw_hyperslab_selection);

/* Close and reopen file objects to flush cache */
if (H5Dclose(dset_float) < 0)
PART_TEST_ERROR(rw_all_float);
Expand Down Expand Up @@ -10098,6 +10104,11 @@ test_dataset_vlen_io(void)
if ((H5Dread(dset_float, vlen_float, space_id, H5S_ALL, H5P_DEFAULT, (void *)rbuf)) < 0)
PART_TEST_ERROR(rw_all_float);

/* Close to finalize read */
if (H5Dclose(dset_float) < 0)
PART_TEST_ERROR(rw_all_float);
dset_float = H5I_INVALID_HID;

/* Verify data */
for (size_t i = 0; i < DATASET_VLEN_IO_DSET_DIMS; i++) {
if (!rbuf[i].p)
Expand All @@ -10116,7 +10127,6 @@ test_dataset_vlen_io(void)
}

/* Reset buffers */

if (H5Treclaim(vlen_float, space_id, H5P_DEFAULT, rbuf) < 0)
PART_TEST_ERROR(rw_all_float);

Expand Down Expand Up @@ -10155,6 +10165,9 @@ test_dataset_vlen_io(void)
if ((H5Dwrite(dset_string, vlen_string, space_id, H5S_ALL, H5P_DEFAULT, (const void *)wbuf)) < 0)
PART_TEST_ERROR(rw_all_string);

if (H5Dflush(dset_string) < 0)
PART_TEST_ERROR(rw_hyperslab_selection);

/* Close and reopen file objects to flush cache */
if (H5Dclose(dset_string) < 0)
PART_TEST_ERROR(rw_all_string);
Expand Down Expand Up @@ -10185,6 +10198,11 @@ test_dataset_vlen_io(void)
if ((H5Dread(dset_string, vlen_string, space_id, H5S_ALL, H5P_DEFAULT, (void *)rbuf)) < 0)
PART_TEST_ERROR(rw_all_string);

/* Close to finalize read */
if (H5Dclose(dset_string) < 0)
PART_TEST_ERROR(rw_all_string);
dset_string = H5I_INVALID_HID;

/* Verify data */
for (size_t i = 0; i < DATASET_VLEN_IO_DSET_DIMS; i++) {
if (!rbuf[i].p)
Expand All @@ -10205,7 +10223,6 @@ test_dataset_vlen_io(void)
}

/* Reset buffers */

if (H5Treclaim(vlen_string, space_id, H5P_DEFAULT, rbuf) < 0)
PART_TEST_ERROR(rw_all_string);

Expand Down Expand Up @@ -10251,6 +10268,9 @@ test_dataset_vlen_io(void)
if ((H5Dwrite(dset_int, vlen_int, space_id, space_id, H5P_DEFAULT, (const void *)wbuf)) < 0)
PART_TEST_ERROR(rw_point_selection);

if (H5Dflush(dset_int) < 0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I'm surprised adding a flush here would fix the problem since I'd assume that would just move the segfault to here instead. Plus, H5Dflush is mostly used for SWMR and I wouldn't think it would be needed here. Since the native VOL was passing without this call, I wonder if this is either a problem in the cache VOL or a problem in the VOL interface instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The segfault seems to be due to the cache vol still using the dynamically allocated heap buffers when they're freed after the reads/writes. As it turns out, this doesn't resolve the issue either - it cropped up again in the CI here. The cache VOL docs say that H5Dwrite/H5Dclose should be semi-blocking, so this may be an issue on the cache VOL.

PART_TEST_ERROR(rw_hyperslab_selection);

/* Close and reopen file objects to flush cache */
if (H5Dclose(dset_int) < 0)
PART_TEST_ERROR(rw_point_selection);
Expand All @@ -10277,6 +10297,11 @@ test_dataset_vlen_io(void)
if ((H5Dread(dset_int, vlen_int, space_id, space_id, H5P_DEFAULT, (void *)rbuf)) < 0)
PART_TEST_ERROR(rw_point_selection);

/* Close to finalize read */
if (H5Dclose(dset_int) < 0)
PART_TEST_ERROR(rw_point_selection);
dset_int = H5I_INVALID_HID;

/* Verify data */
for (size_t i = 0; i < DATASET_VLEN_IO_DSET_DIMS; i++) {
if (i % 2 == 0) {
Expand All @@ -10299,10 +10324,6 @@ test_dataset_vlen_io(void)
}
}

if (H5Dclose(dset_int) < 0)
PART_TEST_ERROR(rw_point_selection);
dset_int = H5I_INVALID_HID;

/* Reset buffers */
if (H5Treclaim(vlen_int, space_id, H5P_DEFAULT, rbuf) < 0)
PART_TEST_ERROR(rw_point_selection);
Expand Down Expand Up @@ -10349,6 +10370,9 @@ test_dataset_vlen_io(void)
if ((H5Dwrite(dset_int, vlen_int, space_id, space_id, H5P_DEFAULT, (const void *)wbuf)) < 0)
PART_TEST_ERROR(rw_hyperslab_selection);

if (H5Dflush(dset_int) < 0)
PART_TEST_ERROR(rw_hyperslab_selection);

/* Close and reopen file objects to flush cache */
if (H5Dclose(dset_int) < 0)
PART_TEST_ERROR(rw_hyperslab_selection);
Expand All @@ -10375,6 +10399,11 @@ test_dataset_vlen_io(void)
if ((H5Dread(dset_int, vlen_int, space_id, space_id, H5P_DEFAULT, (void *)rbuf)) < 0)
PART_TEST_ERROR(rw_hyperslab_selection);

/* Close to finalize read */
if (H5Dclose(dset_int) < 0)
PART_TEST_ERROR(rw_hyperslab_selection);
dset_int = H5I_INVALID_HID;

/* Verify data */
for (size_t i = 0; i < DATASET_VLEN_IO_DSET_DIMS; i++) {
if (i % stride[0] == 0) {
Expand Down Expand Up @@ -10415,57 +10444,30 @@ test_dataset_vlen_io(void)

TESTING_2("test cleanup");

if (H5Fclose(file_id) < 0)
TEST_ERROR;
if (H5Gclose(container_group) < 0)
TEST_ERROR;
if (H5Dclose(dset_int) < 0)
TEST_ERROR;
if (H5Dclose(dset_float) < 0)
TEST_ERROR;
if (H5Dclose(dset_string) < 0)
TEST_ERROR;
if (H5Sclose(space_id) < 0)
TEST_ERROR;

/* In case of memory allocation error, not all hvl_t buffers in array may be allocated.
* Free one-by-one */
for (size_t i = 0; i < DATASET_VLEN_IO_DSET_DIMS; i++) {
if (wbuf[i].p) {
free(wbuf[i].p);
wbuf[i].p = NULL;
}
}

for (size_t i = 0; i < DATASET_VLEN_IO_DSET_DIMS; i++) {
if (rbuf[i].p) {
free(rbuf[i].p);
rbuf[i].p = NULL;
}
}

if (H5Tclose(vlen_int) < 0)
TEST_ERROR;
if (H5Tclose(vlen_float) < 0)
TEST_ERROR;
if (H5Tclose(vlen_string) < 0)
TEST_ERROR;

if (H5Gclose(container_group) < 0)
TEST_ERROR;
if (H5Fclose(file_id) < 0)
TEST_ERROR;
PASSED();
return 0;
error:

H5E_BEGIN_TRY
{
H5Fclose(file_id);
H5Gclose(container_group);
if (dset_int != H5I_INVALID_HID)
H5Dclose(dset_int);
if (dset_float != H5I_INVALID_HID)
H5Dclose(dset_float);
if (dset_string != H5I_INVALID_HID)
H5Dclose(dset_string);
H5Dclose(dset_int);
H5Dclose(dset_float);
H5Dclose(dset_string);
H5Sclose(space_id);
/* In case of memory allocation error, not all hvl_t buffers in array may be allocated.
* Free one-by-one */
for (size_t i = 0; i < DATASET_VLEN_IO_DSET_DIMS; i++) {
if (wbuf[i].p) {
free(wbuf[i].p);
Expand All @@ -10482,6 +10484,8 @@ test_dataset_vlen_io(void)
H5Tclose(vlen_int);
H5Tclose(vlen_float);
H5Tclose(vlen_string);
H5Gclose(container_group);
H5Fclose(file_id);
}
H5E_END_TRY

Expand Down Expand Up @@ -10513,7 +10517,8 @@ test_dataset_set_extent_chunked_unlimited(void)

/* Make sure the connector supports the API functions being tested */
if (!(vol_cap_flags_g & H5VL_CAP_FLAG_FILE_BASIC) || !(vol_cap_flags_g & H5VL_CAP_FLAG_GROUP_BASIC) ||
!(vol_cap_flags_g & H5VL_CAP_FLAG_DATASET_BASIC) || !(vol_cap_flags_g & H5VL_CAP_FLAG_DATASET_MORE)) {
!(vol_cap_flags_g & H5VL_CAP_FLAG_DATASET_BASIC) || !(vol_cap_flags_g & H5VL_CAP_FLAG_DATASET_MORE) ||
!(vol_cap_flags_g & H5VL_CAP_FLAG_DATASET_MORE)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate check for H5VL_CAP_FLAG_DATASET_MORE flag here

SKIPPED();
printf(" API functions for basic file, group, basic or more dataset aren't supported with this "
"connector\n");
Expand Down