Skip to content

Commit

Permalink
Change default NTP servers from US to international ones. (#49)
Browse files Browse the repository at this point in the history
Addresses [the comment from a previous PR](#48 (comment)).

Server addresses given to me at Minsk are from Belarus. Meaning the pool seem to provide not random addresses (like [the NTP page mentions](https://www.ntppool.org/en/use.html)) but localized ones.

PTAL @buildbreaker, @ameliariely, @amphora001
  • Loading branch information
arturdryomov authored Nov 19, 2020
1 parent feeea3d commit ce34b7b
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 22 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ Kronos comes with a set of reasonable default configurations. You can customize
* `syncListener`
* Allows you to log sync operation successes and errors, which maybe useful for custom analytics. Pass an implementation of `SyncListener`.
* `ntpHosts`
* Specify a list of NTP servers with which to sync.
* Specify a list of NTP servers with which to sync. Default servers are set to [the NTP pool](https://www.ntppool.org/en/use.html).
* `requestTimeoutMs`
* Lengthen or shorten the timeout value. If the NTP server fails to respond within the given time, the next server will be contacted. If none of the server respond within the given time, the sync operation will be considered a failure.
* `minWaitTimeBetweenSyncMs`
Expand Down
4 changes: 2 additions & 2 deletions kronos-java/src/main/java/com/lyft/kronos/DefaultParam.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ package com.lyft.kronos
import java.util.concurrent.TimeUnit

object DefaultParam {
val NTP_HOSTS = listOf("2.us.pool.ntp.org", "1.us.pool.ntp.org", "0.us.pool.ntp.org")
val NTP_HOSTS = listOf("0.pool.ntp.org", "1.pool.ntp.org", "2.pool.ntp.org", "3.pool.ntp.org")
// Sync with NTP if the cache is older than this value
val CACHE_EXPIRATION_MS = TimeUnit.MINUTES.toMillis(1)
// Sync with NTP only after MIN_WAIT_TIME_BETWEEN_SYNC_MS regardless of success or failure
val MIN_WAIT_TIME_BETWEEN_SYNC_MS = TimeUnit.MINUTES.toMillis(1)
val TIMEOUT_MS = TimeUnit.SECONDS.toMillis(6)
val MAX_NTP_RESPONSE_TIME_MS = TimeUnit.SECONDS.toMillis(5)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class SntpServiceTest {
private val responseCache = mock<SntpResponseCache>()
private val sntpSyncListener = mock<SyncListener>()
private val mockResponse = mock<SntpClient.Response>()
private val ntpHosts = listOf("2.us.pool.ntp.org", "1.us.pool.ntp.org", "0.us.pool.ntp.org")
private val ntpHosts = listOf("2.fake.pool.ntp.org", "1.fake.pool.ntp.org", "0.fake.pool.ntp.org")

init {
whenever(deviceClock.getElapsedTimeMs()).then({ System.currentTimeMillis() })
Expand All @@ -32,27 +32,27 @@ class SntpServiceTest {
assertThat(sntpService.sync()).isFalse()

verify(sntpSyncListener, times(3)).onStartSync(any())
verify(sntpSyncListener, times(1)).onError("2.us.pool.ntp.org", mockError)
verify(sntpSyncListener, times(1)).onError("1.us.pool.ntp.org", mockError)
verify(sntpSyncListener, times(1)).onError("0.us.pool.ntp.org", mockError)
verify(sntpSyncListener, times(1)).onError("2.fake.pool.ntp.org", mockError)
verify(sntpSyncListener, times(1)).onError("1.fake.pool.ntp.org", mockError)
verify(sntpSyncListener, times(1)).onError("0.fake.pool.ntp.org", mockError)

whenever(sntpClient.requestTime(any(), any())).thenReturn(mockResponse)
assertThat(sntpService.sync()).isTrue

verify(sntpSyncListener, times(2)).onStartSync("2.us.pool.ntp.org")
verify(sntpSyncListener, times(2)).onStartSync("2.fake.pool.ntp.org")
verify(sntpSyncListener, times(1)).onSuccess(any(), any())
}

@Test
fun testOneOfThreeServerHasError() {
val mockError = mock<IOException>()
whenever(sntpClient.requestTime("2.us.pool.ntp.org", TIMEOUT_MS)).thenThrow(mockError)
whenever(sntpClient.requestTime("1.us.pool.ntp.org", TIMEOUT_MS)).thenReturn(mockResponse)
whenever(sntpClient.requestTime("2.fake.pool.ntp.org", TIMEOUT_MS)).thenThrow(mockError)
whenever(sntpClient.requestTime("1.fake.pool.ntp.org", TIMEOUT_MS)).thenReturn(mockResponse)
assertThat(sntpService.sync()).isTrue

verify(sntpSyncListener, times(1)).onStartSync("2.us.pool.ntp.org")
verify(sntpSyncListener, times(1)).onError("2.us.pool.ntp.org", mockError)
verify(sntpSyncListener, times(1)).onStartSync("1.us.pool.ntp.org")
verify(sntpSyncListener, times(1)).onStartSync("2.fake.pool.ntp.org")
verify(sntpSyncListener, times(1)).onError("2.fake.pool.ntp.org", mockError)
verify(sntpSyncListener, times(1)).onStartSync("1.fake.pool.ntp.org")
verify(sntpSyncListener, times(1)).onSuccess(any(), any())
}

Expand All @@ -64,13 +64,13 @@ class SntpServiceTest {
.thenReturn(now + MAX_NTP_RESPONSE_TIME_MS + 1) //first response time
.thenReturn(now + MAX_NTP_RESPONSE_TIME_MS + 10) //second request time
.thenReturn(now + MAX_NTP_RESPONSE_TIME_MS + 50) //second response time
whenever(sntpClient.requestTime("2.us.pool.ntp.org", TIMEOUT_MS)).thenReturn(mockResponse)
whenever(sntpClient.requestTime("1.us.pool.ntp.org", TIMEOUT_MS)).thenReturn(mockResponse)
whenever(sntpClient.requestTime("2.fake.pool.ntp.org", TIMEOUT_MS)).thenReturn(mockResponse)
whenever(sntpClient.requestTime("1.fake.pool.ntp.org", TIMEOUT_MS)).thenReturn(mockResponse)
assertThat(sntpService.sync()).isTrue

verify(sntpSyncListener, times(1)).onStartSync("2.us.pool.ntp.org")
verify(sntpSyncListener, times(1)).onError(eq("2.us.pool.ntp.org"), any<NTPSyncException>())
verify(sntpSyncListener, times(1)).onStartSync("1.us.pool.ntp.org")
verify(sntpSyncListener, times(1)).onStartSync("2.fake.pool.ntp.org")
verify(sntpSyncListener, times(1)).onError(eq("2.fake.pool.ntp.org"), any<NTPSyncException>())
verify(sntpSyncListener, times(1)).onStartSync("1.fake.pool.ntp.org")
verify(sntpSyncListener, times(1)).onSuccess(any(), any())
}

Expand All @@ -85,13 +85,13 @@ class SntpServiceTest {

@Test
fun `throw error if response has negative time value`() {
whenever(sntpClient.requestTime("1.us.pool.ntp.org", TIMEOUT_MS)).thenReturn(mockResponse)
whenever(sntpClient.requestTime("2.us.pool.ntp.org", TIMEOUT_MS)).thenReturn(mockResponse)
whenever(sntpClient.requestTime("1.fake.pool.ntp.org", TIMEOUT_MS)).thenReturn(mockResponse)
whenever(sntpClient.requestTime("2.fake.pool.ntp.org", TIMEOUT_MS)).thenReturn(mockResponse)
whenever(mockResponse.currentTimeMs).thenReturn(-1)

assertThat(sntpService.sync()).isFalse

verify(sntpSyncListener, times(1)).onError(eq("1.us.pool.ntp.org"), any<NTPSyncException>())
verify(sntpSyncListener, times(1)).onError(eq("2.us.pool.ntp.org"), any<NTPSyncException>())
verify(sntpSyncListener, times(1)).onError(eq("1.fake.pool.ntp.org"), any<NTPSyncException>())
verify(sntpSyncListener, times(1)).onError(eq("2.fake.pool.ntp.org"), any<NTPSyncException>())
}
}

0 comments on commit ce34b7b

Please sign in to comment.