Skip to content

Commit 2034234

Browse files
committed
[server] Add lkc heartbeat header for active-key-count consistency check
across replicas When the active-key-count replica consistency check is enabled, the A/A leader attaches an `lkc` (leader key count) PubSub header on every VT heartbeat carrying its current active key count. Followers compare against their own count when processing the heartbeat; a mismatch records `ingestion.key.active_count_mismatch_across_replicas` (Tehuti + OTel) and emits a rate-limited WARN log. The follower's count is NOT invalidated — the check is diagnostic-only. Design notes: - New config `server.active.key.count.replica.consistency.check.enabled` (default false). Effective only when `server.active.key.count.for.hybrid.store.enabled` is also true; the `VeniceServerConfig` accessor ANDs the two so registration and recording sites can't drift. - `PartitionConsumptionState.lastVTProduceCallFuture` switched from a volatile `CompletableFuture<Void>` to a `final AtomicReference< CompletableFuture<Void>>` with `swapLastVTProduceCallFuture(next)` returning the prior head. The new swap rejects null. `sendIngestionHeartbeatToVT` chains the HB send behind the previous chain entry via `whenCompleteAsync` so the lkc lands at the broker after preceding view-writer data writes. - The lkc value is read synchronously on the SIT thread BEFORE the swap and captured in the chained-callback lambda. Reading from inside the deferred callback would observe later SIT increments for records queued AFTER this HB → false-positive mismatches. - The chained callback propagates the upstream chain failure (skipping the HB to avoid stamping records that never landed) and surfaces synchronous `sendIngestionHeartbeat` failures into the chain head; async broker rejections are logged separately and don't gate the chain (matches `queueUpVersionTopicWritesWithViewWriters` enqueue-completion semantics). - `closeVeniceViewWriters` now short-circuits the chain head unconditionally so hybrid A/A configs without view writers (which can still grow the chain via the HB swap) don't leave a pending head orphaned at close. `checkAndWaitForLastVTProduceFuture` bounds the EOP-time wait with `VIEW_WRITER_CLOSE_TIMEOUT_IN_MS` and surfaces `TimeoutException` with explicit attribution at the caller. - `decodeLeaderKeyCountHeaderValue` throws `IllegalArgumentException` on wrong-length payloads rather than returning a `Long.MIN_VALUE` sentinel that would collide with a legitimately round-trippable value. - `ActiveKeyCountInvalidationReason` now implements `VeniceDimensionInterface` and is wired as a dimension on the existing `ingestion.key.active_count_invalidation` metric. Three corruption variants were renamed for clarity: `CORRUPT_KCS_SIGNAL_VALUE → CORRUPT_KEY_COUNT_SIGNAL_HEADER_VALUE`, `CORRUPT_KCS_HEADER_LENGTH → CORRUPT_KEY_COUNT_SIGNAL_HEADER_LENGTH`, and the new `CORRUPT_LEADER_KEY_COUNT_HEADER_LENGTH` for malformed lkc. The enum guards `getMessage` vs `getMessage(int)` against mismatched overload usage via a constructor-computed `templateWithExtraData` flag. - `VeniceWriter.sendHeartbeat` gained a 7-arg overload that attaches an optional extra PubSub header. The 6-arg overload still exists and delegates to the 7-arg with null. Tests migrated to stub the 7-arg signature; production code calls the 7-arg form unconditionally. - `EmptyPubSubMessageHeaders` singleton is promoted to a fresh mutable instance before appending the extra header. Wire constants moved to `PubSubMessageHeaders`: `VENICE_KEY_COUNT_SIGNAL_HEADER` (kcs) and `VENICE_LEADER_KEY_COUNT_HEADER` (lkc). Test coverage: - `ActiveKeyCountHeartbeatTest`: encode/decode round-trip including Long.MIN_VALUE and Long.MAX_VALUE; `buildLeaderActiveKeyCountHeader` gating (check disabled, pre-EOP, untracked, null PCS, happy path); `compareLeaderActiveKeyCountOnHeartbeat` branches (check disabled, pre-EOP, follower not tracking, header absent, leader sentinel, match, mismatch, corrupt length); `sendIngestionHeartbeatToVT` ordering, upstream-failure propagation, sync send-failure propagation. - `ActiveKeyCountInvalidationReasonTest`: dimension fixture for all 7 enum values. - `ActiveKeyCountScenarioTest`: end-to-end Tehuti+OTel parity for both `recordActiveKeyCountInvalidation` and `recordActiveKeyCountMismatchAcrossReplicas`, plus cross-system isolation. - `PartitionConsumptionStateTest`: 32-thread chain atomicity test for `swapLastVTProduceCallFuture`; null-arg NPE test. - `LeaderFollowerStoreIngestionTaskTest`: bounded EOP-wait timeout, empty-view-writers + pending chain head short-circuit, view-writer/ CM-write chain-failure-propagation tests. - `IngestionOtelMetricEntityTest` / `IngestionOtelStatsTest` / `ServerMetricEntityTest` / `VeniceMetricsDimensionsTest`: updated for the new dimension and the new mismatch metric entity.
1 parent d2dd325 commit 2034234

27 files changed

Lines changed: 1869 additions & 165 deletions

clients/da-vinci-client/src/main/java/com/linkedin/davinci/config/VeniceServerConfig.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@
7171
import static com.linkedin.venice.ConfigKeys.SERVER_AA_WC_WORKLOAD_PARALLEL_PROCESSING_THREAD_POOL_SIZE;
7272
import static com.linkedin.venice.ConfigKeys.SERVER_ACTIVE_KEY_COUNT_FOR_ALL_BATCH_PUSH_ENABLED;
7373
import static com.linkedin.venice.ConfigKeys.SERVER_ACTIVE_KEY_COUNT_FOR_HYBRID_STORE_ENABLED;
74+
import static com.linkedin.venice.ConfigKeys.SERVER_ACTIVE_KEY_COUNT_REPLICA_CONSISTENCY_CHECK_ENABLED;
7475
import static com.linkedin.venice.ConfigKeys.SERVER_ADAPTIVE_THROTTLER_ENABLED;
7576
import static com.linkedin.venice.ConfigKeys.SERVER_ADAPTIVE_THROTTLER_MULTI_GET_LATENCY_THRESHOLD;
7677
import static com.linkedin.venice.ConfigKeys.SERVER_ADAPTIVE_THROTTLER_READ_COMPUTE_GET_LATENCY_THRESHOLD;
@@ -738,6 +739,7 @@ public class VeniceServerConfig extends VeniceClusterConfig {
738739
private final boolean addRmdToBatchPushForHybridStores;
739740
private final boolean activeKeyCountForAllBatchPushEnabled;
740741
private final boolean activeKeyCountForHybridStoreEnabled;
742+
private final boolean activeKeyCountReplicaConsistencyCheckEnabled;
741743
private final int partialUpdateLargeResultLogThresholdBytes;
742744
private final long partialUpdateAmplificationReportIntervalMs;
743745

@@ -1290,6 +1292,8 @@ public VeniceServerConfig(VeniceProperties serverProperties, Map<String, Map<Str
12901292
serverProperties.getBoolean(SERVER_ACTIVE_KEY_COUNT_FOR_ALL_BATCH_PUSH_ENABLED, false);
12911293
this.activeKeyCountForHybridStoreEnabled =
12921294
serverProperties.getBoolean(SERVER_ACTIVE_KEY_COUNT_FOR_HYBRID_STORE_ENABLED, false);
1295+
this.activeKeyCountReplicaConsistencyCheckEnabled =
1296+
serverProperties.getBoolean(SERVER_ACTIVE_KEY_COUNT_REPLICA_CONSISTENCY_CHECK_ENABLED, false);
12931297
this.partialUpdateLargeResultLogThresholdBytes =
12941298
serverProperties.getInt(PARTIAL_UPDATE_LARGE_RESULT_LOG_THRESHOLD_BYTES, 100 * 1024);
12951299
this.partialUpdateAmplificationReportIntervalMs =
@@ -2337,6 +2341,15 @@ public boolean isActiveKeyCountForHybridStoreEnabled() {
23372341
return activeKeyCountForHybridStoreEnabled;
23382342
}
23392343

2344+
/**
2345+
* @return {@code true} when the replica-consistency check will actually run — i.e. both the check flag
2346+
* ({@code server.active.key.count.replica.consistency.check.enabled}) AND its hybrid-tracking
2347+
* prerequisite ({@link #isActiveKeyCountForHybridStoreEnabled()}) are on.
2348+
*/
2349+
public boolean isActiveKeyCountReplicaConsistencyCheckEnabled() {
2350+
return activeKeyCountForHybridStoreEnabled && activeKeyCountReplicaConsistencyCheckEnabled;
2351+
}
2352+
23402353
public boolean isAnyActiveKeyCountTrackingEnabled() {
23412354
return activeKeyCountForAllBatchPushEnabled || activeKeyCountForHybridStoreEnabled;
23422355
}

clients/da-vinci-client/src/main/java/com/linkedin/davinci/kafka/consumer/ActiveActiveStoreIngestionTask.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -113,12 +113,14 @@ public class ActiveActiveStoreIngestionTask extends LeaderFollowerStoreIngestion
113113
static final byte KEY_CREATED_SIGNAL_VALUE = 1;
114114
static final byte KEY_DELETED_SIGNAL_VALUE = -1;
115115
static final byte KEY_COUNT_INVALIDATE_SIGNAL_VALUE = 0;
116-
static final PubSubMessageHeader KEY_CREATED_SIGNAL =
117-
new PubSubMessageHeader(StoreIngestionTask.KEY_COUNT_SIGNAL_HEADER, new byte[] { KEY_CREATED_SIGNAL_VALUE });
118-
static final PubSubMessageHeader KEY_DELETED_SIGNAL =
119-
new PubSubMessageHeader(StoreIngestionTask.KEY_COUNT_SIGNAL_HEADER, new byte[] { KEY_DELETED_SIGNAL_VALUE });
116+
static final PubSubMessageHeader KEY_CREATED_SIGNAL = new PubSubMessageHeader(
117+
PubSubMessageHeaders.VENICE_KEY_COUNT_SIGNAL_HEADER,
118+
new byte[] { KEY_CREATED_SIGNAL_VALUE });
119+
static final PubSubMessageHeader KEY_DELETED_SIGNAL = new PubSubMessageHeader(
120+
PubSubMessageHeaders.VENICE_KEY_COUNT_SIGNAL_HEADER,
121+
new byte[] { KEY_DELETED_SIGNAL_VALUE });
120122
static final PubSubMessageHeader KEY_COUNT_INVALIDATE_SIGNAL = new PubSubMessageHeader(
121-
StoreIngestionTask.KEY_COUNT_SIGNAL_HEADER,
123+
PubSubMessageHeaders.VENICE_KEY_COUNT_SIGNAL_HEADER,
122124
new byte[] { KEY_COUNT_INVALIDATE_SIGNAL_VALUE });
123125

124126
/** RMD bytes (ts=0) with superset schema ID prepended. For chunk manifests (superset schema ID known at construction). */
Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,30 @@
11
package com.linkedin.davinci.kafka.consumer;
22

3+
import static com.linkedin.venice.stats.dimensions.VeniceMetricsDimensions.VENICE_ACTIVE_KEY_COUNT_INVALIDATION_REASON;
4+
5+
import com.linkedin.venice.stats.dimensions.VeniceDimensionInterface;
6+
import com.linkedin.venice.stats.dimensions.VeniceMetricsDimensions;
7+
8+
39
/**
410
* Reasons for invalidating a partition's active-key-count tracking. Each value carries a message
511
* template (some with a {@code %d} placeholder for runtime detail like the offending signal value
612
* or header length) that becomes the prefix of the operator-visible ERROR log emitted by
713
* {@link StoreIngestionTask#invalidateActiveKeyCount}.
14+
*
15+
* <p>Also serves as an OTel dimension on
16+
* {@link com.linkedin.davinci.stats.ingestion.IngestionOtelMetricEntity#ACTIVE_KEY_COUNT_INVALIDATION}.
17+
* The Tehuti sensor remains a flat total.
818
*/
9-
public enum ActiveKeyCountInvalidationReason {
19+
public enum ActiveKeyCountInvalidationReason implements VeniceDimensionInterface {
1020
/** Follower received {@code kcs=-1} but its count was already zero (count drifted). */
1121
FOLLOWER_DECREMENT_UNDERFLOW("Decrement underflow on follower from kcs=-1"),
1222
/** Follower received {@code kcs=0} (leader-propagated invalidation signal). */
1323
LEADER_PROPAGATED_INVALIDATION("Leader propagated invalidation signal"),
1424
/** Follower received a single-byte {@code kcs} value outside the {-1, 0, +1} contract. */
15-
CORRUPT_KCS_SIGNAL_VALUE("Unexpected kcs signal value %d"),
16-
/** Follower received a multi-byte {@code kcs} header (corrupt or future producer). */
17-
CORRUPT_MULTI_BYTE_KCS_SIGNAL("Unexpected multi-byte kcs signal (length=%d)"),
25+
CORRUPT_KEY_COUNT_SIGNAL_HEADER_VALUE("Unexpected kcs signal value %d"),
26+
/** Follower received a {@code kcs} header with the wrong byte length (expected 1; corrupt or future producer). */
27+
CORRUPT_KEY_COUNT_SIGNAL_HEADER_LENGTH("Unexpected kcs header length=%d"),
1828
/** Leader detected an underflow during DCR (count was zero but a delete was processed). */
1929
LEADER_DCR_UNDERFLOW("Decrement underflow on leader during DCR"),
2030
/**
@@ -23,19 +33,40 @@ public enum ActiveKeyCountInvalidationReason {
2333
* The transient I/O failure must not stop ingestion or leave the active count in a wrong state —
2434
* invalidate so we stop publishing a stale value.
2535
*/
26-
KEY_EXISTS_FAILURE("RocksDB value column family lookup failed");
36+
KEY_EXISTS_FAILURE("RocksDB value column family lookup failed"),
37+
/** Follower received an {@code lkc} header with the wrong byte length (expected 8; corrupt or future producer). */
38+
CORRUPT_LEADER_KEY_COUNT_HEADER_LENGTH("Unexpected lkc header length=%d");
2739

2840
private final String messageTemplate;
41+
private final boolean templateWithExtraData;
2942

3043
ActiveKeyCountInvalidationReason(String messageTemplate) {
3144
this.messageTemplate = messageTemplate;
45+
this.templateWithExtraData = messageTemplate.contains("%d");
3246
}
3347

3448
String getMessage() {
49+
if (templateWithExtraData) {
50+
throw new IllegalStateException(
51+
"Reason " + name() + " carries a '%d' placeholder; caller must use getMessage(int detail).");
52+
}
3553
return messageTemplate;
3654
}
3755

3856
String getMessage(int detail) {
57+
if (!templateWithExtraData) {
58+
throw new IllegalStateException(
59+
"Reason " + name() + " has no '%d' placeholder; caller must use the no-arg getMessage().");
60+
}
3961
return String.format(messageTemplate, detail);
4062
}
63+
64+
/**
65+
* All instances of this enum share the same dimension name.
66+
* Refer to {@link VeniceDimensionInterface#getDimensionName()} for more details.
67+
*/
68+
@Override
69+
public VeniceMetricsDimensions getDimensionName() {
70+
return VENICE_ACTIVE_KEY_COUNT_INVALIDATION_REASON;
71+
}
4172
}

0 commit comments

Comments
 (0)