kfaraz commented on code in PR #19603:
URL: https://github.com/apache/druid/pull/19603#discussion_r3489910141
##########
server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataManager.java:
##########
@@ -567,11 +567,11 @@ void forceOrWaitOngoingDatabasePoll()
&& ((PeriodicDatabasePoll)
latestDatabasePoll).lastPollStartTimestampInMs > checkStartTime) {
return;
}
- // Verify if there was a on-demand poll completed while we were
waiting for the lock
+ // Verify if there was an on-demand poll completed while we were
waiting for the lock
if (latestDatabasePoll instanceof OnDemandDatabasePoll) {
- long checkStartTimeNanos =
TimeUnit.MILLISECONDS.toNanos(checkStartTime);
OnDemandDatabasePoll latestOnDemandPoll = (OnDemandDatabasePoll)
latestDatabasePoll;
- if (latestOnDemandPoll.initiationTimeNanos > checkStartTimeNanos) {
+ if (latestOnDemandPoll.initiationTimeMillis > checkStartTime) {
+ Futures.getUnchecked(latestOnDemandPoll.pollCompletionFuture);
Review Comment:
Is this really needed?
##########
server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataManager.java:
##########
@@ -100,12 +100,12 @@ static class PeriodicDatabasePoll implements DatabasePoll
@VisibleForTesting
static class OnDemandDatabasePoll implements DatabasePoll
{
- final long initiationTimeNanos = System.nanoTime();
+ final long initiationTimeMillis = System.currentTimeMillis();
Review Comment:
Let's stick to using nanos since it is more reliably monotonic.
##########
server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataManager.java:
##########
@@ -567,11 +567,11 @@ void forceOrWaitOngoingDatabasePoll()
&& ((PeriodicDatabasePoll)
latestDatabasePoll).lastPollStartTimestampInMs > checkStartTime) {
return;
}
- // Verify if there was a on-demand poll completed while we were
waiting for the lock
+ // Verify if there was an on-demand poll completed while we were
waiting for the lock
if (latestDatabasePoll instanceof OnDemandDatabasePoll) {
- long checkStartTimeNanos =
TimeUnit.MILLISECONDS.toNanos(checkStartTime);
Review Comment:
We should change this line to simply be:
```java
long checkStartTimeNanos = System.nanoTime();
```
We should also change `PeriodicDatabasePoll.lastPollStartTimestampInMs` to
be `lastPollStartTimestampInNanos`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
