PHPLIB-1115: Fix waitForSnapshot() for MongoDB 7.0+ #1083
Conversation
|
Note: test failures on mongodb-latest are due to PHPLIB-1071. |
| /* Retry until a snapshot query succeeds or ten seconds elapse, | ||
| * whichwever comes first. | ||
| * | ||
| * TODO: use hrtime() once the library requires PHP 7.3+ */ |
There was a problem hiding this comment.
This todo makes sense.
You can also remove the microtime calls and count 10000 iterations with 1ms sleep.
There was a problem hiding this comment.
count 10000 iterations with 1ms sleep.
Doing so wouldn't consider the time spent executing the FindOne operation.
This todo makes sense
I don't recall my original intention behind adding a TODO for hrtime() (it wasn't discussed in #900), but I gave this some thought when implementing this PR and couldn't see any reason to use it:
microtime()isn't deprecated and we don't need the precision offered byhrtime()- The variable return types for
hrtime()seemed more complicated to work with - A monotonic timer is nice, but not necessary for a rarely used test method where we also don't need to worry about system clock changes
Side note: I did review the 7.3 polyfill while working on this PR.
Having said all that, we do have many uses of microtime() in the test suite:
- DocumentationExamplesTest
- FindFunctionalTest
- WatchFunctionalTest
- UnifiedSpecTests/EventCollector and Loop (these should remain as-is)
The first three instances all pertain to calculating durations (actual timestamp isn't relevant) so I wouldn't object if you want to create a ticket to integrate hrtime() there.
This fixes a bug in the original implementation where the method would return even if the query did not match a document. Instead, we should retry if either the query fails to match or a SnapshotUnavailable error is raised. This issue was only discovered due to increased snapshot latency in MongoDB 7.0-dev. In previous versions, the snapshot was always available by the subsequent queries in testSnapshotQueries(), which uses waitForSnapshot(). Additionally, usleep() is added to avoid spamming the server with retry attempts. The TODO item for hrtime() was also removed, as there's no significant benefit to using it in this context and it's return types are not as convenient as microtime().
https://jira.mongodb.org/browse/PHPLIB-1115
Using find instead of aggregate should ensure documents are available in the calling test case on 7.0+ servers.