Skip to content

docs(clustering): correct the stale server-heartbeat interval comment - #37014

Open
dsolistorres wants to merge 1 commit into
mainfrom
fix-stale-heartbeat-interval-comment
Open

docs(clustering): correct the stale server-heartbeat interval comment#37014
dsolistorres wants to merge 1 commit into
mainfrom
fix-stale-heartbeat-interval-comment

Conversation

@dsolistorres

@dsolistorres dsolistorres commented Aug 11, 2026

Copy link
Copy Markdown
Member

Proposed Changes

Fixes #37015.

SERVER_HEARTBEAT_RUN_EVERY_SECONDS defaults to 60, but the trailing comment read // runs every 5 seconds.

-       final int delaySeconds = Config.getIntProperty("SERVER_HEARTBEAT_RUN_EVERY_SECONDS", 60); // runs every 5 seconds.
+       // Both default to one minute, matching the HEARTBEAT_CRON_EXPRESSION (0 0/1 * * * ?) this
+       // replaced when the schedulers were unified. The cadence bounds how quickly a node notices
+       // a cluster membership change or retries a failed cache-transport rewire.
+       final int delaySeconds = Config.getIntProperty("SERVER_HEARTBEAT_RUN_EVERY_SECONDS", 60);

Why it was wrong

Not a default that changed and left the comment behind — the comment was wrong when it was written.

999036092e (#19291, unifying the 3 quartz schedulers, Oct 2020) created this block by adapting the SystemEventsJob block ~150 lines above:

final int delaySeconds = Config.getIntProperty("SYSTEM_EVENTS_DELAY_SECONDS", 5); // runs every 5 seconds.

That one is correct — it really does default to 5. The heartbeat copy changed the property name and the default to 60 and kept the comment verbatim.

The cadence never changed either. Before that commit the heartbeat was a Quartz CronTrigger on HEARTBEAT_CRON_EXPRESSION, defaulted to 0 0/1 * * * ? in dotcms-config-cluster.properties — once a minute. The 60-second fixed delay preserved it; only the mechanism changed.

Testing

Comment only, no behaviour change — nothing to test. Found while tracing the heartbeat cadence for #36803, where this interval bounds how quickly a failed cache-transport rewire is retried.

🤖 Generated with Claude Code

@github-actions github-actions Bot added the Area : Backend PR changes Java/Maven backend code label Aug 11, 2026
@claude

claude Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Claude finished @dsolistorres's task in 28s —— View job


Code Review

  • Read the diff (DotInitScheduler.java) and surrounding context
  • Verify the comment's accuracy against the code

New Issues

No issues found.

The change is comment-only. It's accurate on every point I can verify from the code:

  • SERVER_HEARTBEAT_RUN_EVERY_SECONDS defaults to 60 (DotInitScheduler.java:587), so the old inline // runs every 5 seconds. was wrong — the new "one minute" wording is correct.
  • SERVER_HEARTBEAT_INITIAL_DELAY_SECONDS also defaults to 60 (DotInitScheduler.java:583), so "Both default to one minute" holds.
  • The cron expression 0 0/1 * * * ? cited in the comment does fire once per minute, matching the 60-second scheduleAtFixedRate delay — the equivalence is stated correctly.

No behavior change, no test impact. Clean.

fix-stale-heartbeat-interval-comment

SERVER_HEARTBEAT_RUN_EVERY_SECONDS defaults to 60, but the trailing comment read
"runs every 5 seconds."

The comment was wrong from the moment it was written, not left behind by a later
change of default. Commit 9990360 (#19291, unifying the three quartz
schedulers) introduced this block by adapting the SystemEventsJob block ~150
lines above, which does default to 5 seconds and carries that exact comment; the
property name and default were changed to 60 and the comment was not.

The cadence itself never changed either. Before that commit the heartbeat was a
Quartz CronTrigger on HEARTBEAT_CRON_EXPRESSION, defaulted to "0 0/1 * * * ?" in
dotcms-config-cluster.properties -- once a minute. The 60-second fixed delay
preserved it.

Replaced with a note on what the interval actually governs, since it bounds how
quickly a node notices a membership change or retries a failed cache-transport
rewire (#36803).

Comment only; no behaviour change.
@dsolistorres
dsolistorres force-pushed the fix-stale-heartbeat-interval-comment branch from 840636b to fdb0344 Compare August 15, 2026 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Correct the stale server-heartbeat interval comment in DotInitScheduler

1 participant