[server] Fix follower heartbeat entries for AA stores to use local region only

[sixpluszero]

[server] Fix follower heartbeat entries for AA stores to use local region only

Commit a7f4d1eca ("Introduce record level delay with heartbeat delay") refactored HeartbeatMonitoringService from nested maps to flat HeartbeatKey maps. During that refactoring, the !isFollower guard was accidentally dropped from initializeEntry().

Before (correct):

if (version.isActiveActiveReplicationEnabled() && !isFollower) {
    // ALL regions — leader only
} else {
    // LOCAL region only — follower, or non-AA
}

After regression (current):

if (version.isActiveActiveReplicationEnabled()) {
    // ALL regions — both leader AND follower (wrong!)
}

Impact: For AA-enabled stores, follower replicas now get heartbeat entries for ALL regions. Since followers only consume from local VT, non-local entries are never updated — their delays grow unbounded, generating misleading catch_up metrics (both Tehuti and OTel) for all regions instead of just the local region.

Solution

Restore the !isFollower guard in initializeEntry() so that only leaders get heartbeat entries for all regions when AA is enabled. Followers should only get the local region entry regardless of AA configuration.

Code changes

• Added new code behind a config. If so list the config names and their default values in the PR description.
• Introduced new log lines.
  • Confirmed if logs need to be rate limited to avoid excessive logging.

Concurrency-Specific Checks

Both reviewer and PR author to verify

• Code has no race conditions or thread safety issues.
• Proper synchronization mechanisms (e.g., synchronized, RWLock) are used where needed.
• No blocking calls inside critical sections that could lead to deadlocks or performance degradation.
• Verified thread-safe collections are used (e.g., ConcurrentHashMap, CopyOnWriteArrayList).
• Validated proper exception handling in multi-threaded code to avoid silent thread termination.

How was this PR tested?

• New unit tests added.
• New integration tests added.
• Modified or extended existing tests.
• Verified backward compatibility (if applicable).

Updated two assertions in HeartbeatMonitoringServiceTest:

1. AA followers should have 1 region entry (local only), not all regions
2. REMOTE_FABRIC entry should be null (not present) for AA followers

All 11 tests in HeartbeatMonitoringServiceTest pass.

Does this PR introduce any user-facing or breaking changes?

• No. You can skip the rest of this section.
• Yes. Clearly explain the behavior change and its impact.

[Copilot]

Pull request overview

Restores correct heartbeat entry initialization behavior in HeartbeatMonitoringService so that for Active/Active (AA) stores, leaders initialize heartbeat tracking for all regions while followers only initialize tracking for the local region—preventing unbounded lag growth and misleading catch-up metrics for non-local regions on follower replicas.

Changes:

• Fix regression by reintroducing the !isFollower guard in HeartbeatMonitoringService.initializeEntry() for AA versions.
• Update HeartbeatMonitoringServiceTest assertions to validate that AA followers only have a single (local) region entry and that REMOTE_FABRIC entries are absent.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
clients/da-vinci-client/src/main/java/com/linkedin/davinci/stats/ingestion/heartbeat/HeartbeatMonitoringService.java	Restores leader-vs-follower conditional so AA followers don’t initialize heartbeat entries for all regions.
clients/da-vinci-client/src/test/java/com/linkedin/davinci/stats/ingestion/heartbeat/HeartbeatMonitoringServiceTest.java	Updates expectations to ensure AA follower maps only contain local-region entries (and no REMOTE_FABRIC).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

clients/da-vinci-client/src/main/java/com/linkedin/davinci/stats/ingestion/heartbeat/HeartbeatMonitoringService.java:147

• With this change, new AA follower lag monitors will only initialize the local-region entry, but any non-local follower entries that were already created by the previous regression will remain in followerHeartbeatTimeStamps for partitions that stay followers (since initializeEntry() only does putIfAbsent). Consider pruning/removing any existing follower entries for the same (store, version, partition) whose region != localRegionName when (re)adding a follower monitor, so the fix takes effect without requiring a process restart or a leader transition to trigger removeEntry().

    if (version.isActiveActiveReplicationEnabled() && !isFollower) {
      for (String region: regionNames) {
        if (Utils.isSeparateTopicRegion(region) && !version.isSeparateRealTimeTopicEnabled()) {
          continue;
        }
        HeartbeatKey key = new HeartbeatKey(storeName, versionNum, partition, region);
        heartbeatTimestamps.putIfAbsent(key, new IngestionTimestampEntry(currentTime, currentTime, false, false));
      }
    } else {
      HeartbeatKey key = new HeartbeatKey(storeName, versionNum, partition, localRegionName);
      heartbeatTimestamps.putIfAbsent(key, new IngestionTimestampEntry(currentTime, currentTime, false, false));
    }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[misyel]

Thanks for the fix!