fix: Fix invalid time comparison#19603
Conversation
534f417 to
81c9773
Compare
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 0 |
| Total | 1 |
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 0 |
| Total | 1 |
Reviewed 1 of 1 changed files.
This is an automated review by Codex GPT-5.5
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 0 |
| P3 | 1 |
| Total | 1 |
Reviewed 2 of 2 changed files.
This is an automated review by Codex GPT-5.5
|
@vivek807 |
Yes, this will avoid confusion in future. Let me refactor it to use same type of clock here |
Signed-off-by: Vivek Dhiman <approach2vivek@gmail.com>
b80a10d to
33138ae
Compare
FrankChen021
left a comment
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 0 |
| Total | 1 |
Reviewed 2 of 2 changed files.
This is an automated review by Codex GPT-5.5
| long millisElapsedFromInitiation() | ||
| { | ||
| return System.nanoTime() - initiationTimeNanos; | ||
| return System.currentTimeMillis() - initiationTimeMillis; |
There was a problem hiding this comment.
[P2] Use monotonic time for freshness elapsed checks
millisElapsedFromInitiation() now computes elapsed time from System.currentTimeMillis(). If the wall clock moves backward after an on-demand poll starts, this value can become negative or too small, so useLatestSnapshotIfWithinDelay() can keep accepting a stale snapshot and the periodic poll task can sleep longer than periodicPollDelay. Keep a monotonic nanoTime() value for elapsed/freshness calculations and use a separate wall-clock timestamp only where this code compares against wall-clock poll start times.
Fixes #19602.
Description
forceOrWaitOngoingDatabasePollinSqlSegmentsMetadataManagerchecks whether a concurrentOnDemandDatabasePollthat completed while the calling thread waited for the write lock makes a new poll unnecessary. This check was always false due to a clock mismatch, causing an extra database poll on every contended call.Fixed invalid clock comparison in
forceOrWaitOngoingDatabasePollOnDemandDatabasePoll.initiationTimeNanosis assigned viaSystem.nanoTime(), which is a monotonic clock with an arbitrary JVM-internal reference point.The comparison value was constructed as:
System.currentTimeMillis() returns milliseconds since the Unix epoch (~1.7 × 10¹² ms). Converting to nanoseconds yields ~1.7 × 10²¹ ns, which is orders of magnitude larger than any System.nanoTime() value. The condition
initiationTimeNanos > checkStartTimeNanoswas therefore always false, so an in-progress on-demand poll was never recognised as sufficient.This PR has: