Make the h5py plugin's flush timestamp use the same clock as everything else - #85
Merged
Merged
Conversation
The plugin skipped writing an attribute whose "created" value was less than _flush_time, but the two came from different clocks: Hdf5db stamps "created" with getNow(), while _flush_time was read from time.time(). On POSIX these agree, since getNow() is time.time() there. On Windows before 3.13 they do not. getNow() advances a wall-clock anchor by a perf_counter delta, and that anchor is captured once from a time.time() that quantises to a ~15.6ms tick, so the two clocks sit on number lines whose offset is arbitrary within one tick. An attribute created after a flush could therefore carry a "created" value that compares less than _flush_time and be silently skipped - never written to the file. That is the intermittent Windows CI failure in test/unit/h5py_test.py: testSimple sees one of two attributes, and testReaderWithUpdate reads a stale value because the update was dropped. Python 3.13 does not fail, because it made time.time() precise on Windows and closed the gap. Reproduced by quantising time.time() to the Windows tick and pointing time_util at os.name == 'nt': 13 of 20 runs failed before the change, 0 of 20 after, with the POSIX suite unaffected. Note the fix is consistency, not resolution. getNow() is monotonic and both call sites share the process-level anchor, so anything created after a flush compares greater; the comparison is strict, so equal timestamps fall on the write side and cost a redundant write rather than a lost one.
github-project-automation
Bot
moved this from To be triaged
to Done
in HSDS - TRIAGE & TRACK
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Make timing mechanism consistent across OSes to prevent flaky test failures.
The plugin skipped attributes whose
created(stamped withgetNow()) compared less than_flush_time(read fromtime.time()). Those agree on POSIX but not on Windows before 3.13, so attributes created after a flush could be silently never written.