Set clock precision to milliseconds for Datetime->Instant migration by CydeWeys · Pull Request #2999 · google/nomulus

CydeWeys · 2026-03-31T21:49:02Z

Our existing precision is milliseconds so we want to stick with that for Instants. If we want to increase the precision globally after that we can do so all in one go post-migration, but for now, it would be a bad thing to have mixed precision going on just depending on whether a class happens to be migrated yet or not.

This PR also migrates all existing DateTime.nowUtc() calls to use the Clock interface, so that when they are migrated they will get the benefit of this precision-setting as well.

BUG= http://b/496985355

This change is

core/src/main/java/google/registry/batch/DeleteProberDataAction.java

    AtomicInteger hardDeletedDomains = new AtomicInteger();
    AtomicReference<ImmutableList<Domain>> domainsBatch = new AtomicReference<>();
-    DateTime startTime = DateTime.now(UTC);
+    DateTime startTime = clock.nowUtc();


core/src/main/java/google/registry/batch/DeleteProberDataAction.java


      // Automatically kill the job if it is running for over 20 hours
-    } while (DateTime.now(UTC).isBefore(startTime.plusHours(20))
+    } while (clock.nowUtc().isBefore(startTime.plusHours(20))


core/src/main/java/google/registry/batch/SendExpiringCertificateNotificationEmailAction.java

      updateLastNotificationSentDate(
          registrar,
-          DateTime.now(UTC).minusMinutes((int) UPDATE_TIME_OFFSET.getStandardMinutes()),
+          clock.nowUtc().minusMinutes((int) UPDATE_TIME_OFFSET.getStandardMinutes()),


core/src/main/java/google/registry/bigquery/BigqueryConnection.java


      public Builder timeToLive(Duration duration) {
-        this.table.setExpirationTime(DateTime.now(UTC).plus(duration).getMillis());
+        this.table.setExpirationTime(clock.nowUtc().plus(duration).getMillis());


core/src/main/java/google/registry/model/tld/label/PremiumListDao.java

core/src/main/java/google/registry/ui/server/console/ConsoleDumDownloadAction.java

    consoleApiParams
        .response()
-        .setDateHeader("Expires", DateTime.now(UTC).withTimeAtStartOfDay().plusDays(1));
+        .setDateHeader("Expires", clock.nowUtc().withTimeAtStartOfDay().plusDays(1));


util/src/main/java/google/registry/util/SelfSignedCaCertificate.java

-        keyGen.generateKeyPair(), DEFAULT_ISSUER_FQDN, DEFAULT_NOT_BEFORE, DEFAULT_NOT_AFTER);
+        keyGen.generateKeyPair(),
+        DEFAULT_ISSUER_FQDN,
+        clock.nowUtc().minusHours(1),


util/src/main/java/google/registry/util/SelfSignedCaCertificate.java

+        keyGen.generateKeyPair(),
+        DEFAULT_ISSUER_FQDN,
+        clock.nowUtc().minusHours(1),
+        clock.nowUtc().plusDays(1));


util/src/main/java/google/registry/util/SelfSignedCaCertificate.java

-  public static SelfSignedCaCertificate create(String fqdn) throws Exception {
-    return create(fqdn, DEFAULT_NOT_BEFORE, DEFAULT_NOT_AFTER);
+  public static SelfSignedCaCertificate create(String fqdn, Clock clock) throws Exception {
+    return create(fqdn, clock.nowUtc().minusHours(1), clock.nowUtc().plusDays(1));


util/src/main/java/google/registry/util/SelfSignedCaCertificate.java

-  public static SelfSignedCaCertificate create(String fqdn) throws Exception {
-    return create(fqdn, DEFAULT_NOT_BEFORE, DEFAULT_NOT_AFTER);
+  public static SelfSignedCaCertificate create(String fqdn, Clock clock) throws Exception {
+    return create(fqdn, clock.nowUtc().minusHours(1), clock.nowUtc().plusDays(1));


gbrodman

if we're trying to get rid of all the calls to DateTime.now() except in the special allowed Clock classes, we should add a presubmit check forbidding that except in those special cases

@gbrodman reviewed 56 files and all commit messages, and made 5 comments.
Reviewable status: all files reviewed, 21 unresolved discussions (waiting on CydeWeys and ptkach).

core/src/main/java/google/registry/model/tld/label/PremiumListDao.java line 132 at r4 (raw file):

  public static PremiumList save(String name, CurrencyUnit currencyUnit, List<String> inputData) {
    checkArgument(!inputData.isEmpty(), "New premium list data cannot be empty");
    return tm().transact(

need to use reTransact or refactor things to avoid nested transactions

core/src/test/java/google/registry/model/tld/label/PremiumListUtilsTest.java line 31 at r4 (raw file):

class PremiumListUtilsTest {

  private static final DateTime SAMPLE_TIME = DateTime.now(UTC);

use a constant instead of "now" to avoid potential random test failure as time goes on

core/src/test/java/google/registry/reporting/icann/IcannReportingStagerTest.java line 46 at r4 (raw file):

class IcannReportingStagerTest {

  private final FakeClock clock = new FakeClock(DateTime.now(UTC));

ditto comment about using a static time rather than a variable one

util/src/test/java/google/registry/util/PosixTarHeaderTest.java line 206 at r4 (raw file):

            .setName("(◕‿◕).txt")
            .setSize(31337)
            .setMtime(DateTime.now(DateTimeZone.UTC))

we should probably use a clock in this test class too

gbrodman

@gbrodman reviewed 9 files and all commit messages.
Reviewable status: all files reviewed, 21 unresolved discussions (waiting on CydeWeys and ptkach).

prober/src/main/java/google/registry/monitoring/blackbox/token/EppToken.java

    return String.format(
        "prober-%s-%d-%d",
-        "localhost", System.currentTimeMillis(), clientIdSuffix.incrementAndGet());
+        "localhost", clock.nowUtc().getMillis(), clientIdSuffix.incrementAndGet());


CydeWeys

@CydeWeys made 1 comment.
Reviewable status: all files reviewed, 22 unresolved discussions (waiting on gbrodman and ptkach).

util/src/test/java/google/registry/util/PosixTarHeaderTest.java line 206 at r4 (raw file):

Previously, gbrodman wrote…

we should probably use a clock in this test class too

It doesn't really make sense to use a FakeClock instance solely for the purpose of getting the time in tests. A static method that handles the millisecond-chopping should be sufficient. Can do that in a follow-up. (There are a lot of calls of DateTime.now() in tests).

CydeWeys

Seems right for a follow-up. Also it's more than just DateTime.now() -- it's in particular things like Instant.now() and the equivalent ZoneDateTime method, as those are the ones that include finer precision by default.

@CydeWeys made 1 comment.
Reviewable status: 62 of 64 files reviewed, 22 unresolved discussions (waiting on gbrodman and ptkach).

CydeWeys

Seems right for a follow-up. Also it's more than just DateTime.now() -- it's in particular things like Instant.now() and the equivalent ZoneDateTime method, as those are the ones that include finer precision by default.

@CydeWeys made 1 comment.
Reviewable status: 62 of 64 files reviewed, 22 unresolved discussions (waiting on gbrodman and ptkach).

CydeWeys

Seems right for a follow-up. Also it's more than just DateTime.now() -- it's in particular things like Instant.now() and the equivalent ZoneDateTime method, as those are the ones that include finer precision by default.

@CydeWeys made 1 comment.
Reviewable status: 62 of 64 files reviewed, 22 unresolved discussions (waiting on gbrodman and ptkach).

CydeWeys

@CydeWeys made 1 comment.
Reviewable status: 62 of 64 files reviewed, 22 unresolved discussions (waiting on gbrodman and ptkach).

core/src/main/java/google/registry/model/tld/label/PremiumListDao.java line 132 at r4 (raw file):

Previously, gbrodman wrote…

need to use reTransact or refactor things to avoid nested transactions

Fixed.

CydeWeys

@CydeWeys made 2 comments.
Reviewable status: 60 of 64 files reviewed, 22 unresolved discussions (waiting on gbrodman and ptkach).

core/src/test/java/google/registry/model/tld/label/PremiumListUtilsTest.java line 31 at r4 (raw file):

Previously, gbrodman wrote…

use a constant instead of "now" to avoid potential random test failure as time goes on

Done.

core/src/test/java/google/registry/reporting/icann/IcannReportingStagerTest.java line 46 at r4 (raw file):

Previously, gbrodman wrote…

ditto comment about using a static time rather than a variable one

Done.

CydeWeys

PTAL

@CydeWeys made 1 comment.
Reviewable status: 60 of 64 files reviewed, 22 unresolved discussions (waiting on gbrodman and ptkach).

core/src/main/java/google/registry/model/tld/label/PremiumListDao.java

+    tm().assertInTransaction();
+    return save(
+        PremiumListUtils.parseToPremiumList(
+            name, currencyUnit, inputData, tm().getTransactionTime()));


gbrodman

@gbrodman reviewed 4 files and all commit messages, and resolved 4 discussions.
Reviewable status: all files reviewed, 19 unresolved discussions (waiting on CydeWeys and ptkach).

Our existing precision is milliseconds so we want to stick with that for Instants. If we want to increase the precision globally after that we can do so all in one go post-migration, but for now, it would be a bad thing to have mixed precision going on just depending on whether a class happens to be migrated yet or not. This PR also migrates all existing DateTime.nowUtc() calls to use the Clock interface, so that when they are migrated they will get the benefit of this precision-setting as well. BUG= http://b/496985355

CydeWeys · 2026-04-03T20:16:42Z

PTAL, had to make some more changes related to transaction-handling because of tm() removals in PremiumListDao.

gbrodman

@gbrodman reviewed 8 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 20 unresolved discussions (waiting on CydeWeys).

core/src/test/java/google/registry/testing/DatabaseHelper.java line 349 at r9 (raw file):

    // unit tests.
    tm().reTransact(tm()::allocateId);
    tm().transact(() -> PremiumListDao.save(premiumList));

we probably want reTransact here and, if they exist, any other test-library places

CydeWeys

@CydeWeys made 1 comment.
Reviewable status: all files reviewed, 20 unresolved discussions (waiting on gbrodman).

core/src/test/java/google/registry/testing/DatabaseHelper.java line 349 at r9 (raw file):

Previously, gbrodman wrote…

we probably want reTransact here and, if they exist, any other test-library places

I'm actually not sure we do. (And the existing reTransact() call around allocateId should perhaps just be normal transact). We don't typically have a pattern in test classes of large wrapping transactions around multiple things happening, and none of the callers to persistPremiumList() are wrapping it in a transaction.

gbrodman

@gbrodman made 1 comment and resolved 1 discussion.
Reviewable status: all files reviewed, 19 unresolved discussions (waiting on CydeWeys).

CydeWeys requested a review from gbrodman March 31, 2026 21:49

github-advanced-security bot found potential problems Mar 31, 2026

View reviewed changes

CydeWeys force-pushed the datetime2-clock-precision branch 3 times, most recently from 6334a05 to 07fae7b Compare April 3, 2026 15:32

CydeWeys requested a review from ptkach April 3, 2026 15:32

CydeWeys force-pushed the datetime2-clock-precision branch from 07fae7b to fc829f4 Compare April 3, 2026 15:59

gbrodman requested changes Apr 3, 2026

View reviewed changes

CydeWeys force-pushed the datetime2-clock-precision branch from fc829f4 to 4f440ec Compare April 3, 2026 17:22

gbrodman reviewed Apr 3, 2026

View reviewed changes

github-advanced-security bot found potential problems Apr 3, 2026

View reviewed changes

CydeWeys commented Apr 3, 2026

View reviewed changes

CydeWeys force-pushed the datetime2-clock-precision branch from 4f440ec to 968c9ac Compare April 3, 2026 17:44

CydeWeys commented Apr 3, 2026

View reviewed changes

CydeWeys force-pushed the datetime2-clock-precision branch from 968c9ac to 565a39e Compare April 3, 2026 17:59

CydeWeys commented Apr 3, 2026

View reviewed changes

CydeWeys force-pushed the datetime2-clock-precision branch from 565a39e to 7869a7d Compare April 3, 2026 18:09

github-advanced-security bot found potential problems Apr 3, 2026

View reviewed changes

gbrodman approved these changes Apr 3, 2026

View reviewed changes

CydeWeys enabled auto-merge April 3, 2026 18:45

CydeWeys removed the request for review from ptkach April 3, 2026 19:01

CydeWeys force-pushed the datetime2-clock-precision branch 2 times, most recently from afcc312 to ded9fa3 Compare April 3, 2026 19:33

CydeWeys force-pushed the datetime2-clock-precision branch from ded9fa3 to b377f41 Compare April 3, 2026 19:40

gbrodman requested changes Apr 3, 2026

View reviewed changes

CydeWeys commented Apr 3, 2026

View reviewed changes

gbrodman approved these changes Apr 3, 2026

View reviewed changes

CydeWeys added this pull request to the merge queue Apr 3, 2026

Merged via the queue into google:master with commit 49f14b5 Apr 3, 2026
9 of 10 checks passed

CydeWeys deleted the datetime2-clock-precision branch April 3, 2026 21:37

Conversation

CydeWeys commented Mar 31, 2026 • edited by gbrodman Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Check notice

Uh oh!

Check notice

Uh oh!

Check notice

Uh oh!

Check notice

Uh oh!

Uh oh!

Check notice

Uh oh!

Check notice

Uh oh!

Check notice

Uh oh!

Check notice

Uh oh!

Check notice

Uh oh!

gbrodman left a comment

Choose a reason for hiding this comment

Uh oh!

gbrodman left a comment

Choose a reason for hiding this comment

Uh oh!

Check notice

Uh oh!

CydeWeys left a comment

Choose a reason for hiding this comment

Uh oh!

CydeWeys left a comment

Choose a reason for hiding this comment

Uh oh!

CydeWeys left a comment

Choose a reason for hiding this comment

Uh oh!

CydeWeys left a comment

Choose a reason for hiding this comment

Uh oh!

CydeWeys left a comment

Choose a reason for hiding this comment

Uh oh!

CydeWeys left a comment

Choose a reason for hiding this comment

Uh oh!

CydeWeys left a comment

Choose a reason for hiding this comment

Uh oh!

Check notice

Uh oh!

gbrodman left a comment

Choose a reason for hiding this comment

Uh oh!

CydeWeys commented Apr 3, 2026

Uh oh!

gbrodman left a comment

Choose a reason for hiding this comment

Uh oh!

CydeWeys left a comment

Choose a reason for hiding this comment

Uh oh!

gbrodman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

CydeWeys commented Mar 31, 2026 •

edited by gbrodman

Loading