Replace timeoutMs: Long with timeout: Duration across the timeout API#9
Replace timeoutMs: Long with timeout: Duration across the timeout API#9rusio wants to merge 2 commits intoanschnapp:masterfrom
timeoutMs: Long with timeout: Duration across the timeout API#9Conversation
… API Using Duration removes the millisecond unit assumption, eliminates the nanos conversion factor, and lets Session.isExpired() carry its own logic cleanly. Deadline storage moves from a sentinel long (0 = none) to a nullable Instant, making the absent-deadline case explicit. Adds SessionTimeoutTest covering the key timeout scenarios.
| includeTargets: List<String> = emptyList(), | ||
| excludeTargets: List<String> = emptyList(), | ||
| timeoutMs: Long = 60_000, | ||
| timeout: Duration = Duration.ofMinutes(1) |
There was a problem hiding this comment.
comma is missing after this argument, so the build fails.
i guess this was coming in when you merged from master (so some kind of merge issue)
i'm also not sure if this is the only one issue of this kind.
to make it more easy to check for those errors i have now a tiny CI pipeline.
so after you have fixed it please pull from master again, then you get the pipeline on our next push and it build and tests will be checked within the github UI
| } else 0L | ||
| currentSession = Session(activeMutation, deadlineNanos = deadlineNanos) | ||
| val deadline = if (activeMutation != null && timeout != null) { | ||
| Instant.now() + timeout |
There was a problem hiding this comment.
The old code used System.nanoTime() which is monotonic -- it only moves forward, regardless of what happens to the system clock.
Here you use Instant.now() which reads the wall clock and can jump backward or forward due to NTP sync, clock adjustments etc...
For a timeout that detects infinite loops in mutation runs:
- Clock jumps forward -> false timeout, a healthy mutation gets killed prematurely
- Clock jumps backward -> missed timeout, an actual infinite loop runs longer than expected (or never triggers)
I would say let's keep the Duration as the public API type (that's a good improvement), but internally still convert to a System.nanoTime()-based deadline for the actual expiry check.
2 participants
Using Duration removes the millisecond unit assumption, eliminates the nanos conversion factor, and lets Session.isExpired() carry its own logic cleanly. Deadline storage moves from a sentinel long (0 = none) to a nullable Instant, making the absent-deadline case explicit. Adds SessionTimeoutTest covering the key timeout scenarios.