Skip to content

[cleanup][broker] Standardize java.time.Clock usage in broker service#25935

Open
Praveenkumar76 wants to merge 2 commits into
apache:masterfrom
cognitree:fix/issue-25660-clock-standardization
Open

[cleanup][broker] Standardize java.time.Clock usage in broker service#25935
Praveenkumar76 wants to merge 2 commits into
apache:masterfrom
cognitree:fix/issue-25660-clock-standardization

Conversation

@Praveenkumar76

@Praveenkumar76 Praveenkumar76 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Fixes apache#25660

Motivation

The usage of java.time.Clock is currently inconsistent across the broker module, with many classes directly relying on System.currentTimeMillis() for timestamp generation.

Standardizing timestamp handling through the centralized PulsarService clock improves consistency across the codebase and makes time-dependent behavior easier to control during testing. This reduces test flakiness by enabling deterministic time mocking in unit and integration tests.

Because there are hundreds of occurrences of System.currentTimeMillis() across the repository, this PR intentionally scopes the change to a tightly focused set of classes. It specifically targets classes where the Clock dependency can be cleanly injected via the constructor, ensuring a clear separation of concerns and keeping the review safe and manageable.

Modifications

Replaced direct usages of System.currentTimeMillis() with this.clock.millis() by utilizing field injection for the Clock dependency in the following broker module classes:

  • MessageDeduplication.java
  • SchemaRegistryStats.java
  • PulsarStats.java

Additional changes:

  • Left existing System.nanoTime() usages unchanged, as they are used for monotonic latency measurements rather than wall-clock timestamps.

Verifying this change

This change can be verified as follows:

  • Confirmed successful compilation and that existing broker tests continue to pass.
  • Verified timestamp-related logic in the modified classes now consistently uses the centralized broker clock via constructor injection.
  • Confirmed no behavioral changes in latency-related code paths using System.nanoTime().

Does this pull request potentially affect one of the following parts:

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

@Praveenkumar76 Praveenkumar76 force-pushed the fix/issue-25660-clock-standardization branch from 1277267 to 3a439ac Compare June 5, 2026 06:30
@Praveenkumar76 Praveenkumar76 marked this pull request as ready for review June 5, 2026 07:50
@Praveenkumar76 Praveenkumar76 force-pushed the fix/issue-25660-clock-standardization branch 2 times, most recently from ca1957f to 3a439ac Compare June 7, 2026 17:31
@Praveenkumar76 Praveenkumar76 force-pushed the fix/issue-25660-clock-standardization branch from 3a439ac to a79fbc5 Compare June 7, 2026 17:35
@Praveenkumar76 Praveenkumar76 marked this pull request as draft June 7, 2026 17:50
@Praveenkumar76 Praveenkumar76 marked this pull request as ready for review June 7, 2026 17:56
@Praveenkumar76 Praveenkumar76 marked this pull request as draft June 8, 2026 04:42
@Praveenkumar76 Praveenkumar76 marked this pull request as ready for review June 8, 2026 07:35
@Praveenkumar76 Praveenkumar76 marked this pull request as draft June 8, 2026 07:45
@Praveenkumar76 Praveenkumar76 force-pushed the fix/issue-25660-clock-standardization branch from d3a9bbc to 53f8f24 Compare June 8, 2026 07:53
@Praveenkumar76 Praveenkumar76 marked this pull request as ready for review June 8, 2026 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Usage of java.time.Clock for timestamps isn't consistently performed in Pulsar code base

1 participant