Skip to content

Commit 6c22d7c

Browse files
authored
[server][da-vinci] Add per-replica intended state guardrail to DefaultIngestionBackend (linkedin#2471)
Introduce a per-replica `ReplicaIntendedState` state machine in `DefaultIngestionBackend` to guard against duplicate or overlapping Helix state model transitions (start/stop/drop). The state machine prevents races such as duplicate starts, stops without prior start, and starts during an ongoing stop by tracking per-replica lifecycle states (`NOT_EXIST`, `RUNNING`, `STOPPED`) and enforcing correct transitions. - `startConsumption`: no-ops if already RUNNING; waits for STOPPED teardown to complete before proceeding and transitions to RUNNING. - `stopConsumption`: only proceeds from RUNNING and transitions to STOPPED; otherwise returns as a no-op. - `dropStoragePartitionGracefully`: performs stop+wait then sets state to NOT_EXIST and removes context. Added new unit and integration tests covering all state transitions and updated existing tests to work with the state machine. No user-facing or breaking changes introduced.
1 parent 40d4e90 commit 6c22d7c

3 files changed

Lines changed: 401 additions & 12 deletions

File tree

clients/da-vinci-client/src/main/java/com/linkedin/davinci/ingestion/DefaultIngestionBackend.java

Lines changed: 91 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import com.linkedin.davinci.store.StoragePartitionConfig;
1515
import com.linkedin.venice.exceptions.VenicePeersNotFoundException;
1616
import com.linkedin.venice.kafka.protocol.state.StoreVersionState;
17+
import com.linkedin.venice.meta.IngestionMode;
1718
import com.linkedin.venice.meta.Store;
1819
import com.linkedin.venice.meta.StoreVersionInfo;
1920
import com.linkedin.venice.meta.Version;
@@ -52,11 +53,15 @@ public class DefaultIngestionBackend implements IngestionBackend {
5253
private final Map<String, AtomicReference<StorageEngine>> topicStorageEngineReferenceMap =
5354
new VeniceConcurrentHashMap<>();
5455
private final BlobTransferManager blobTransferManager;
56+
private final boolean isIsolatedIngestion;
5557

5658
// Per-replica locks to ensure mutual exclusion between blob transfer triggerred consumption start and cancel
5759
// operations
5860
private final Map<String, Lock> consumptionLocks = new VeniceConcurrentHashMap<>();
5961

62+
// Per-replica consumption state tracking for coordinating start/stop/drop lifecycle
63+
private final Map<String, ReplicaConsumptionContext> replicaContexts = new VeniceConcurrentHashMap<>();
64+
6065
public DefaultIngestionBackend(
6166
StorageMetadataService storageMetadataService,
6267
KafkaStoreIngestionService storeIngestionService,
@@ -68,6 +73,7 @@ public DefaultIngestionBackend(
6873
this.storageService = storageService;
6974
this.blobTransferManager = blobTransferManager;
7075
this.serverConfig = serverConfig;
76+
this.isIsolatedIngestion = serverConfig != null && IngestionMode.ISOLATED.equals(serverConfig.getIngestionMode());
7177
}
7278

7379
@Override
@@ -82,6 +88,30 @@ public void startConsumption(
8288
Supplier<StoreVersionState> svsSupplier = () -> storageMetadataService.getStoreVersionState(storeVersion);
8389
syncStoreVersionConfig(storeAndVersion.getStore(), storeConfig);
8490

91+
String replicaId = Utils.getReplicaId(storeVersion, partition);
92+
93+
if (!isIsolatedIngestion) {
94+
ReplicaConsumptionContext replicaContext = getOrCreateReplicaContext(replicaId);
95+
96+
if (replicaContext.state == ReplicaIntendedState.RUNNING) {
97+
LOGGER.info("startConsumption called for replica {} but it is already RUNNING. Ignoring duplicate.", replicaId);
98+
return;
99+
}
100+
101+
if (replicaContext.state == ReplicaIntendedState.STOPPED) {
102+
LOGGER.info(
103+
"startConsumption: Waiting for blob transfer and PubSub consumption to stop for replica {}.",
104+
replicaId);
105+
// TODO: Refactor the ingestion service to take in blob ingestion/transfer, pubsub ingestion logic.
106+
int stopConsumptionTimeout = serverConfig.getStopConsumptionTimeoutInSeconds();
107+
stopBlobTransferAndWait(storeConfig, partition, stopConsumptionTimeout);
108+
getStoreIngestionService().stopConsumptionAndWait(storeConfig, partition, 1, stopConsumptionTimeout, false);
109+
}
110+
111+
replicaContext.state = ReplicaIntendedState.RUNNING;
112+
LOGGER.info("Replica {} state set to RUNNING.", replicaId);
113+
}
114+
85115
Runnable runnable = () -> {
86116
StorageEngine storageEngine = storageService.openStoreForNewPartition(storeConfig, partition, svsSupplier);
87117
topicStorageEngineReferenceMap.compute(storeVersion, (key, storageEngineAtomicReference) -> {
@@ -106,7 +136,6 @@ public void startConsumption(
106136
if (!blobTransferActiveInReceiver || blobTransferManager == null) {
107137
runnable.run();
108138
} else {
109-
String replicaId = Utils.getReplicaId(storeVersion, partition);
110139
// Status: null -> TRANSFER_NOT_STARTED
111140
blobTransferManager.getTransferStatusTrackingManager().initialTransfer(replicaId);
112141

@@ -438,6 +467,22 @@ public boolean isReplicaLaggedAndNeedBlobTransfer(
438467

439468
@Override
440469
public CompletableFuture<Void> stopConsumption(VeniceStoreVersionConfig storeConfig, int partition) {
470+
if (!isIsolatedIngestion) {
471+
String storeVersion = storeConfig.getStoreVersionName();
472+
String replicaId = Utils.getReplicaId(storeVersion, partition);
473+
ReplicaConsumptionContext replicaContext = getOrCreateReplicaContext(replicaId);
474+
475+
if (replicaContext.state != ReplicaIntendedState.RUNNING) {
476+
LOGGER.info(
477+
"stopConsumption called for replica {} but state is {} (not RUNNING). Skipping.",
478+
replicaId,
479+
replicaContext.state);
480+
return CompletableFuture.completedFuture(null);
481+
}
482+
replicaContext.state = ReplicaIntendedState.STOPPED;
483+
LOGGER.info("Replica {} state set to STOPPED.", replicaId);
484+
}
485+
441486
cancelBlobTransferIfInProgressInternal(storeConfig, partition);
442487
return getStoreIngestionService().stopConsumption(storeConfig, partition);
443488
}
@@ -499,12 +544,29 @@ public CompletableFuture<Void> dropStoragePartitionGracefully(
499544
int partition,
500545
int timeoutInSeconds,
501546
boolean removeEmptyStorageEngine) {
547+
String storeVersion = storeConfig.getStoreVersionName();
548+
String replicaId = Utils.getReplicaId(storeVersion, partition);
549+
if (!isIsolatedIngestion) {
550+
LOGGER.info(
551+
"dropStoragePartitionGracefully: Replica {} state {} before gracefully dropping",
552+
replicaId,
553+
getOrCreateReplicaContext(replicaId));
554+
}
555+
502556
// Stop consumption of the partition.
503557
final int waitIntervalInSecond = 1;
504558
final int maxRetry = timeoutInSeconds / waitIntervalInSecond;
505559
stopBlobTransferAndWait(storeConfig, partition, maxRetry);
506560
getStoreIngestionService().stopConsumptionAndWait(storeConfig, partition, waitIntervalInSecond, maxRetry, true);
507-
return getStoreIngestionService().dropStoragePartitionGracefully(storeConfig, partition);
561+
562+
try {
563+
return getStoreIngestionService().dropStoragePartitionGracefully(storeConfig, partition);
564+
} finally {
565+
if (!isIsolatedIngestion) {
566+
replicaContexts.remove(replicaId);
567+
LOGGER.info("dropStoragePartitionGracefully: Replica {} context removed.", replicaId);
568+
}
569+
}
508570
}
509571

510572
private void stopBlobTransferAndWait(VeniceStoreVersionConfig storeConfig, int partition, int timeoutInSeconds) {
@@ -588,6 +650,20 @@ StorageMetadataService getStorageMetadataService() {
588650
return storageMetadataService;
589651
}
590652

653+
private ReplicaConsumptionContext getOrCreateReplicaContext(String replicaId) {
654+
return replicaContexts.computeIfAbsent(replicaId, k -> new ReplicaConsumptionContext());
655+
}
656+
657+
// For test assertions
658+
ReplicaIntendedState getReplicaIntendedState(String replicaId) {
659+
ReplicaConsumptionContext context = replicaContexts.get(replicaId);
660+
return context == null ? ReplicaIntendedState.NOT_EXIST : context.state;
661+
}
662+
663+
void removeReplicaConsumptionContext(String replicaId) {
664+
replicaContexts.remove(replicaId);
665+
}
666+
591667
/**
592668
* This method is used to sync the store version config with on the store metadata obtained from ZK.
593669
* VeniceStoreVersionConfig was introduced to allow store-version level configs be configurable via a config file.
@@ -684,4 +760,17 @@ private boolean shouldEnableBlobTransfer(Store store) {
684760
// case 3: Default case, both are NOT_SPECIFIED or null, feature is disabled
685761
return false;
686762
}
763+
764+
enum ReplicaIntendedState {
765+
/** The replica has never been started on this host, or has been dropped. */
766+
NOT_EXIST,
767+
/** Consumption is active (blob transfer in progress or Kafka consumer subscribed). */
768+
RUNNING,
769+
/** A stop has been requested (but not yet dropped). */
770+
STOPPED
771+
}
772+
773+
static class ReplicaConsumptionContext {
774+
ReplicaIntendedState state = ReplicaIntendedState.NOT_EXIST;
775+
}
687776
}

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1483,11 +1483,15 @@ private boolean ingestionTaskHasAnySubscription(String topic) {
14831483
private void resetConsumptionOffset(VeniceStoreVersionConfig veniceStore, int partitionId) {
14841484
String topic = veniceStore.getStoreVersionName();
14851485
StoreIngestionTask consumerTask = topicNameToIngestionTaskMap.get(topic);
1486-
if (consumerTask != null && consumerTask.isRunning()) {
1487-
consumerTask.resetPartitionConsumptionOffset(
1488-
new PubSubTopicPartitionImpl(pubSubTopicRepository.getTopic(topic), partitionId));
1486+
try {
1487+
if (consumerTask != null && consumerTask.isRunning()) {
1488+
consumerTask.resetPartitionConsumptionOffset(
1489+
new PubSubTopicPartitionImpl(pubSubTopicRepository.getTopic(topic), partitionId));
1490+
}
1491+
LOGGER.info("Offset reset to beginning - Replica: {}.", Utils.getReplicaId(topic, partitionId));
1492+
} catch (Exception e) {
1493+
LOGGER.warn("Error resetting replica offset for replica: {}.", Utils.getReplicaId(topic, partitionId));
14891494
}
1490-
LOGGER.info("Offset reset to beginning - Replica: {}.", Utils.getReplicaId(topic, partitionId));
14911495
}
14921496

14931497
@VisibleForTesting

0 commit comments

Comments
 (0)