Skip to content

[improve][broker] Recalculate delayed delivery time using server clock to mitigate clock skew#25310

Open
Radiancebobo wants to merge 1 commit intoapache:masterfrom
Radiancebobo:delayedDeliveryRecalculateWithServerClock
Open

[improve][broker] Recalculate delayed delivery time using server clock to mitigate clock skew#25310
Radiancebobo wants to merge 1 commit intoapache:masterfrom
Radiancebobo:delayedDeliveryRecalculateWithServerClock

Conversation

@Radiancebobo
Copy link
Contributor

@Radiancebobo Radiancebobo commented Mar 11, 2026

Motivation

When clients and brokers have clock skew, delayed message delivery can behave incorrectly. For example:

  • If a client's clock is 5 minutes behind the server and uses deliverAfter(3min), the message may be
    delivered immediately instead of after 3 minutes
  • If a client's clock is 5 minutes ahead, the message may be delivered 5 minutes later than intended

This happens because the broker uses the client-provided deliverAtTime timestamp directly, which is
calculated based on the client's clock.

Modifications

  • Modified PersistentDispatcherMultipleConsumers.trackDelayedDelivery() to recalculate delivery time based on
    server clock
  • Modified PersistentDispatcherMultipleConsumersClassic.trackDelayedDelivery() with the same logic
  • The fix calculates the relative delay (deliverAtTime - publishTime) and adds it to the current server time
  • Falls back to original behavior when publishTime is missing or when relativeDelay <= 0
  • Added comprehensive unit tests in TrackDelayedDeliveryClockSkewTest covering various clock skew scenarios

Verifying this change

This change added tests and can be verified as follows:

  • Added unit tests TrackDelayedDeliveryClockSkewTest with 7 test cases:
    • testDeliverAfterWithClientClockBehindServer - verifies correct behavior when client clock is behind
    • testDeliverAfterWithClientClockAheadOfServer - verifies correct behavior when client clock is ahead
    • testFallbackWhenNoPublishTime - verifies fallback when publishTime is missing
    • testFallbackWhenRelativeDelayNegative - verifies fallback for abnormal data
    • testNoDeliverAtTime - verifies normal non-delayed messages
    • testNoClockSkew - verifies behavior with synchronized clocks
    • testClassicDispatcherClockSkewRecalculation - verifies Classic dispatcher has same behavior

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

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: x

@github-actions
Copy link

@Radiancebobo Please add the following content to your PR description and select a checkbox:

- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

@github-actions github-actions bot added doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Mar 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant