Onion VFD: revisions silently lost when the file is small relative to page_size
- Dominant language
- C
- Stars
- 988
- Forks
- 355
- Avg merge
- 4d 2h
- Merged PRs (30d)
- 12
Description
### Summary
With the onion VFD, a revision history written to a *small* file is silently incomplete when `page_size` is 512 or larger: some revisions are lost, and `H5FDonion_get_revision_count()` still reports the full count. No error is raised at any point.
### Reproducer
```c
/* Onion VFD: revisions are silently lost when the file is small relative to
* page_size.
*
* Build: h5cc -o onion_lost_revisions onion_lost_revisions.c
* Run: ./onion_lost_revisions 256 -> all three revisions correct
* ./onion_lost_revisions 512 -> revision 1 empty, revision 3 == revision 2
*
* Three write sessions against one canonical file, each replacing a single
* scalar dataset /a with a new value. Each session is one open/close cycle,
* so the history should hold three revisions with /a == 10, 11, 12.
*/
#include
#include
#include
#include
#include "hdf5.h"
#include "H5FDonion.h"
#define FNAME "onion_lost_revisions.h5"
#define NREV 3
static hid_t
onion_fapl(uint64_t revision, uint32_t page_size)
{
H5FD_onion_fapl_info_t info;
hid_t fapl;
memset(&info, 0, sizeof(info));
info.version = H5FD_ONION_FAPL_INFO_VERSION_CURR;
info.backing_fapl_id = H5Pcreate(H5P_FILE_ACCESS);
info.page_size = page_size;
info.store_target = H5FD_ONION_STORE_TARGET_ONION;
info.revision_num = revision;
strcpy(info.comment, "repro");
if ((fapl = H5Pcreate(H5P_FILE_ACCESS)) < 0)
return H5I_INVALID_HID;
if (H5Pset_fapl_onion(fapl, &info) < 0) {
H5Pclose(fapl);
return H5I_INVALID_HID;
}
return fapl;
}
int
main(int argc, char **argv)
{
uint32_t page = (argc > 1) ? (uint32_t)strtoul(argv[1], NULL, 10) : 512;
hid_t fid, fapl, sp, ds;
int i, bad = 0;
unlink(FNAME);
unlink(FNAME ".onion");
unlink(FNAME ".onion.recovery");
/* Canonical file first, plainly, as test/onion.c does. */
if ((fid = H5Fcreate(FNAME, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT)) < 0)
return 1;
H5Fclose(fid);
/* Three write sessions == three revisions. */
for (i = 0; i < NREV; i++) {
int v = 10 + i;
if ((fapl = onion_fapl(H5FD_ONION_FAPL_INFO_REVISION_ID_LATEST, page)) < 0)
return 1;
if ((fid = H5Fopen(FNAME, H5F_ACC_RDWR, fapl)) < 0) {
printf("session %d: open failed\n", i);
return 1;
}
H5E_BEGIN_TRY
{
H5Ldelete(fid, "/a", H5P_DEFAULT);
}
H5E_END_TRY
sp = H5Screate(H5S_SCALAR);
ds = H5Dcreate2(fid, "/a", H5T_NATIVE_INT, sp, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
H5Dwrite(ds, H5T_NATIVE_INT, H5S_ALL, H5S_ALL, H5P_DEFAULT, &v);
H5Dclose(ds);
H5Sclose(sp);
H5Fclose(fid);
H5Pclose(fapl);
}
{
hsize_t nrev = 0;
hid_t cf = onion_fapl(H5FD_ONION_FAPL_INFO_REVISION_ID_LATEST, page);
if (cf >= 0 && H5FDonion_get_revision_count(FNAME, cf, &nrev) >= 0)
printf("page_size %u: H5FDonion_get_revision_count() = %llu\n", page,
(unsigned long long)nrev);
if (cf >= 0)
H5Pclose(cf);
}
/* Revision i+1 should hold /a == 10 + i. */
for (i = 0; i < NREV; i++) {
int v = -1;
if ((fapl = onion_fapl((uint64_t)(i + 1), page)) < 0)
return 1;
H5E_BEGIN_TRY
{
fid = H5Fopen(FNAME, H5F_ACC_RDONLY, fapl);
}
H5E_END_TRY
if (fid < 0) {
printf(" revision %d: cannot open\n", i + 1);
bad = 1;
H5Pclose(fapl);
continue;
}
H5E_BEGIN_TRY
{
ds = H5Dopen2(fid, "/a", H5P_DEFAULT);
}
H5E_END_TRY
if (ds >= 0) {
H5Dread(ds, H5T_NATIVE_INT, H5S_ALL, H5S_ALL, H5P_DEFAULT, &v);
H5Dclose(ds);
}
printf(" revision %d: /a = %-4d (expected %d)%s\n", i + 1, v, 10 + i,
(v == 10 + i) ? "" : " <-- WRONG");
if (v != 10 + i)
bad = 1;
H5Fclose(fid);
H5Pclose(fapl);
}
printf("page_size %u: %s\n", page, bad ? "REVISIONS LOST" : "ok");
return bad;
}
```
Three write sessions against one canonical file, each replacing a single scalar dataset `/a` with a new
value. Each session is one open/close cycle, so the history should hold three revisions with `/a` == 10,
11, 12.
### Observed
```
$ ./onion_lost_revisions 256
page_size 256: H5FDonion_get_revision_count() = 3
revision 1: /a = 10 (expected 10)
revision 2: /a = 11 (expected 11)
revision 3: /a = 12 (expected 12)
page_size 256: ok
$ ./onion_lost_revisions 512
page_size 512: H5FDonion_get_revision_count() = 3
revision 1: /a = -1 (expected 10) <-- WRONG
revision 2: /a = 11 (expected 11)
revision 3: /a = 11 (expected 12) <-- WRONG
page_size 512: REVISIONS LOST
```
Revision 1 comes back empty (`/a` does not exist) and revision 3 is a duplicate of revision 2, while the
revision count still says 3. Sweeping page sizes on this reproducer:
| page_size | result |
|---|---|
| 32, 64, 128, 256 | correct |
| 512, 1024, 2048, 4096 | revisions lost |
### It depends on file size, not on page size alone
The same three-session sequence is correct at **every** page size tested, 4096 included, once each session
also writes a bulk dataset (4096 ints) so the file comfortably exceeds one page. The failure appears when
the whole file is small relative to `page_size` — the reproducer above produces a ~2 KB file.
`H5Ocopy` is not involved: creating the datasets directly (as above) and `H5Ocopy`ing them in from another
file behave identically at every page size tried.
### Expected
Either all three revisions are recorded and readable, or the write fails loudly. Silently dropping
committed revisions while reporting the full revision count means an application cannot tell a complete
history from a truncated one.
### Environment
- HDF5 `develop` @ d006fb8d3a09c7d09048f25b45336afeddd9d87d (2026-08-06), version 2.3.0
- Linux, gcc 15.2.1, `CMAKE_BUILD_TYPE=Debug`
### Note on coverage
`test/onion.c` uses only `ONION_TEST_PAGE_SIZE_1` (4) and `ONION_TEST_PAGE_SIZE_5` (32), both in the range
that behaves correctly here, so this regime appears to be untested rather than misused. A 4096-byte page is
what an application would reach for on real data, which is how I ran into it — building a tool that turns a
streaming HDF5 file's steps into addressable onion revisions.
Contributor guide
Assessment
This issue has not been assessed yet.