From 75e07651a4e4c963c2969ba0d1f4d3e24b0f8043 Mon Sep 17 00:00:00 2001 From: Kai-Sern Lim Date: Tue, 21 Jul 2026 22:18:18 -0700 Subject: [PATCH 1/5] Fix version lifecycle leaks Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../stats/AggVersionedStorageEngineStats.java | 18 + .../stats/StorageEngineOtelMetricEntity.java | 11 +- .../davinci/stats/StorageEngineOtelStats.java | 65 +- .../davinci/storage/StorageService.java | 2 + .../AggVersionedStorageEngineStatsTest.java | 25 + .../StorageEngineOtelMetricEntityTest.java | 10 +- .../stats/StorageEngineOtelStatsTest.java | 46 +- .../davinci/storage/StorageServiceTest.java | 42 + .../data-management/version-lifecycle.md | 103 ++ docs/operations/index.md | 2 + .../AsyncMetricEntityStateOneEnum.java | 13 +- .../AsyncMetricEntityStateTwoEnums.java | 14 +- .../AsyncMetricEntityStateOneEnumTest.java | 18 + .../AsyncMetricEntityStateTwoEnumsTest.java | 19 + .../linkedin/venice/meta/AbstractStore.java | 14 +- .../avro/AvroProtocolDefinition.java | 2 +- mkdocs.yml | 1 + .../DeferredVersionSwapService.java | 205 ++- .../StoreBackupVersionCleanupService.java | 25 +- .../venice/controller/VeniceHelixAdmin.java | 70 + .../controller/VeniceParentHelixAdmin.java | 29 + .../kafka/consumer/AdminExecutionTask.java | 13 + .../protocol/enums/AdminMessageType.java | 6 +- .../AdminOperation/v102/AdminOperation.avsc | 1370 +++++++++++++++++ ...rsionSwapServiceWithSequentialRollout.java | 319 ++++ .../TestStoreBackupVersionCleanupService.java | 49 + .../controller/TestVeniceHelixAdmin.java | 58 + .../consumer/AdminExecutionTaskTest.java | 74 + 28 files changed, 2556 insertions(+), 67 deletions(-) create mode 100644 docs/operations/data-management/version-lifecycle.md create mode 100644 services/venice-controller/src/main/resources/avro/AdminOperation/v102/AdminOperation.avsc diff --git a/clients/da-vinci-client/src/main/java/com/linkedin/davinci/stats/AggVersionedStorageEngineStats.java b/clients/da-vinci-client/src/main/java/com/linkedin/davinci/stats/AggVersionedStorageEngineStats.java index 6692fb885ae..d059e887099 100644 --- a/clients/da-vinci-client/src/main/java/com/linkedin/davinci/stats/AggVersionedStorageEngineStats.java +++ b/clients/da-vinci-client/src/main/java/com/linkedin/davinci/stats/AggVersionedStorageEngineStats.java @@ -93,6 +93,24 @@ public void setStorageEngine(String topicName, StorageEngine storageEngine) { } } + public void unsetStorageEngine(String topicName) { + if (!Version.isVersionTopicOrStreamReprocessingTopic(topicName)) { + LOGGER.warn("Invalid topic name: {}", topicName); + return; + } + String storeName = Version.parseStoreFromKafkaTopicName(topicName); + int version = Version.parseVersionFromKafkaTopicName(topicName); + try { + getStats(storeName, version).setStorageEngine(null); + otelStatsMap.computeIfPresent(storeName, (k, stats) -> { + stats.onVersionRemoved(version); + return stats; + }); + } catch (Exception e) { + LOGGER.warn("Failed to unset StorageEngine for store: {}, version: {}", storeName, version, e); + } + } + public void recordRocksDBOpenFailure(String topicName) { if (!Version.isVersionTopicOrStreamReprocessingTopic(topicName)) { LOGGER.warn("Invalid topic name: {}", topicName); diff --git a/clients/da-vinci-client/src/main/java/com/linkedin/davinci/stats/StorageEngineOtelMetricEntity.java b/clients/da-vinci-client/src/main/java/com/linkedin/davinci/stats/StorageEngineOtelMetricEntity.java index eb0b5fd2164..7feac1573ca 100644 --- a/clients/da-vinci-client/src/main/java/com/linkedin/davinci/stats/StorageEngineOtelMetricEntity.java +++ b/clients/da-vinci-client/src/main/java/com/linkedin/davinci/stats/StorageEngineOtelMetricEntity.java @@ -17,10 +17,11 @@ /** * OTel metric entities for storage engine statistics. * - *

Consolidates 4 Tehuti AsyncGauge sensors into 3 OTel metrics: + *

Consolidates storage-engine Tehuti sensors and adds bounded role-level visibility: *

@@ -28,10 +29,16 @@ public enum StorageEngineOtelMetricEntity implements ModuleMetricEntityInterface { DISK_USAGE( "ingestion.disk.used", MetricType.ASYNC_GAUGE, MetricUnit.BYTES, - "Disk usage in bytes by record type (data or replication metadata)", + "Total disk usage in bytes across all versions with each role, by record type", setOf(VENICE_CLUSTER_NAME, VENICE_STORE_NAME, VENICE_VERSION_ROLE, VENICE_RECORD_TYPE) ), + VERSION_COUNT( + "ingestion.disk.version_count", MetricType.ASYNC_GAUGE, MetricUnit.NUMBER, + "Number of locally loaded storage-engine versions by role", + setOf(VENICE_CLUSTER_NAME, VENICE_STORE_NAME, VENICE_VERSION_ROLE) + ), + ROCKSDB_OPEN_FAILURE_COUNT( "rocksdb.open.failure_count", MetricType.COUNTER, MetricUnit.NUMBER, "Count of RocksDB open failures; VERSION_ROLE reflects the version's role at failure time, not its current role", diff --git a/clients/da-vinci-client/src/main/java/com/linkedin/davinci/stats/StorageEngineOtelStats.java b/clients/da-vinci-client/src/main/java/com/linkedin/davinci/stats/StorageEngineOtelStats.java index b5cd40e1da1..2a5d586a2d2 100644 --- a/clients/da-vinci-client/src/main/java/com/linkedin/davinci/stats/StorageEngineOtelStats.java +++ b/clients/da-vinci-client/src/main/java/com/linkedin/davinci/stats/StorageEngineOtelStats.java @@ -5,6 +5,7 @@ import static com.linkedin.davinci.stats.StorageEngineOtelMetricEntity.DISK_USAGE; import static com.linkedin.davinci.stats.StorageEngineOtelMetricEntity.KEY_COUNT_ESTIMATE; import static com.linkedin.davinci.stats.StorageEngineOtelMetricEntity.ROCKSDB_OPEN_FAILURE_COUNT; +import static com.linkedin.davinci.stats.StorageEngineOtelMetricEntity.VERSION_COUNT; import static com.linkedin.venice.meta.Store.NON_EXISTING_VERSION; import com.linkedin.davinci.stats.AggVersionedStorageEngineStats.StorageEngineStatsWrapper; @@ -26,10 +27,11 @@ /** * Per-store OTel stats for storage engine metrics. * - *

Holds 3 OTel metrics: + *

Holds 4 OTel metrics: *

@@ -61,6 +63,9 @@ public class StorageEngineOtelStats implements Closeable { /** Key count ASYNC_GAUGE with VersionRole dimension */ private final AsyncMetricEntityStateOneEnum keyCountMetric; + /** Number of local storage engines with each VersionRole */ + private final AsyncMetricEntityStateOneEnum versionCountMetric; + /** RocksDB open failure COUNTER with VersionRole dimension */ private final MetricEntityStateOneEnum openFailureMetric; @@ -78,9 +83,8 @@ public StorageEngineOtelStats(MetricsRepository metricsRepository, String storeN Map baseDimensionsMap = otelSetup.getBaseDimensionsMap(); /* - * Two-callback contract: the liveStateResolver returns the wrapper or null (null -> dormant, - * no emission); the valueResolver reads the metric value from that wrapper. The null return - * is the liveness signal, enforced by the API. + * The live-state resolver returns the bounded per-version map when a role is present, or null + * when dormant. The value resolver aggregates that role without creating per-version attributes. */ this.diskUsageMetrics = AsyncMetricEntityStateTwoEnums.create( DISK_USAGE.getMetricEntity(), @@ -88,8 +92,8 @@ public StorageEngineOtelStats(MetricsRepository metricsRepository, String storeN baseDimensionsMap, VeniceRecordType.class, VersionRole.class, - (recordType, role) -> getWrapperForRole(role), - (wrapper, recordType, role) -> diskUsage(wrapper, recordType)); + (recordType, role) -> hasVersionForRole(role) ? wrappersByVersion : null, + (wrappers, recordType, role) -> diskUsageForRole(wrappers, role, recordType)); this.keyCountMetric = AsyncMetricEntityStateOneEnum.create( KEY_COUNT_ESTIMATE.getMetricEntity(), @@ -99,12 +103,21 @@ public StorageEngineOtelStats(MetricsRepository metricsRepository, String storeN role -> getWrapperForRole(role), (wrapper, role) -> wrapper.getKeyCountEstimate()); + this.versionCountMetric = AsyncMetricEntityStateOneEnum.create( + VERSION_COUNT.getMetricEntity(), + otelRepository, + baseDimensionsMap, + VersionRole.class, + role -> wrappersByVersion.isEmpty() ? null : wrappersByVersion, + (wrappers, role) -> countVersionsForRole(wrappers, role)); + // RocksDB open failure count: COUNTER with VersionRole dimension this.openFailureMetric = MetricEntityStateOneEnum .create(ROCKSDB_OPEN_FAILURE_COUNT.getMetricEntity(), otelRepository, baseDimensionsMap, VersionRole.class); } else { this.diskUsageMetrics = null; this.keyCountMetric = null; + this.versionCountMetric = null; this.openFailureMetric = null; } } @@ -175,6 +188,33 @@ private StorageEngineStatsWrapper getWrapperForRole(VersionRole role) { return wrappersByVersion.get(version); } + private boolean hasVersionForRole(VersionRole role) { + return countVersionsForRole(wrappersByVersion, role) > 0; + } + + private long countVersionsForRole(Map wrappers, VersionRole role) { + VersionInfo snapshot = versionInfo; + return wrappers.keySet().stream().filter(version -> classifyVersion(version, snapshot) == role).count(); + } + + private long diskUsageForRole( + Map wrappers, + VersionRole role, + VeniceRecordType recordType) { + VersionInfo snapshot = versionInfo; + if (role == VersionRole.BACKUP) { + return wrappers.entrySet() + .stream() + .filter(entry -> classifyVersion(entry.getKey(), snapshot) == VersionRole.BACKUP) + .mapToLong(entry -> diskUsage(entry.getValue(), recordType)) + .sum(); + } + + int version = getVersionForRole(role, snapshot, wrappers.keySet()); + StorageEngineStatsWrapper wrapper = wrappers.get(version); + return wrapper == null ? 0 : diskUsage(wrapper, recordType); + } + /** Reads disk usage (data or RMD) from a resolved wrapper. */ private static long diskUsage(StorageEngineStatsWrapper wrapper, VeniceRecordType recordType) { switch (recordType) { @@ -187,14 +227,15 @@ private static long diskUsage(StorageEngineStatsWrapper wrapper, VeniceRecordTyp } } - /** - * Clears internal wrapper references. On subsequent collections each async-gauge's - * {@code liveStateResolver} will return {@code null} for every role and no data points will be - * emitted. The SDK instruments themselves are NOT deregistered — they remain registered and - * are polled until the SDK is shut down. - */ + /** Stops observable callbacks and clears all storage-engine references. */ @Override public void close() { wrappersByVersion.clear(); + versionInfo = VersionInfo.NON_EXISTING; + if (emitOtelMetrics) { + diskUsageMetrics.close(); + keyCountMetric.close(); + versionCountMetric.close(); + } } } diff --git a/clients/da-vinci-client/src/main/java/com/linkedin/davinci/storage/StorageService.java b/clients/da-vinci-client/src/main/java/com/linkedin/davinci/storage/StorageService.java index a423372ec62..da64baa94ce 100644 --- a/clients/da-vinci-client/src/main/java/com/linkedin/davinci/storage/StorageService.java +++ b/clients/da-vinci-client/src/main/java/com/linkedin/davinci/storage/StorageService.java @@ -526,6 +526,7 @@ public synchronized void removeStorageEngine(String kafkaTopic) { LOGGER.warn("Storage engine {} does not exist, ignoring remove request.", kafkaTopic); return; } + aggVersionedStorageEngineStats.unsetStorageEngine(kafkaTopic); storageEngine.drop(); VeniceStoreVersionConfig storeConfig = configLoader.getStoreConfig(kafkaTopic); @@ -552,6 +553,7 @@ public synchronized void closeStorageEngine(String kafkaTopic) { LOGGER.warn("Storage engine {} does not exist, ignoring close request.", kafkaTopic); return; } + aggVersionedStorageEngineStats.unsetStorageEngine(kafkaTopic); storageEngine.close(); VeniceStoreVersionConfig storeConfig = configLoader.getStoreConfig(kafkaTopic); diff --git a/clients/da-vinci-client/src/test/java/com/linkedin/davinci/stats/AggVersionedStorageEngineStatsTest.java b/clients/da-vinci-client/src/test/java/com/linkedin/davinci/stats/AggVersionedStorageEngineStatsTest.java index e2b79865ea3..c5d7d09569d 100644 --- a/clients/da-vinci-client/src/test/java/com/linkedin/davinci/stats/AggVersionedStorageEngineStatsTest.java +++ b/clients/da-vinci-client/src/test/java/com/linkedin/davinci/stats/AggVersionedStorageEngineStatsTest.java @@ -84,6 +84,31 @@ public void testSetStorageEngineWiresOtelStats() { } } + @Test + public void testUnsetStorageEngineRemovesAndReopenRestoresOtelStats() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + try (VeniceMetricsRepository veniceRepo = createOtelMetricsRepository(reader)) { + OtelTestContext ctx = createOtelTestContext(veniceRepo); + StorageEngineStats mockEngineStats = mock(StorageEngineStats.class); + doReturn(5000L).when(mockEngineStats).getStoreSizeInBytes(); + StorageEngine mockEngine = mock(StorageEngine.class); + doReturn(mockEngineStats).when(mockEngine).getStats(); + Attributes attrs = buildDiskUsageDataAttrs(ctx.clusterName, ctx.storeName, VersionRole.BACKUP); + String diskMetric = StorageEngineOtelMetricEntity.DISK_USAGE.getMetricEntity().getMetricName(); + + ctx.stats.setStorageEngine(ctx.topicName, mockEngine); + OpenTelemetryDataTestUtils.validateLongPointDataFromGauge(reader, 5000, attrs, diskMetric, OTEL_PREFIX); + + ctx.stats.unsetStorageEngine(ctx.topicName); + LongPointData pointAfterClose = OpenTelemetryDataTestUtils + .getLongPointDataFromGaugeIfPresent(reader.collectAllMetrics(), diskMetric, OTEL_PREFIX, attrs); + Assert.assertNull(pointAfterClose, "Closed local engine must not emit a version metric"); + + ctx.stats.setStorageEngine(ctx.topicName, mockEngine); + OpenTelemetryDataTestUtils.validateLongPointDataFromGauge(reader, 5000, attrs, diskMetric, OTEL_PREFIX); + } + } + @Test public void testRecordRocksDBOpenFailureWiresBothTehutiAndOtel() { InMemoryMetricReader reader = InMemoryMetricReader.create(); diff --git a/clients/da-vinci-client/src/test/java/com/linkedin/davinci/stats/StorageEngineOtelMetricEntityTest.java b/clients/da-vinci-client/src/test/java/com/linkedin/davinci/stats/StorageEngineOtelMetricEntityTest.java index d3a819f5428..e281176b649 100644 --- a/clients/da-vinci-client/src/test/java/com/linkedin/davinci/stats/StorageEngineOtelMetricEntityTest.java +++ b/clients/da-vinci-client/src/test/java/com/linkedin/davinci/stats/StorageEngineOtelMetricEntityTest.java @@ -29,8 +29,16 @@ private static Map expec "ingestion.disk.used", MetricType.ASYNC_GAUGE, MetricUnit.BYTES, - "Disk usage in bytes by record type (data or replication metadata)", + "Total disk usage in bytes across all versions with each role, by record type", setOf(VENICE_CLUSTER_NAME, VENICE_STORE_NAME, VENICE_VERSION_ROLE, VENICE_RECORD_TYPE))); + map.put( + StorageEngineOtelMetricEntity.VERSION_COUNT, + new MetricEntityExpectation( + "ingestion.disk.version_count", + MetricType.ASYNC_GAUGE, + MetricUnit.NUMBER, + "Number of locally loaded storage-engine versions by role", + setOf(VENICE_CLUSTER_NAME, VENICE_STORE_NAME, VENICE_VERSION_ROLE))); map.put( StorageEngineOtelMetricEntity.ROCKSDB_OPEN_FAILURE_COUNT, new MetricEntityExpectation( diff --git a/clients/da-vinci-client/src/test/java/com/linkedin/davinci/stats/StorageEngineOtelStatsTest.java b/clients/da-vinci-client/src/test/java/com/linkedin/davinci/stats/StorageEngineOtelStatsTest.java index aa35a118fde..5ed869b80bb 100644 --- a/clients/da-vinci-client/src/test/java/com/linkedin/davinci/stats/StorageEngineOtelStatsTest.java +++ b/clients/da-vinci-client/src/test/java/com/linkedin/davinci/stats/StorageEngineOtelStatsTest.java @@ -32,6 +32,8 @@ public class StorageEngineOtelStatsTest { private static final String DISK_USAGE_METRIC = StorageEngineOtelMetricEntity.DISK_USAGE.getMetricEntity().getMetricName(); + private static final String VERSION_COUNT_METRIC = + StorageEngineOtelMetricEntity.VERSION_COUNT.getMetricEntity().getMetricName(); private static final String KEY_COUNT_METRIC = StorageEngineOtelMetricEntity.KEY_COUNT_ESTIMATE.getMetricEntity().getMetricName(); private static final String OPEN_FAILURE_METRIC = @@ -144,17 +146,23 @@ public void testDiskUsageRmdBackupVersion() { } @Test - public void testDiskUsageBackupSelectsSmallestVersion() { - // current=1, future=2. Versions 3 and 5 are both backups — should select version 3 (smallest) + public void testDiskUsageAggregatesAllBackupVersions() { + // current=1, future=2. Versions 3 and 5 are both backups. stats.setStatsWrapper(3, new MockWrapper(3000, 300, 30)); stats.setStatsWrapper(5, new MockWrapper(5000, 500, 50)); OpenTelemetryDataTestUtils.validateLongPointDataFromGauge( inMemoryMetricReader, - 3000, + 8000, buildDiskUsageAttributes(VersionRole.BACKUP, VeniceRecordType.DATA), DISK_USAGE_METRIC, METRIC_PREFIX); + OpenTelemetryDataTestUtils.validateLongPointDataFromGauge( + inMemoryMetricReader, + 800, + buildDiskUsageAttributes(VersionRole.BACKUP, VeniceRecordType.REPLICATION_METADATA), + DISK_USAGE_METRIC, + METRIC_PREFIX); } @Test @@ -326,6 +334,29 @@ public void testLiveValueUpdatesAfterVersionInfoChange() { METRIC_PREFIX); } + @Test + public void testVersionCountTracksAddRemoveAndRoleChanges() { + stats.setStatsWrapper(1, new MockWrapper(1000, 100, 10)); + stats.setStatsWrapper(2, new MockWrapper(2000, 200, 20)); + stats.setStatsWrapper(3, new MockWrapper(3000, 300, 30)); + stats.setStatsWrapper(5, new MockWrapper(5000, 500, 50)); + + assertVersionCount(VersionRole.CURRENT, 1); + assertVersionCount(VersionRole.FUTURE, 1); + assertVersionCount(VersionRole.BACKUP, 2); + + stats.onVersionRemoved(3); + assertVersionCount(VersionRole.BACKUP, 1); + + stats.updateVersionInfo(5, 2); + assertVersionCount(VersionRole.CURRENT, 1); + assertVersionCount(VersionRole.FUTURE, 1); + assertVersionCount(VersionRole.BACKUP, 1); + + stats.onVersionRemoved(1); + assertVersionCount(VersionRole.BACKUP, 0); + } + @Test public void testRemoveVersionClearsWrapper() { AggVersionedStorageEngineStats.StorageEngineStatsWrapper wrapper = new MockWrapper(1000, 100, 10); @@ -394,6 +425,15 @@ private static Attributes buildVersionRoleAttributes(VersionRole role) { .build(); } + private void assertVersionCount(VersionRole role, long expectedCount) { + OpenTelemetryDataTestUtils.validateLongPointDataFromGauge( + inMemoryMetricReader, + expectedCount, + buildVersionRoleAttributes(role), + VERSION_COUNT_METRIC, + METRIC_PREFIX); + } + @Test public void testVersionRoleEnumCount() { // getVersionForRole returns NON_EXISTING_VERSION for unknown roles — but a new VersionRole diff --git a/clients/da-vinci-client/src/test/java/com/linkedin/davinci/storage/StorageServiceTest.java b/clients/da-vinci-client/src/test/java/com/linkedin/davinci/storage/StorageServiceTest.java index 8832dd3015b..8234d9422e9 100644 --- a/clients/da-vinci-client/src/test/java/com/linkedin/davinci/storage/StorageServiceTest.java +++ b/clients/da-vinci-client/src/test/java/com/linkedin/davinci/storage/StorageServiceTest.java @@ -16,6 +16,7 @@ import com.linkedin.davinci.config.VeniceStoreVersionConfig; import com.linkedin.davinci.stats.AggVersionedStorageEngineStats; import com.linkedin.davinci.stats.RocksDBMemoryStats; +import com.linkedin.davinci.store.DelegatingStorageEngine; import com.linkedin.davinci.store.StorageEngine; import com.linkedin.davinci.store.StorageEngineFactory; import com.linkedin.venice.exceptions.VeniceNoStoreException; @@ -76,6 +77,47 @@ public void testDeleteStorageEngineOnRocksDBError() { verify(factory, times(2)).removeStorageEngine(storageEngineName); } + @Test + public void testRemoveAndCloseStorageEngineDetachStats() { + VeniceConfigLoader configLoader = mock(VeniceConfigLoader.class); + VeniceServerConfig serverConfig = mock(VeniceServerConfig.class); + when(serverConfig.getDataBasePath()).thenReturn("/tmp"); + when(configLoader.getVeniceServerConfig()).thenReturn(serverConfig); + VeniceStoreVersionConfig storeConfig = mock(VeniceStoreVersionConfig.class); + when(storeConfig.getStorePersistenceType()).thenReturn(PersistenceType.BLACK_HOLE); + when(configLoader.getStoreConfig(storageEngineName)).thenReturn(storeConfig); + + AggVersionedStorageEngineStats storageEngineStats = mock(AggVersionedStorageEngineStats.class); + StorageEngineFactory factory = mock(StorageEngineFactory.class); + Map factories = new HashMap<>(); + factories.put(PersistenceType.BLACK_HOLE, factory); + StorageService storageService = new StorageService( + configLoader, + storageEngineStats, + mock(RocksDBMemoryStats.class), + mock(InternalAvroSpecificSerializer.class), + mock(InternalAvroSpecificSerializer.class), + storeRepository, + false, + false, + ignored -> true, + Optional.of(factories)); + + DelegatingStorageEngine storageEngine = mock(DelegatingStorageEngine.class); + when(storageEngine.getStoreVersionName()).thenReturn(storageEngineName); + when(storageEngine.getType()).thenReturn(PersistenceType.BLACK_HOLE); + storageService.getStorageEngineRepository().addLocalStorageEngine(storageEngine); + + storageService.removeStorageEngine(storageEngineName); + verify(storageEngineStats).unsetStorageEngine(storageEngineName); + verify(storageEngine).drop(); + + storageService.getStorageEngineRepository().addLocalStorageEngine(storageEngine); + storageService.closeStorageEngine(storageEngineName); + verify(storageEngineStats, times(2)).unsetStorageEngine(storageEngineName); + verify(storageEngine).close(); + } + @Test public void testGetStoreAndUserPartitionsMapping() { VeniceConfigLoader configLoader = mock(VeniceConfigLoader.class); diff --git a/docs/operations/data-management/version-lifecycle.md b/docs/operations/data-management/version-lifecycle.md new file mode 100644 index 00000000000..2b53b293a66 --- /dev/null +++ b/docs/operations/data-management/version-lifecycle.md @@ -0,0 +1,103 @@ +# Version Lifecycle and Deletion + +Venice protects versions that may still serve reads or complete an in-flight push, while cleaning +terminal non-current copies by count and time. A version's status alone is not enough to decide deletion: +the controller also considers whether it is current, the newest completed bootstrap (future), a +backup, within configured retention, or part of a store migration. + +## Deletion decision table + +The count-based sweep is implemented by `Store.retrieveVersionsToDelete`. The time-based sweep is +implemented by `StoreBackupVersionCleanupService`. Deferred version swap terminal transitions first +reconcile bootstrap-complete, non-current child copies to `ROLLED_BACK`, so they cannot remain +invisible to both sweeps. + +| Initial status | Role / initial condition | Count retention | Time / safety gate | Trigger | Deletion decision | +| --- | --- | --- | --- | --- | --- | +| Any | Current version | Any | Any | Any cleanup | **KEEP** | +| `NOT_CREATED` or `CREATED` | Non-current version at or above current | Not eligible | Not below current | Any cleanup | **KEEP** | +| `NOT_CREATED` or `CREATED` | Below current before standard cleanup is ready | Not eligible | Minimum/default retention gate not met | Time sweep | **DEFER** | +| `NOT_CREATED` or `CREATED` | Below current after standard cleanup is ready | Not eligible | Minimum delay met with multiple eligible backups, or retention expired | Time sweep | **DELETE** | +| `STARTED` | Newest version (active ingestion) | Protected | Not below current | Any cleanup | **KEEP** | +| `STARTED` | Older version at or above current while store is migrating | Protected during migration | Not below current | Any cleanup | **KEEP** | +| `STARTED` | Below current while migrating, before standard cleanup is ready | Protected during migration | Minimum/default retention gate not met | Time sweep | **DEFER** | +| `STARTED` | Below current while migrating, after standard cleanup is ready | Protected from count only | Minimum delay met with multiple eligible backups, or retention expired | Time sweep | **DELETE** | +| `STARTED` | Older non-current version, store is not migrating | Not protected | Not eligible | Count sweep | **DELETE** | +| `PUSHED` | Non-current version without a terminal parent decision | Excluded because it may be an active deferred or concurrent swap candidate | Not eligible | Count or time cleanup | **KEEP** | +| `PUSHED` or `ONLINE` | Still current in a child after parent `ERROR` or `ROLLED_BACK` | Protected while serving; reconciliation remains incomplete | Not eligible | Parent terminal reconciliation | **DEFER: keep and retry** | +| `PUSHED` or `ONLINE` | Still current in a child after parent `PARTIALLY_ONLINE` | Serving the intentional partial state | Not eligible | Parent terminal reconciliation | **KEEP** | +| `PUSHED` or non-current `ONLINE` | Parent deferred swap becomes `ERROR`, `PARTIALLY_ONLINE`, or `ROLLED_BACK` | Removed from count sweep | Rolled-back retention gate applies | Parent terminal transition | **DEFER: mark `ROLLED_BACK`** | +| `ONLINE` | Backup within the configured preserved count | Within limit | Not considered | Count sweep | **KEEP** | +| `ONLINE` | Backup beyond the configured preserved count | Exceeds limit | Not considered | Count sweep | **DELETE** | +| `ONLINE` | Backup considered by retention cleanup | Not considered | Before minimum retention | Time sweep | **DEFER** | +| `ONLINE` | Backup considered by retention cleanup | Not considered | Routers or servers do not agree on current version | Time sweep | **DEFER** | +| `ONLINE` | Eligible old backup | Not considered | Retention expired and metadata validation passes | Time sweep | **DELETE** | +| `ERROR` or `KILLED` | Non-current version | Not protected | Minimum cleanup safety gate where applicable | Immediate or periodic cleanup | **DELETE** | +| `ROLLED_BACK` | Non-current version | Excluded from count sweep | Before rolled-back retention expires | Time sweep | **DEFER** | +| `ROLLED_BACK` | Non-current version | Excluded from count sweep | Rolled-back retention expired | Time sweep | **DELETE** | + +Count-based and time-based cleanup are independent triggers. The first applicable trigger may delete +an eligible backup, but neither trigger may delete the current version. `PUSHED` versions are never +deleted from status and version number alone because multiple deferred or concurrent swaps can be +active. A terminal parent decision explicitly converts bootstrap-complete non-current child copies +to `ROLLED_BACK`. The controller scans every deferred terminal parent version, including versions +superseded by a newer push, and retries while a child is unreachable, missing the target metadata, +in progress, or unexpectedly still current after parent `ERROR` or `ROLLED_BACK`. + +When a child copy becomes `ROLLED_BACK`, the controller resets the store-level latest-promotion +timestamp to start the rollback retention window. A subsequent promotion can reset that shared clock +again, so a rolled-back version may be retained longer than the configured duration; it cannot be +deleted immediately because the current version was promoted long before the rollback. + +## State machine + +```mermaid +stateDiagram-v2 + [*] --> NOT_CREATED + NOT_CREATED --> CREATED: metadata initialized + CREATED --> STARTED: ingestion starts + STARTED --> PUSHED: bootstrap completes + STARTED --> ERROR: push fails + STARTED --> KILLED: push is killed + PUSHED --> ONLINE: version swap succeeds + PUSHED --> ROLLED_BACK: deferred swap terminates while non-current + ONLINE --> ROLLED_BACK: rollback or abandoned non-current copy + + NOT_CREATED --> DELETED: stale below-current metadata after time gate + CREATED --> DELETED: stale below-current metadata after time gate + STARTED --> DELETED: stale backup via count or time + ONLINE --> DELETED: backup exceeds count or time retention + ERROR --> DELETED: immediate or periodic cleanup + KILLED --> DELETED: immediate or periodic cleanup + ROLLED_BACK --> DELETED: rolled-back retention expires + + note right of STARTED + Newest active ingestion: KEEP + Migrating copy at/above current: KEEP + Below current: time policy still applies + end note + + note right of PUSHED + Before terminal parent decision: KEEP + Current after ERROR/ROLLED_BACK: retry + Current after PARTIALLY_ONLINE: KEEP + Parent terminal + non-current: ROLLED_BACK + end note + + note right of ONLINE + Current: always KEEP + Backup: count/time policy + end note + + note right of ROLLED_BACK + Before retention expiry: DEFER + end note +``` + +## Backup observability + +The `ingestion.disk.used` gauge sums disk usage across every local version with the same role, so the +`backup` series represents cumulative backup disk rather than one representative version. The +`ingestion.disk.version_count` gauge reports the number of locally loaded storage engines by +`current`, `future`, and `backup` role. Both metrics use bounded role dimensions rather than a +per-version dimension. diff --git a/docs/operations/index.md b/docs/operations/index.md index 901c0a7faf1..2f275e87887 100644 --- a/docs/operations/index.md +++ b/docs/operations/index.md @@ -6,6 +6,8 @@ This guide covers operational tasks for Venice administrators and operators. - [Repush](data-management/repush.md) - Re-ingest data from source of truth to repair inconsistencies or apply schema changes +- [Version Lifecycle and Deletion](data-management/version-lifecycle.md) - Version states, retention gates, and deletion + decisions - [TTL](data-management/ttl.md) - Configure time-to-live to automatically expire old records - [System Stores](data-management/system-stores.md) - Internal stores used by Venice for metadata and coordination diff --git a/internal/venice-client-common/src/main/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateOneEnum.java b/internal/venice-client-common/src/main/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateOneEnum.java index 52e002bb0bb..ede5f075280 100644 --- a/internal/venice-client-common/src/main/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateOneEnum.java +++ b/internal/venice-client-common/src/main/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateOneEnum.java @@ -6,6 +6,8 @@ import com.linkedin.venice.stats.metrics.AsyncMetricResolvers.LiveStateResolverOneEnum; import com.linkedin.venice.stats.metrics.AsyncMetricResolvers.ValueResolverOneEnum; import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.ObservableDoubleGauge; +import io.opentelemetry.api.metrics.ObservableLongGauge; import java.util.EnumMap; import java.util.Map; import java.util.function.ObjDoubleConsumer; @@ -36,7 +38,7 @@ * cost is {@code O(|E|)} {@code liveStateResolver} calls plus one {@code measurement.record(...)} * per emitted combo. */ -public class AsyncMetricEntityStateOneEnum & VeniceDimensionInterface> { +public class AsyncMetricEntityStateOneEnum & VeniceDimensionInterface> implements AutoCloseable { private final boolean emitOpenTelemetryMetrics; /** Precomputed per-enum attributes; {@code null} when OTel is disabled. */ private final EnumMap attributesByEnum; @@ -177,4 +179,13 @@ public EnumMap getAttributesByEnum() { public Object getInstrument() { return instrument; } + + @Override + public void close() { + if (instrument instanceof ObservableLongGauge) { + ((ObservableLongGauge) instrument).close(); + } else if (instrument instanceof ObservableDoubleGauge) { + ((ObservableDoubleGauge) instrument).close(); + } + } } diff --git a/internal/venice-client-common/src/main/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateTwoEnums.java b/internal/venice-client-common/src/main/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateTwoEnums.java index 31bd8eed731..0f67e2114cc 100644 --- a/internal/venice-client-common/src/main/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateTwoEnums.java +++ b/internal/venice-client-common/src/main/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateTwoEnums.java @@ -6,6 +6,8 @@ import com.linkedin.venice.stats.metrics.AsyncMetricResolvers.LiveStateResolverTwoEnums; import com.linkedin.venice.stats.metrics.AsyncMetricResolvers.ValueResolverTwoEnums; import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.ObservableDoubleGauge; +import io.opentelemetry.api.metrics.ObservableLongGauge; import java.util.EnumMap; import java.util.Map; import java.util.function.ObjDoubleConsumer; @@ -37,7 +39,8 @@ * is {@code O(|E1| × |E2|)} {@code liveStateResolver} calls plus one * {@code measurement.record(...)} per emitted pair. */ -public class AsyncMetricEntityStateTwoEnums & VeniceDimensionInterface, E2 extends Enum & VeniceDimensionInterface> { +public class AsyncMetricEntityStateTwoEnums & VeniceDimensionInterface, E2 extends Enum & VeniceDimensionInterface> + implements AutoCloseable { private final boolean emitOpenTelemetryMetrics; /** Precomputed per-pair attributes; {@code null} when OTel is disabled. */ private final EnumMap> attributesByEnum; @@ -187,4 +190,13 @@ public EnumMap> getAttributesByEnum() { public Object getInstrument() { return instrument; } + + @Override + public void close() { + if (instrument instanceof ObservableLongGauge) { + ((ObservableLongGauge) instrument).close(); + } else if (instrument instanceof ObservableDoubleGauge) { + ((ObservableDoubleGauge) instrument).close(); + } + } } diff --git a/internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateOneEnumTest.java b/internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateOneEnumTest.java index aafbafb4be3..59141b88cbb 100644 --- a/internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateOneEnumTest.java +++ b/internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateOneEnumTest.java @@ -20,6 +20,7 @@ import com.linkedin.venice.stats.metrics.AsyncMetricResolvers.ValueResolverOneEnum; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.ObservableDoubleMeasurement; +import io.opentelemetry.api.metrics.ObservableLongGauge; import io.opentelemetry.api.metrics.ObservableLongMeasurement; import java.util.EnumMap; import java.util.EnumSet; @@ -84,6 +85,23 @@ public void testCreateRegistersExactlyOneObservableGauge() { } } + @Test + public void testCloseUnregistersObservableGauge() { + ObservableLongGauge gauge = mock(ObservableLongGauge.class); + when(mockOtelRepository.registerObservableLongGauge(eq(mockMetricEntity), any())).thenReturn(gauge); + AsyncMetricEntityStateOneEnum metricState = AsyncMetricEntityStateOneEnum.create( + mockMetricEntity, + mockOtelRepository, + baseDimensionsMap, + DimensionEnum1.class, + e -> e, + (state, e) -> 1L); + + metricState.close(); + + verify(gauge).close(); + } + @Test public void testCallbackEmitsOnlyWhenLiveStateResolverReturnsNonNull() { // liveStateResolver returns state for DIMENSION_ONE only; DIMENSION_TWO is dormant. diff --git a/internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateTwoEnumsTest.java b/internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateTwoEnumsTest.java index b8c70b57286..0343d9bbfb5 100644 --- a/internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateTwoEnumsTest.java +++ b/internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateTwoEnumsTest.java @@ -21,6 +21,7 @@ import com.linkedin.venice.stats.metrics.AsyncMetricResolvers.ValueResolverTwoEnums; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.ObservableDoubleMeasurement; +import io.opentelemetry.api.metrics.ObservableLongGauge; import io.opentelemetry.api.metrics.ObservableLongMeasurement; import java.util.HashMap; import java.util.HashSet; @@ -94,6 +95,24 @@ public void testCreateRegistersExactlyOneObservableGauge() { assertEquals(leafCount, DimensionEnum1.values().length * DimensionEnum2.values().length); } + @Test + public void testCloseUnregistersObservableGauge() { + ObservableLongGauge gauge = mock(ObservableLongGauge.class); + when(mockOtelRepository.registerObservableLongGauge(eq(mockMetricEntity), any())).thenReturn(gauge); + AsyncMetricEntityStateTwoEnums metricState = AsyncMetricEntityStateTwoEnums.create( + mockMetricEntity, + mockOtelRepository, + baseDimensionsMap, + DimensionEnum1.class, + DimensionEnum2.class, + (e1, e2) -> e1, + (state, e1, e2) -> 1L); + + metricState.close(); + + verify(gauge).close(); + } + @Test public void testCallbackEmitsOnlyWhenLiveStateResolverReturnsNonNull() { LiveStateResolverTwoEnums liveStateResolver = (e1, e2) -> { diff --git a/internal/venice-common/src/main/java/com/linkedin/venice/meta/AbstractStore.java b/internal/venice-common/src/main/java/com/linkedin/venice/meta/AbstractStore.java index 44c494e28a9..691d60235c9 100644 --- a/internal/venice-common/src/main/java/com/linkedin/venice/meta/AbstractStore.java +++ b/internal/venice-common/src/main/java/com/linkedin/venice/meta/AbstractStore.java @@ -343,6 +343,7 @@ public static List computeVersionsToDelete( * b) ERROR version (ideally should not be there as AbstractPushmonitor#handleErrorPush deletes those) * c) STARTED versions if its not the last one and the store is not migrating. * d) KILLED versions by {@link org.apache.kafka.clients.admin.Admin#killOfflinePush} api. + * e) ROLLED_BACK versions after their dedicated time-based retention expires. */ // current version need not be the largest version, preseve it before finding other versions > current version for (int i = lastElementIndex; i >= 0; i--) { @@ -353,26 +354,27 @@ public static List computeVersionsToDelete( for (int i = lastElementIndex; i >= 0; i--) { Version version = versions.get(i); + VersionStatus status = version.getStatus(); if (version.getNumber() == currentVersion) { // currentVersion is always preserved continue; } - if (VersionStatus.isVersionRolledBack(version.getStatus())) { + if (VersionStatus.isVersionRolledBack(status)) { // ROLLED_BACK versions are retained and cleaned up by StoreBackupVersionCleanupService // with a dedicated retention period. Skip them here so the retention window is honored. // Note: if the backup version retention-based cleanup service is disabled for the cluster, // ROLLED_BACK versions will not be automatically deleted by this path. The admin tool's // deleteOldVersion can still be used for manual cleanup in that case. continue; - } else if (VersionStatus.canDelete(version.getStatus())) { // ERROR and KILLED versions are always deleted + } else if (VersionStatus.canDelete(status)) { // ERROR and KILLED versions are always deleted versionsToDelete.add(version); - } else if (VersionStatus.ONLINE.equals(version.getStatus())) { + } else if (VersionStatus.preserveLastFew(status)) { if (curNumVersionsToPreserve > 0) { // keep the minimum number of version to preserve curNumVersionsToPreserve--; } else { versionsToDelete.add(version); } - } else if (VersionStatus.STARTED.equals(version.getStatus()) && (i != lastElementIndex) && !isMigrating) { + } else if (VersionStatus.STARTED.equals(status) && (i != lastElementIndex) && !isMigrating) { // For the non-last started version, if it's not the current version(STARTED version should not be the current // version, just prevent some edge cases here.), we should delete it only if the store is not migrating // as during store migration are there are concurrent pushes with STARTED version. @@ -380,8 +382,8 @@ public static List computeVersionsToDelete( // version status properly. versionsToDelete.add(version); } - // TODO here we don't deal with the PUSHED version, just keep all of them, need to consider collect them too in - // the future. + // PUSHED versions can still be active deferred or concurrent swap candidates. They require an + // explicit terminal transition before cleanup and are never deleted by count alone. } return versionsToDelete; } diff --git a/internal/venice-common/src/main/java/com/linkedin/venice/serialization/avro/AvroProtocolDefinition.java b/internal/venice-common/src/main/java/com/linkedin/venice/serialization/avro/AvroProtocolDefinition.java index 0ca65d946c2..e87516a046c 100644 --- a/internal/venice-common/src/main/java/com/linkedin/venice/serialization/avro/AvroProtocolDefinition.java +++ b/internal/venice-common/src/main/java/com/linkedin/venice/serialization/avro/AvroProtocolDefinition.java @@ -71,7 +71,7 @@ public enum AvroProtocolDefinition { * * TODO: Move AdminOperation to venice-common module so that we can properly reference it here. */ - ADMIN_OPERATION(101, SpecificData.get().getSchema(ByteBuffer.class), "AdminOperation"), + ADMIN_OPERATION(102, SpecificData.get().getSchema(ByteBuffer.class), "AdminOperation"), /** * Single chunk of a large multi-chunk value. Just a bunch of bytes. diff --git a/mkdocs.yml b/mkdocs.yml index a10502b2951..a2758c5f3cc 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -121,6 +121,7 @@ nav: - operations/index.md - Data Management: - Repush: operations/data-management/repush.md + - Version Lifecycle and Deletion: operations/data-management/version-lifecycle.md - TTL: operations/data-management/ttl.md - System Stores: operations/data-management/system-stores.md - Alerting: diff --git a/services/venice-controller/src/main/java/com/linkedin/venice/controller/DeferredVersionSwapService.java b/services/venice-controller/src/main/java/com/linkedin/venice/controller/DeferredVersionSwapService.java index 5f87b36c3f7..8be2d3035ee 100644 --- a/services/venice-controller/src/main/java/com/linkedin/venice/controller/DeferredVersionSwapService.java +++ b/services/venice-controller/src/main/java/com/linkedin/venice/controller/DeferredVersionSwapService.java @@ -85,8 +85,12 @@ public class DeferredVersionSwapService extends AbstractVeniceService { private static final Set VERSION_SWAP_COMPLETION_STATUSES = Utils.setOf(ONLINE, PARTIALLY_ONLINE, ERROR); private static final Set TERMINAL_PUSH_VERSION_STATUSES = Utils.setOf(ONLINE); + private static final Set ABANDONED_VERSION_STATUSES = + Utils.setOf(ERROR, PARTIALLY_ONLINE, VersionStatus.ROLLED_BACK); private Cache storeWaitTimeCacheForSequentialRollout = Caffeine.newBuilder().expireAfterWrite(1, TimeUnit.HOURS).build(); + private final Cache terminalVersionReconciliationCache = + Caffeine.newBuilder().maximumSize(100_000).expireAfterWrite(1, TimeUnit.DAYS).build(); private static final int CONTROLLER_CLIENT_REQUEST_TIMEOUT = 1 * Time.MS_PER_SECOND; private static final int LOG_LATENCY_THRESHOLD = 5 * Time.MS_PER_SECOND; private final Map clusterToExecutorMap = new ConcurrentHashMap<>(); @@ -421,6 +425,20 @@ private boolean isPushInTerminalState( logMessageIfNotRedundant(message); return true; } + break; + case ERROR: + case PARTIALLY_ONLINE: + case ROLLED_BACK: + String reconciliationKey = getVersionProcessingKey(clusterName, storeName, targetVersionNum); + if (terminalVersionReconciliationCache.getIfPresent(reconciliationKey) == null + && reconcileAbandonedVersionInChildRegions( + clusterName, + storeName, + targetVersionNum, + targetVersion.getStatus())) { + terminalVersionReconciliationCache.put(reconciliationKey, true); + } + return false; } logMessageIfNotRedundant( @@ -899,7 +917,7 @@ private void handleFailedRollForward( return v + 1; }); - if (attemptedRetries == MAX_ROLL_FORWARD_RETRY_LIMIT) { + if (attemptedRetries >= MAX_ROLL_FORWARD_RETRY_LIMIT) { deferredVersionSwapStats.recordDeferredVersionSwapFailedRollForwardMetric(clusterName, parentStore.getName()); updateStore(clusterName, parentStore.getName(), PARTIALLY_ONLINE, targetVersionNum); failedRollforwardRetryCountMap.remove(kafkaTopicName); @@ -948,6 +966,55 @@ private void finishProcessingStore(String kafkaTopicName) { storesBeingProcessed.remove(kafkaTopicName); } + private static String getVersionProcessingKey(String clusterName, String storeName, int versionNumber) { + return clusterName + ":" + Version.composeKafkaTopic(storeName, versionNumber); + } + + private static boolean isAbandonedDeferredVersion(Version version) { + return version != null && version.isVersionSwapDeferred() && ABANDONED_VERSION_STATUSES.contains(version.getStatus()); + } + + private void submitTerminalVersionReconciliationTasks( + String clusterName, + Store parentStore, + ThreadPoolExecutor clusterExecutorService) { + for (Version version: parentStore.getVersions()) { + if (!isAbandonedDeferredVersion(version)) { + continue; + } + + int versionNumber = version.getNumber(); + VersionStatus parentStatus = version.getStatus(); + String processingKey = getVersionProcessingKey(clusterName, parentStore.getName(), versionNumber); + if (terminalVersionReconciliationCache.getIfPresent(processingKey) != null + || !tryStartProcessingStore(processingKey)) { + continue; + } + + clusterExecutorService.submit(() -> { + try { + if (reconcileAbandonedVersionInChildRegions( + clusterName, + parentStore.getName(), + versionNumber, + parentStatus)) { + terminalVersionReconciliationCache.put(processingKey, true); + } + } catch (Exception e) { + LOGGER.warn( + "Caught exception while reconciling terminal version: {} for store: {} in cluster: {}", + versionNumber, + parentStore.getName(), + clusterName, + e); + deferredVersionSwapStats.recordDeferredVersionSwapExceptionMetric(clusterName); + } finally { + finishProcessingStore(processingKey); + } + }); + } + } + private Runnable getRunnableForDeferredVersionSwap() { return () -> { LogContext.setLogContext(veniceControllerMultiClusterConfig.getLogContext()); @@ -982,6 +1049,11 @@ private Runnable getRunnableForDeferredVersionSwap() { // Filter out stores that aren't doing a target region push w/ deferred swap List eligibleStoresToProcess = new ArrayList<>(); for (Store parentStore: parentStores) { + submitTerminalVersionReconciliationTasks(cluster, parentStore, clusterExecutorService); + Version latestVersion = parentStore.getVersion(parentStore.getLargestUsedVersionNumber()); + if (isAbandonedDeferredVersion(latestVersion)) { + continue; + } if (!isTargetRegionPushWithDeferredSwapEnabled(parentStore)) { continue; } @@ -994,9 +1066,9 @@ private Runnable getRunnableForDeferredVersionSwap() { Version targetVersion = parentStore.getVersion(parentStore.getLargestUsedVersionNumber()); // Check if store is already being processed - String kafkaTopicName = - Version.composeKafkaTopic(parentStore.getName(), parentStore.getLargestUsedVersionNumber()); - if (!tryStartProcessingStore(kafkaTopicName)) { + String processingKey = + getVersionProcessingKey(cluster, parentStore.getName(), parentStore.getLargestUsedVersionNumber()); + if (!tryStartProcessingStore(processingKey)) { String message = "Skipping store " + parentStore.getName() + " as it's already being processed"; logMessageIfNotRedundant(message); continue; @@ -1015,8 +1087,16 @@ private Runnable getRunnableForDeferredVersionSwap() { } else { performParallelRollForward(cluster, parentStore, childControllerClientMap, targetVersion); } + } catch (Exception e) { + LOGGER.warn( + "Caught exception while processing deferred version swap for store: {} in cluster: {}", + parentStore.getName(), + cluster, + e); + deferredVersionSwapStats.recordDeferredVersionSwapExceptionMetric(cluster); } finally { - finishProcessingStore(Version.composeKafkaTopic(parentStore.getName(), targetVersion.getNumber())); + finishProcessingStore( + getVersionProcessingKey(cluster, parentStore.getName(), targetVersion.getNumber())); } }); } @@ -1251,36 +1331,105 @@ private void performParallelRollForward( public void updateStore(String clusterName, String storeName, VersionStatus status, int targetVersionNum) { HelixVeniceClusterResources resources = veniceParentHelixAdmin.getVeniceHelixAdmin().getHelixVeniceClusterResources(clusterName); - try (AutoCloseableLock ignore = resources.getClusterLockManager().createStoreWriteLock(storeName)) { - ReadWriteStoreRepository repository = resources.getStoreMetadataRepository(); - Store store = repository.getStore(storeName); - LOGGER.info( - "Updating store: {} version: {} from status {} to status {}", - storeName, - targetVersionNum, - store.getVersionStatus(targetVersionNum), - status); - store.updateVersionStatus(targetVersionNum, status); - if (status == ONLINE || status == PARTIALLY_ONLINE) { - store.setCurrentVersion(targetVersionNum); - - // For jobs that stop polling early or for pushes that don't poll (empty push), we need to truncate the parent - // VT here to unblock the next push - String kafkaTopicName = Version.composeKafkaTopic(storeName, targetVersionNum); - ConcurrentPushDetectionStrategy strategy = - veniceControllerMultiClusterConfig.getControllerConfig(clusterName).getConcurrentPushDetectionStrategy(); - // skip truncating if the topic was not created based on ConcurrentPushDetectionStrategy - if (strategy.isTopicWriteNeeded() && !veniceParentHelixAdmin.isTopicTruncated(kafkaTopicName)) { - LOGGER.info("Truncating parent VT for {}", kafkaTopicName); - veniceParentHelixAdmin.truncateKafkaTopic(Version.composeKafkaTopic(storeName, targetVersionNum)); + try { + try (AutoCloseableLock ignore = resources.getClusterLockManager().createStoreWriteLock(storeName)) { + ReadWriteStoreRepository repository = resources.getStoreMetadataRepository(); + Store store = repository.getStore(storeName); + LOGGER.info( + "Updating store: {} version: {} from status {} to status {}", + storeName, + targetVersionNum, + store.getVersionStatus(targetVersionNum), + status); + store.updateVersionStatus(targetVersionNum, status); + if (status == ONLINE || status == PARTIALLY_ONLINE) { + store.setCurrentVersion(targetVersionNum); + + // For jobs that stop polling early or for pushes that don't poll (empty push), we need to truncate the parent + // VT here to unblock the next push + String kafkaTopicName = Version.composeKafkaTopic(storeName, targetVersionNum); + ConcurrentPushDetectionStrategy strategy = + veniceControllerMultiClusterConfig.getControllerConfig(clusterName).getConcurrentPushDetectionStrategy(); + // skip truncating if the topic was not created based on ConcurrentPushDetectionStrategy + if (strategy.isTopicWriteNeeded() && !veniceParentHelixAdmin.isTopicTruncated(kafkaTopicName)) { + LOGGER.info("Truncating parent VT for {}", kafkaTopicName); + veniceParentHelixAdmin.truncateKafkaTopic(Version.composeKafkaTopic(storeName, targetVersionNum)); + } + } + repository.updateStore(store); + } + + if (status == ERROR || status == PARTIALLY_ONLINE || status == VersionStatus.ROLLED_BACK) { + String reconciliationKey = getVersionProcessingKey(clusterName, storeName, targetVersionNum); + if (reconcileAbandonedVersionInChildRegions(clusterName, storeName, targetVersionNum, status)) { + terminalVersionReconciliationCache.put(reconciliationKey, true); } } - repository.updateStore(store); } catch (Exception e) { LOGGER.warn("Failed to execute updateStore for store: {} in cluster: {}", storeName, clusterName, e); } } + private boolean reconcileAbandonedVersionInChildRegions( + String clusterName, + String storeName, + int targetVersionNum, + VersionStatus parentStatus) { + Map controllerClientMap = + veniceParentHelixAdmin.getVeniceHelixAdmin().getControllerClientMap(clusterName); + if (controllerClientMap == null || controllerClientMap.isEmpty()) { + throw new VeniceException( + "Cannot reconcile abandoned version " + targetVersionNum + " for store " + storeName + " in cluster " + + clusterName + ": no child regions are configured"); + } + + Set regionsToReconcile = new HashSet<>(); + boolean allRegionsTerminal = true; + for (String region: controllerClientMap.keySet()) { + StoreResponse storeResponse = getStoreForRegion(clusterName, region, storeName); + if (storeResponse == null || storeResponse.getStore() == null) { + allRegionsTerminal = false; + continue; + } + + StoreInfo childStore = storeResponse.getStore(); + Version childVersion = + getVersionFromStoreInRegion(region, storeName, targetVersionNum, storeResponse); + if (childVersion == null) { + allRegionsTerminal = false; + continue; + } + if (VersionStatus.isVersionRolledBack(childVersion.getStatus()) + || VersionStatus.canDelete(childVersion.getStatus())) { + continue; + } + if (childStore.getCurrentVersion() == targetVersionNum) { + if (parentStatus != PARTIALLY_ONLINE) { + allRegionsTerminal = false; + } + continue; + } + + if (VersionStatus.isBootstrapCompleted(childVersion.getStatus())) { + regionsToReconcile.add(region); + } else { + allRegionsTerminal = false; + } + } + + if (!regionsToReconcile.isEmpty()) { + String regionsFilter = RegionUtils.composeRegionList(regionsToReconcile); + veniceParentHelixAdmin.markVersionRolledBack(clusterName, storeName, targetVersionNum, regionsFilter); + LOGGER.info( + "Reconciled version: {} for store: {} as ROLLED_BACK in child regions: {} after parent status: {}", + targetVersionNum, + storeName, + regionsFilter, + parentStatus); + } + return allRegionsTerminal; + } + private void markTargetRegionPromoted(String clusterName, String storeName, int targetVersionNum) { try { LOGGER.info("Marking targetRegionPromoted=true for store: {} version: {}", storeName, targetVersionNum); diff --git a/services/venice-controller/src/main/java/com/linkedin/venice/controller/StoreBackupVersionCleanupService.java b/services/venice-controller/src/main/java/com/linkedin/venice/controller/StoreBackupVersionCleanupService.java index 6a9941eda9b..d9bc875bb16 100644 --- a/services/venice-controller/src/main/java/com/linkedin/venice/controller/StoreBackupVersionCleanupService.java +++ b/services/venice-controller/src/main/java/com/linkedin/venice/controller/StoreBackupVersionCleanupService.java @@ -156,10 +156,10 @@ protected static boolean whetherStoreReadyToBeCleanup( long minCleanupDelayMs) { List versions = store.getVersions(); - // Regardless of retention, if there are more than 1 non-rolled-back versions strictly below the current version, - // we should clean up. ROLLED_BACK versions are excluded since they have their own retention-based cleanup path. + // Regardless of retention, clean up when more than one standard-cleanup-eligible version is below current. + // PUSHED versions remain protected until an explicit terminal decision, and ROLLED_BACK has its own retention path. if (versions.stream() - .filter(v -> v.getNumber() < currentVersion && !VersionStatus.isVersionRolledBack(v.getStatus())) + .filter(v -> v.getNumber() < currentVersion && isEligibleForStandardBackupCleanup(v)) .count() > 1) { return true; } @@ -333,7 +333,7 @@ protected boolean cleanupBackupVersion(Store store, String clusterName) { HashSet repushChainVersions = new HashSet<>(); // all versions repushed into the current version readyToBeRemovedVersions = versions.stream() - .filter(v -> !VersionStatus.isVersionRolledBack(v.getStatus())) // rolled-back handled separately + .filter(StoreBackupVersionCleanupService::isEligibleForStandardBackupCleanup) .sorted((v1, v2) -> Integer.compare(v2.getNumber(), v1.getNumber())) // sort in descending order .filter(v -> { // always delete past default retention and less than current version @@ -355,7 +355,7 @@ protected boolean cleanupBackupVersion(Store store, String clusterName) { if (isCurrentVersionRepushed && readyToBeRemovedVersions.isEmpty()) { for (Version v: versions) { if (v.getNumber() < currentVersion && v.getRepushSourceVersion() > NON_EXISTING_VERSION - && !VersionStatus.isVersionRolledBack(v.getStatus())) { + && isEligibleForStandardBackupCleanup(v)) { readyToBeRemovedVersions.add(v); } } @@ -409,15 +409,18 @@ protected boolean cleanupBackupVersion(Store store, String clusterName) { return true; } + private static boolean isEligibleForStandardBackupCleanup(Version version) { + VersionStatus status = version.getStatus(); + return status != VersionStatus.PUSHED && !VersionStatus.isVersionRolledBack(status); + } + /** * Deletes ROLLED_BACK versions whose retention period has expired. * - *

The retention clock is anchored to {@link Store#getLatestVersionPromoteToCurrentTimestamp()}, - * which is set when a version becomes current — including the rollback-target version being - * re-promoted. If a subsequent push completes before the retention expires, the timestamp resets - * and the ROLLED_BACK version survives longer than the configured retention. This is intentionally - * conservative: a per-version {@code rolledBackTimestamp} would give exact retention but requires - * a Version schema change. + *

The retention clock is anchored to {@link Store#getLatestVersionPromoteToCurrentTimestamp()}. + * Marking a child version ROLLED_BACK resets this store-level timestamp, and a later promotion may + * reset it again. This is intentionally conservative: exact independent retention for multiple + * rolled-back versions would require a per-version rollback timestamp. */ boolean cleanupRolledBackVersions(Store store, String clusterName, List versions) { long rolledBackVersionRetentionMs = getRolledBackVersionRetentionMs(clusterName); diff --git a/services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceHelixAdmin.java b/services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceHelixAdmin.java index ab7e4505dfb..160be02620b 100644 --- a/services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceHelixAdmin.java +++ b/services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceHelixAdmin.java @@ -5124,6 +5124,76 @@ public void rollbackToBackupVersion(String clusterName, String storeName, String } } + /** + * Mark {@code versionNumber} as {@link VersionStatus#ROLLED_BACK} in the regions selected by + * {@code regionFilter}, WITHOUT changing the store's current version. + * + *

Deferred swaps can end globally while a bootstrap-complete copy is still non-current in some + * regions. Marking that copy ROLLED_BACK makes it eligible for time-based cleanup. In-progress + * versions and the current serving version remain untouched. + */ + public void markVersionRolledBack(String clusterName, String storeName, int versionNumber, String regionFilter) { + String currentRegion = getRegionName(); + if (!StringUtils.isEmpty(regionFilter)) { + Set regionsFilter = parseRegionsFilterList(regionFilter); + if (!regionsFilter.contains(currentRegion)) { + LOGGER.info( + "markVersionRolledBack will be skipped for store: {} version: {} in cluster: {}, because the region filter" + + " is {} which doesn't include the current region: {}", + storeName, + versionNumber, + clusterName, + regionsFilter, + currentRegion); + return; + } + } + + storeMetadataUpdate(clusterName, storeName, (store, resources) -> { + if (!store.containsVersion(versionNumber)) { + LOGGER.info( + "markVersionRolledBack skipped: version {} not found in store {} in cluster {}", + versionNumber, + storeName, + clusterName); + return store; + } + // Never override the current version's status; that version serves reads. + if (store.getCurrentVersion() == versionNumber) { + LOGGER.warn( + "markVersionRolledBack skipped: version {} is the current version of store {} in cluster {}", + versionNumber, + storeName, + clusterName); + return store; + } + VersionStatus currentStatus = store.getVersionStatus(versionNumber); + if (VersionStatus.isVersionRolledBack(currentStatus) || VersionStatus.canDelete(currentStatus)) { + // Already ROLLED_BACK, or already in a terminal cleanable status (KILLED/ERROR) — nothing to do. + return store; + } + if (!VersionStatus.isBootstrapCompleted(currentStatus)) { + LOGGER.info( + "markVersionRolledBack skipped: version {} of store {} in cluster {} has not completed bootstrap (status {})", + versionNumber, + storeName, + clusterName, + currentStatus); + return store; + } + LOGGER.info( + "Marking version {} of store {} in cluster {} as ROLLED_BACK (was {})", + versionNumber, + storeName, + clusterName, + currentStatus); + store.updateVersionStatus(versionNumber, ROLLED_BACK); + store.setLatestVersionPromoteToCurrentTimestamp( + Math.max(store.getLatestVersionPromoteToCurrentTimestamp(), System.currentTimeMillis())); + return store; + }); + } + /** * Update the largest used version number of a specified store. */ diff --git a/services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceParentHelixAdmin.java b/services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceParentHelixAdmin.java index 83010075846..dbc7ac4f28c 100644 --- a/services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceParentHelixAdmin.java +++ b/services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceParentHelixAdmin.java @@ -63,6 +63,7 @@ import com.linkedin.venice.controller.kafka.protocol.admin.PushStatusSystemStoreAutoCreationValidation; import com.linkedin.venice.controller.kafka.protocol.admin.ResumeStore; import com.linkedin.venice.controller.kafka.protocol.admin.RollbackCurrentVersion; +import com.linkedin.venice.controller.kafka.protocol.admin.MarkVersionRolledBack; import com.linkedin.venice.controller.kafka.protocol.admin.SchemaMeta; import com.linkedin.venice.controller.kafka.protocol.admin.SetStoreOwner; import com.linkedin.venice.controller.kafka.protocol.admin.SetStorePartitionCount; @@ -2519,6 +2520,34 @@ void checkNewPushCapacityFromChildren(String clusterName, String storeName) { } } + /** + * Mark {@code versionNum} as ROLLED_BACK in the child regions selected by {@code regionFilter}, + * WITHOUT changing the current version. Used to reconcile the non-target regions after a deferred + * version swap rollback: those regions bootstrapped the target version but never swapped to it, so + * their copy is stranded in PUSHED status above their unchanged current version and leaks disk. + * Marking it ROLLED_BACK makes it visible to {@code StoreBackupVersionCleanupService}. + */ + public void markVersionRolledBack(String clusterName, String storeName, int versionNum, String regionFilter) { + acquireAdminMessageLock(clusterName, storeName); + try { + getVeniceHelixAdmin().checkPreConditionForUpdateStoreMetadata(clusterName, storeName); + + MarkVersionRolledBack markVersionRolledBack = + (MarkVersionRolledBack) AdminMessageType.MARK_VERSION_ROLLED_BACK.getNewInstance(); + markVersionRolledBack.clusterName = clusterName; + markVersionRolledBack.storeName = storeName; + markVersionRolledBack.versionNum = versionNum; + markVersionRolledBack.regionsFilter = regionFilter; + AdminOperation message = new AdminOperation(); + message.operationType = AdminMessageType.MARK_VERSION_ROLLED_BACK.getValue(); + message.payloadUnion = markVersionRolledBack; + + sendAdminMessageAndWaitForConsumed(clusterName, storeName, message); + } finally { + releaseAdminMessageLock(clusterName, storeName); + } + } + private void updateParentVersionStatusAfterRollback( String clusterName, String storeName, diff --git a/services/venice-controller/src/main/java/com/linkedin/venice/controller/kafka/consumer/AdminExecutionTask.java b/services/venice-controller/src/main/java/com/linkedin/venice/controller/kafka/consumer/AdminExecutionTask.java index add09bc2407..2c9e281898f 100644 --- a/services/venice-controller/src/main/java/com/linkedin/venice/controller/kafka/consumer/AdminExecutionTask.java +++ b/services/venice-controller/src/main/java/com/linkedin/venice/controller/kafka/consumer/AdminExecutionTask.java @@ -30,6 +30,7 @@ import com.linkedin.venice.controller.kafka.protocol.admin.ResumeStore; import com.linkedin.venice.controller.kafka.protocol.admin.RollForwardCurrentVersion; import com.linkedin.venice.controller.kafka.protocol.admin.RollbackCurrentVersion; +import com.linkedin.venice.controller.kafka.protocol.admin.MarkVersionRolledBack; import com.linkedin.venice.controller.kafka.protocol.admin.SetStoreCurrentVersion; import com.linkedin.venice.controller.kafka.protocol.admin.SetStoreOwner; import com.linkedin.venice.controller.kafka.protocol.admin.SetStorePartitionCount; @@ -344,6 +345,9 @@ private void processMessage(AdminOperation adminOperation) { case ROLLFORWARD_CURRENT_VERSION: handleRollForwardToFutureVersion((RollForwardCurrentVersion) adminOperation.payloadUnion); break; + case MARK_VERSION_ROLLED_BACK: + handleMarkVersionRolledBack((MarkVersionRolledBack) adminOperation.payloadUnion); + break; default: throw new VeniceException("Unknown admin operation type: " + adminOperation.operationType); } @@ -935,6 +939,15 @@ private void handleRollbackCurrentVersion(RollbackCurrentVersion message) { admin.rollbackToBackupVersion(clusterName, storeName, regionFilter); } + private void handleMarkVersionRolledBack(MarkVersionRolledBack message) { + String clusterName = message.getClusterName().toString(); + String storeName = message.getStoreName().toString(); + int versionNum = message.getVersionNum(); + CharSequence regionsFilter = message.getRegionsFilter(); + String regionFilter = regionsFilter == null ? null : regionsFilter.toString(); + admin.markVersionRolledBack(clusterName, storeName, versionNum, regionFilter); + } + private void handleDeleteUnusedValueSchema(DeleteUnusedValueSchemas message) { String clusterName = message.getClusterName().toString(); String storeName = message.getStoreName().toString(); diff --git a/services/venice-controller/src/main/java/com/linkedin/venice/controller/kafka/protocol/enums/AdminMessageType.java b/services/venice-controller/src/main/java/com/linkedin/venice/controller/kafka/protocol/enums/AdminMessageType.java index 9ec42b29c3b..00d51c5554c 100644 --- a/services/venice-controller/src/main/java/com/linkedin/venice/controller/kafka/protocol/enums/AdminMessageType.java +++ b/services/venice-controller/src/main/java/com/linkedin/venice/controller/kafka/protocol/enums/AdminMessageType.java @@ -15,6 +15,7 @@ import com.linkedin.venice.controller.kafka.protocol.admin.DisableStoreRead; import com.linkedin.venice.controller.kafka.protocol.admin.EnableStoreRead; import com.linkedin.venice.controller.kafka.protocol.admin.KillOfflinePushJob; +import com.linkedin.venice.controller.kafka.protocol.admin.MarkVersionRolledBack; import com.linkedin.venice.controller.kafka.protocol.admin.MetaSystemStoreAutoCreationValidation; import com.linkedin.venice.controller.kafka.protocol.admin.MetadataSchemaCreation; import com.linkedin.venice.controller.kafka.protocol.admin.MigrateStore; @@ -65,7 +66,8 @@ public enum AdminMessageType implements VeniceDimensionInterface { CONFIGURE_INCREMENTAL_PUSH_FOR_CLUSTER(22, true), META_SYSTEM_STORE_AUTO_CREATION_VALIDATION(23, false), PUSH_STATUS_SYSTEM_STORE_AUTO_CREATION_VALIDATION(24, false), CREATE_STORAGE_PERSONA(25, false), DELETE_STORAGE_PERSONA(26, false), UPDATE_STORAGE_PERSONA(27, false), DELETE_UNUSED_VALUE_SCHEMA(28, false), - ROLLBACK_CURRENT_VERSION(29, false), ROLLFORWARD_CURRENT_VERSION(30, false); + ROLLBACK_CURRENT_VERSION(29, false), ROLLFORWARD_CURRENT_VERSION(30, false), + MARK_VERSION_ROLLED_BACK(31, false); private final int value; private final boolean batchUpdate; @@ -138,6 +140,8 @@ public Object getNewInstance() { return new RollForwardCurrentVersion(); case ROLLBACK_CURRENT_VERSION: return new RollbackCurrentVersion(); + case MARK_VERSION_ROLLED_BACK: + return new MarkVersionRolledBack(); default: throw new VeniceException("Unsupported " + getClass().getSimpleName() + " value: " + value); } diff --git a/services/venice-controller/src/main/resources/avro/AdminOperation/v102/AdminOperation.avsc b/services/venice-controller/src/main/resources/avro/AdminOperation/v102/AdminOperation.avsc new file mode 100644 index 00000000000..051df79d210 --- /dev/null +++ b/services/venice-controller/src/main/resources/avro/AdminOperation/v102/AdminOperation.avsc @@ -0,0 +1,1370 @@ +{ + "name": "AdminOperation", + "namespace": "com.linkedin.venice.controller.kafka.protocol.admin", + "type": "record", + "fields": [ + { + "name": "operationType", + "doc": "0 => StoreCreation, 1 => ValueSchemaCreation, 2 => PauseStore, 3 => ResumeStore, 4 => KillOfflinePushJob, 5 => DisableStoreRead, 6 => EnableStoreRead, 7=> DeleteAllVersions, 8=> SetStoreOwner, 9=> SetStorePartitionCount, 10=> SetStoreCurrentVersion, 11=> UpdateStore, 12=> DeleteStore, 13=> DeleteOldVersion, 14=> MigrateStore, 15=> AbortMigration, 16=>AddVersion, 17=> DerivedSchemaCreation, 18=>SupersetSchemaCreation, 19=>EnableNativeReplicationForCluster, 20=>MetadataSchemaCreation, 21=>EnableActiveActiveReplicationForCluster, 25=>CreatePersona, 26=>DeletePersona, 27=>UpdatePersona, 28=>RollbackCurrentVersion, 29=>RollforwardCurrentVersion, 31=>MarkVersionRolledBack", + "type": "int" + }, { + "name": "executionId", + "doc": "ID of a command execution which is used to query the status of this command.", + "type": "long", + "default": 0 + }, { + "name": "payloadUnion", + "doc": "This contains the main payload of the admin operation", + "type": [ + { + "name": "StoreCreation", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + }, + { + "name": "owner", + "type": "string" + }, + { + "name": "keySchema", + "type": { + "type": "record", + "name": "SchemaMeta", + "fields": [ + {"name": "schemaType", "type": "int", "doc": "0 => Avro-1.4, and we can add more if necessary"}, + {"name": "definition", "type": "string"} + ] + } + }, + { + "name": "valueSchema", + "type": "SchemaMeta" + } + ] + }, + { + "name": "ValueSchemaCreation", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + }, + { + "name": "schema", + "type": "SchemaMeta" + }, + { + "name": "schemaId", + "type": "int" + }, + { + "name": "doUpdateSupersetSchemaID", + "type": "boolean", + "doc": "Whether this superset schema ID should be updated to be the value schema ID for this store.", + "default": false + } + ] + }, + { + "name": "PauseStore", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + } + ] + }, + { + "name": "ResumeStore", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + } + ] + }, + { + "name": "KillOfflinePushJob", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "kafkaTopic", + "type": "string" + } + ] + }, + { + "name": "DisableStoreRead", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + } + ] + }, + { + "name": "EnableStoreRead", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + } + ] + }, + { + "name": "DeleteAllVersions", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + } + ] + }, + { + "name": "SetStoreOwner", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + }, + { + "name": "owner", + "type": "string" + } + ] + }, + { + "name": "SetStorePartitionCount", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + }, + { + "name": "partitionNum", + "type": "int" + } + ] + }, + { + "name": "SetStoreCurrentVersion", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + }, + { + "name": "currentVersion", + "type": "int" + } + ] + }, + { + "name": "UpdateStore", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + }, + { + "name": "owner", + "type": "string" + }, + { + "name": "partitionNum", + "type": "int" + }, + { + "name": "currentVersion", + "type": "int" + }, + { + "name": "enableReads", + "type": "boolean" + }, + { + "name": "enableWrites", + "type": "boolean" + }, + { + "name": "storageQuotaInByte", + "type": "long", + "default": 21474836480 + }, + { + "name": "readQuotaInCU", + "type": "long", + "default": 1800 + }, + { + "name": "hybridStoreConfig", + "type": [ + "null", + { + "name": "HybridStoreConfigRecord", + "type": "record", + "fields": [ + { + "name": "rewindTimeInSeconds", + "type": "long" + }, + { + "name": "offsetLagThresholdToGoOnline", + "type": "long" + }, + { + "name": "producerTimestampLagThresholdToGoOnlineInSeconds", + "type": "long", + "default": -1 + }, + { + "name": "dataReplicationPolicy", + "doc": "Real-time Samza job data replication policy. Using int because Avro Enums are not evolvable 0 => NON_AGGREGATE, 1 => AGGREGATE, 2 => NONE, 3 => ACTIVE_ACTIVE", + "type": "int", + "default": 0 + }, + { + "name": "bufferReplayPolicy", + "type": "int", + "doc": "Policy that will be used during buffer replay. rewindTimeInSeconds defines the delta. 0 => REWIND_FROM_EOP (replay from 'EOP - rewindTimeInSeconds'), 1 => REWIND_FROM_SOP (replay from 'SOP - rewindTimeInSeconds')", + "default": 0 + }, + {"name": "realTimeTopicName", "type": "string", "default": "", "doc": "Name of the real time topic this store/version uses"} + ] + } + ], + "default": null + }, + { + "name": "accessControlled", + "type": "boolean", + "default": false + }, + { + "name": "compressionStrategy", + "doc": "Using int because Avro Enums are not evolvable", + "type": "int", + "default": 0 + }, + { + "name": "chunkingEnabled", + "type": "boolean", + "default": false + }, + { + "name": "rmdChunkingEnabled", + "type": "boolean", + "default": false + }, + { + "name": "singleGetRouterCacheEnabled", + "aliases": ["routerCacheEnabled"], + "type": "boolean", + "default": false + }, + { + "name": "batchGetRouterCacheEnabled", + "type": "boolean", + "default": false + }, + { + "name": "batchGetLimit", + "doc": "The max key number allowed in batch get request, and Venice will use cluster-level config if the limit (not positive) is not valid", + "type": "int", + "default": -1 + }, + { + "name": "numVersionsToPreserve", + "doc": "The max number of versions the store should preserve. Venice will use cluster-level config if the number is 0 here.", + "type": "int", + "default": 0 + }, + { + "name": "incrementalPushEnabled", + "doc": "a flag to see if the store supports incremental push or not", + "type": "boolean", + "default": false + }, + { + "name": "separateRealTimeTopicEnabled", + "doc": "Flag to see if the store supports separate real-time topic for incremental push.", + "type": "boolean", + "default": false + }, + { + "name": "isMigrating", + "doc": "Whether or not the store is in the process of migration", + "type": "boolean", + "default": false + }, + { + "name": "writeComputationEnabled", + "doc": "Whether write-path computation feature is enabled for this store", + "type": "boolean", + "default": false + }, + { + "name": "replicationMetadataVersionID", + "doc": "RMD (Replication metadata) version ID on the store-level. Default -1 means NOT_SET and the cluster-level RMD version ID should be used for stores.", + "type": "int", + "default": -1 + }, + { + "name": "readComputationEnabled", + "doc": "Whether read-path computation feature is enabled for this store", + "type": "boolean", + "default": false + }, + { + "name": "bootstrapToOnlineTimeoutInHours", + "doc": "Maximum number of hours allowed for the store to transition from bootstrap to online state", + "type": "int", + "default": 24 + }, + { + "name": "leaderFollowerModelEnabled", + "doc": "Whether or not to use leader follower state transition model for upcoming version", + "type": "boolean", + "default": false + }, + { + "name": "backupStrategy", + "doc": "Strategies to store backup versions.", + "type": "int", + "default": 0 + }, + { + "name": "clientDecompressionEnabled", + "type": "boolean", + "default": true + }, + { + "name": "schemaAutoRegisterFromPushJobEnabled", + "type": "boolean", + "default": false + }, + { + "name": "hybridStoreOverheadBypass", + "type": "boolean", + "default": false + }, + { + "name": "hybridStoreDiskQuotaEnabled", + "doc": "Whether or not to enable disk storage quota for a hybrid store", + "type": "boolean", + "default": false + }, + { + "name": "ETLStoreConfig", + "type": [ + "null", + { + "name": "ETLStoreConfigRecord", + "type": "record", + "fields": [ + { + "name": "etledUserProxyAccount", + "type": ["null", "string"] + }, + { + "name": "regularVersionETLEnabled", + "type": "boolean" + }, + { + "name": "futureVersionETLEnabled", + "type": "boolean" + }, + { + "name": "etlStrategy", + "type": "int", + "default": 1 + }, + { + "name": "etlActiveFabrics", + "type": ["null", {"type": "array", "items": "string"}], + "default": null, + "doc": "Allowlist of fabric names where ETL onboard/offboard fires. null = fire in every fabric (default behavior). When set, only listed fabrics' child controllers trigger the ExternalETLService." + } + ] + } + ], + "default": null + }, + { + "name": "partitionerConfig", + "type": [ + "null", + { + "name": "PartitionerConfigRecord", + "type": "record", + "fields": [ + { + "name": "partitionerClass", + "type": "string" + }, + { + "name": "partitionerParams", + "type": { + "type": "map", + "values": "string" + } + }, + { + "name": "amplificationFactor", + "type": "int" + } + ] + } + ], + "default": null + }, + { + "name": "nativeReplicationEnabled", + "type": "boolean", + "default": false + }, + { + "name": "pushStreamSourceAddress", + "type": ["null", "string"], + "default": null + }, + { + "name": "largestUsedVersionNumber", + "type": ["null", "int"], + "default": null + }, + { + "name": "largestUsedRTVersionNumber", + "type": ["null", "int"], + "doc": "Largest used RT version number used by this store. This is used to create real time topic name while creating a new store-version", + "default": null + }, + { + "name": "incrementalPushPolicy", + "doc": "Incremental Push Policy to reconcile with real time pushes. Using int because Avro Enums are not evolvable 0 => PUSH_TO_VERSION_TOPIC, 1 => INCREMENTAL_PUSH_SAME_AS_REAL_TIME", + "type": "int", + "default": 0 + }, + { + "name": "backupVersionRetentionMs", + "type": "long", + "doc": "Backup version retention time after a new version is promoted to the current version, if not specified, Venice will use the configured retention as the default policy", + "default": -1 + }, + { + "name": "replicationFactor", + "doc": "number of replica each store version will have", + "type": "int", + "default": 3 + }, + { + "name": "migrationDuplicateStore", + "doc": "Whether or not the store is a duplicate store in the process of migration", + "type": "boolean", + "default": false + }, + { + "name": "nativeReplicationSourceFabric", + "doc": "The source fabric to be used when the store is running in Native Replication mode.", + "type": ["null", "string"], + "default": null + }, + { + "name": "activeActiveReplicationEnabled", + "doc": "A command option to enable/disable Active/Active replication feature for a store", + "type": "boolean", + "default": false + }, + { + "name": "disableMetaStore", + "doc": "An UpdateStore command option to disable the companion meta system store", + "type": "boolean", + "default": false + }, + { + "name": "disableDavinciPushStatusStore", + "doc": "An UpdateStore command option to disable the companion davinci push status store", + "type": "boolean", + "default": false + }, + { + "name": "applyTargetVersionFilterForIncPush", + "doc": "An UpdateStore command option to enable/disable applying the target version filter for incremental pushes", + "type": "boolean", + "default": false + }, + { + "name": "updatedConfigsList", + "doc": "The list that contains all updated configs by the UpdateStore command. Most of the fields in UpdateStore are not optional, and changing those fields to Optional (Union) is not a backward compatible change, so we have to add an addition array field to record all updated configs in parent controller.", + "type": { + "type": "array", + "items": "string" + }, + "default": [] + }, + { + "name": "replicateAllConfigs", + "doc": "A flag to indicate whether all store configs in parent cluster will be replicated to child clusters; true by default, so that existing UpdateStore messages in Admin topic will behave the same as before.", + "type": "boolean", + "default": true + }, + { + "name": "regionsFilter", + "doc": "A list of regions that will be impacted by the UpdateStore command", + "type": ["null", "string"], + "default": null + }, + { + "name": "storagePersona", + "doc": "The name of the StoragePersona to add to the store", + "type": ["null", "string"], + "default": null + }, + { + "name": "views", + "doc": "A map of views which describe and configure a downstream view of a venice store. Keys in this map are for convenience of managing configs.", + "type": ["null", + { + "type":"map", + "java-key-class": "java.lang.String", + "avro.java.string": "String", + "values": { + "name": "StoreViewConfigRecord", + "type": "record", + "doc": "A configuration for a particular view. This config should inform Venice leaders how to transform and transmit data to destination views.", + "fields": [ + { + "name": "viewClassName", + "type": "string", + "doc": "This informs what kind of view we are materializing. This then informs what kind of parameters are passed to parse this input. This is expected to be a fully formed class path name for materialization.", + "default": "" + }, + { + "name": "viewParameters", + "doc": "Optional parameters to be passed to the given view config.", + "type": ["null", + { + "type": "map", + "java-key-class": "java.lang.String", + "avro.java.string": "String", + "values": { "type": "string", "avro.java.string": "String" } + } + ], + "default": null + } + ] + } + }], + "default": null + }, + { + "name": "latestSuperSetValueSchemaId", + "doc": "The schema id for the latest superset schema", + "type" : "int", + "default": -1 + }, + { + "name": "storageNodeReadQuotaEnabled", + "doc": "Whether storage node read quota is enabled for this store", + "type": "boolean", + "default": false + }, + { + "name": "compactionEnabled", + "doc": "Whether compaction is enabled for this store", + "type": "boolean", + "default": true + }, + { + "name": "compactionThresholdMilliseconds", + "doc": "Store-level compaction threshold in milliseconds", + "type": "long", + "default": -1 + }, + { + "name": "encryptionEnabled", + "doc": "Whether encryption is enabled for this store", + "type": "boolean", + "default": false + }, + { + "name": "minCompactionLagSeconds", + "doc": "Store-level version topic min compaction lag", + "type": "long", + "default": -1 + }, + { + "name": "maxCompactionLagSeconds", + "doc": "Store-level version topic max compaction lag", + "type": "long", + "default": -1 + }, + { + "name": "maxRecordSizeBytes", + "doc": "Store-level maximum size of any record in bytes for batch push jobs", + "type": "int", + "default": -1 + }, + { + "name": "maxNearlineRecordSizeBytes", + "doc": "Store-level maximum size of any record in bytes for nearline jobs with partial updates", + "type": "int", + "default": -1 + }, + { + "name": "unusedSchemaDeletionEnabled", + "doc": "Whether unused schema deletion is enabled or not.", + "type": "boolean", + "default": false + }, + { + "name": "blobTransferEnabled", + "doc": "Flag to indicate if the blob transfer is allowed or not", + "type": "boolean", + "default": false + }, + { + "name": "nearlineProducerCompressionEnabled", + "doc": "Flag to control whether the producer in Server for nearline workload will enable compression or not", + "type": "boolean", + "default": true + }, + { + "name": "nearlineProducerCountPerWriter", + "doc": "How many producers will be used for the nearline producer in Server to improve producing throughput", + "type": "int", + "default": 1 + }, + { + "name": "targetSwapRegion", + "doc": "Controls what region to swap in the current version during target colo push", + "type": ["null","string"], + "default": null + }, + { + "name": "targetSwapRegionWaitTime", + "doc": "Controls how long to wait in minutes before swapping the version on the regions", + "type": "int", + "default": 60 + }, + { + "name": "isDaVinciHeartBeatReported", + "doc": "Flag to indicate whether DVC is bootstrapping and sending heartbeats", + "type": "boolean", + "default": false + }, + { + "name": "globalRtDivEnabled", + "doc": "Flag to indicate whether the Global RT DIV feature is on. The DIV will be centralized in the ConsumptionTask, and leaders will periodically replicate the RT DIV to followers via VT.", + "type": "boolean", + "default": false + }, + { + "name": "enumSchemaEvolutionAllowed", + "doc": "Flag to control whether a certain store is allowed to evolve enum schema since the readers need to use Avro-1.9+", + "type": "boolean", + "default": false + }, + { + "name": "storeLifecycleHooks", + "doc": "List of store lifecycle hooks", + "type": { + "type": "array", + "items": { + "name": "StoreLifecycleHooksRecord", + "type": "record", + "fields": [ + {"name": "storeLifecycleHooksClassName", "type": "string", "doc": "FQCN of the hook implementation"}, + {"name": "storeLifecycleHooksParams", "type": {"type": "map", "values": "string"}, "doc": "Bag of properties to pass into the hook implementation"} + ] + } + }, + "default": [] + }, + { + "name": "blobTransferInServerEnabled", + "doc": "Flag to indicate if the blob transfer is allowed or not in server. Values can be 'NOT_SPECIFIED' as default, 'ENABLED', or 'DISABLED'.", + "type": "string", + "default": "NOT_SPECIFIED" + }, + { + "name": "keyUrnCompressionEnabled", + "doc": "Whether key URN compression is enabled for this store", + "type": "boolean", + "default": false + }, + { + "name": "keyUrnFields", + "doc": "List of fields in the key schema that will be eligible for key urn compression", + "type": { + "type": "array", + "items": "string" + }, + "default": [] + }, + { + "name": "flinkVeniceViewsEnabled", + "doc": "Whether this store is enabled to use Flink-based Venice Views", + "type": "boolean", + "default": false + }, + { + "name": "throughputQuotaInBytes", + "doc": "The maximum throughput measured in bytes that clients can produce into one store. Default -1 means no limit.", + "type": "long", + "default": -1 + }, + { + "name": "throughputQuotaInRecords", + "doc": "The maximum throughput measured in records that clients can produce into one store. Default -1 means no limit.", + "type": "long", + "default": -1 + }, + { + "name": "blobDbEnabled", + "doc": "Flag to indicate if the RocksDB BlobDB feature is enabled or not. Values can be 'NOT_SPECIFIED' (default, follows cluster level config), 'ENABLED', or 'DISABLED'.", + "type": "string", + "default": "NOT_SPECIFIED" + }, + { + "name": "previousCurrentVersion", + "doc": "Int representing the previous current version before the current version was marked current", + "type": "int", + "default": -1 + }, + { + "name": "transientRecordCacheEnabled", + "doc": "Whether the bounded hot transient record cache is enabled for this store to retain frequently accessed large records across consumer poll boundaries.", + "type": "boolean", + "default": false + }, + { + "name": "mergedValueRmdColumnFamilyEnabled", + "doc": "Whether to store value and RMD in a single column family to reduce read amplification during A/A ingestion.", + "type": "boolean", + "default": false + }, + { + "name": "ingestionPauseMode", + "doc": "Ingestion pause mode. 0 => NOT_PAUSED, 1 => CURRENT_VERSION, 2 => ALL_VERSIONS", + "type": "int", + "default": 0 + }, + { + "name": "ingestionPausedRegions", + "doc": "List of regions where pause is applied. Empty list means all regions.", + "type": {"type": "array", "items": "string"}, + "default": [] + }, + { + "name": "targetRegionPromoted", + "doc": "Flag set by the parent controller when the target region has promoted the future version to current. Propagated to child controllers so DaVinci clients can resume paused ingestion.", + "type": "boolean", + "default": false + }, + { + "name": "storageMode", + "doc": "Store-level default storage mode. The controller persists this value on the store record (StoreProperties.storageMode) and also copies it into StoreVersion.storageMode when a new store version is created; existing versions are unaffected. Controls where version data is persisted in addition to (or instead of) Venice local storage. 0 => INTERNAL (default, Venice-only); 1 => DUAL_WRITE (data is written to both Venice local storage and the configured external storage; the specific dual-write implementation -- leader-consumer-pipeline vs Venice Push Job -- is selected by separate server/VPJ configuration); 2 => EXTERNAL (external-storage-only; Venice's data partition becomes NoOp while metadata partitions are still persisted locally for checkpointing).", + "type": "int", + "default": 0 + }, + { + "name": "externalStorageReadMode", + "doc": "Store-level read routing applied to the store as a whole (not per-version). Controls how clients route reads between Venice local storage and the configured external storage. 0 => VENICE_ONLY (default, reads served from Venice local storage only -- current behavior); 1 => DUAL_MODE_CONSISTENCY_CHECK (the client reads from both Venice and the external storage and verifies/reports divergence; used to validate dual-write correctness before cutover); 2 => DUAL_MODE_EARLY_RETURN (the client issues reads against both Venice and the external storage in parallel and returns the first response, falling back to the slower one on miss); 3 => EXTERNAL_ONLY (reads served from the external storage only, with Venice acting as the metadata/CDC path).", + "type": "int", + "default": 0 + } + ] + }, + { + "name": "DeleteStore", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + }, + { + "name": "largestUsedVersionNumber", + "type": "int" + } + ] + }, + { + "name": "DeleteOldVersion", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + }, + { + "name": "versionNum", + "type": "int" + } + ] + }, + { + "name": "MigrateStore", + "type": "record", + "fields": [ + { + "name": "srcClusterName", + "type": "string" + }, + { + "name": "destClusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + } + ] + }, + { + "name": "AbortMigration", + "type": "record", + "fields": [ + { + "name": "srcClusterName", + "type": "string" + }, + { + "name": "destClusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + } + ] + }, + { + "name": "AddVersion", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + }, + { + "name": "pushJobId", + "type": "string" + }, + { + "name": "versionNum", + "type": "int" + }, + { + "name": "numberOfPartitions", + "type": "int" + }, + { + "name": "pushType", + "doc": "The push type of the new version, 0 => BATCH, 1 => STREAM_REPROCESSING. Previous add version messages will default to BATCH and this is a safe because they were created when BATCH was the only version type", + "type": "int", + "default": 0 + }, + { + "name": "pushStreamSourceAddress", + "type": ["null", "string"], + "default": null + }, + { + "name": "rewindTimeInSecondsOverride", + "doc": "The overridable rewind time config for this specific version of a hybrid store, and if it is not specified, the new version will use the store-level rewind time config", + "type": "long", + "default": -1 + }, + { + "name": "timestampMetadataVersionId", + "doc": "The A/A metadata schema version ID that will be used to deserialize metadataPayload.", + "type": "int", + "default": -1 + }, + { + "name": "versionSwapDeferred", + "doc": "Indicates if swapping this version to current version after push completion should be initiated or not", + "type": "boolean", + "default": false + }, + { + "name": "targetedRegions", + "doc": "The list of regions that is separated by comma for targeted region push. If set, this admin message should only be consumed by the targeted regions", + "type": [ + "null", + { + "type": "array", + "items": "string" + } + ], + "default": null + }, + { + "name": "repushSourceVersion", + "doc": "Indicates the source version from which a repush version is created", + "type": "int", + "default": -1 + }, + { + "name": "currentRTVersionNumber", + "type": "int", + "doc": "current RT version number that should be used to formulate real time topic name during add version", + "default": 0 + }, + { + "name": "repushTtlSeconds", + "type": "int", + "doc": "For store version created from repush, indicates the time-to-live in seconds set during the repush", + "default": -1 + }, + { + "name": "degradedDatacenters", + "doc": "List of datacenter names marked as degraded at the time of version creation. Child controllers use this in AdminExecutionTask to enforce skipConsumption for degraded DCs even when versionSwapDeferred=true, preventing ghost versions in degraded DCs.", + "type": [ + "null", + { + "type": "array", + "items": "string" + } + ], + "default": null + } + ] + }, + { + "name": "DerivedSchemaCreation", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + }, + { + "name": "schema", + "type": "SchemaMeta" + }, + { + "name": "valueSchemaId", + "type": "int" + }, + { + "name": "derivedSchemaId", + "type": "int" + } + ] + }, + { + "name": "SupersetSchemaCreation", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + }, + { + "name": "valueSchema", + "type": "SchemaMeta" + }, + { + "name": "valueSchemaId", + "type": "int" + }, + { + "name": "supersetSchema", + "type": "SchemaMeta" + }, + { + "name": "supersetSchemaId", + "type": "int" + } + ] + }, + { + "name": "ConfigureNativeReplicationForCluster", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeType", + "type": "string" + }, + { + "name": "enabled", + "type": "boolean" + }, + { + "name": "nativeReplicationSourceRegion", + "doc": "The source region to be used when the store is running in Native Replication mode.", + "type": ["null", "string"], + "default": null + }, + { + "name": "regionsFilter", + "type": ["null", "string"], + "default": null + } + ] + }, + { + "name": "MetadataSchemaCreation", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + }, + { + "name": "valueSchemaId", + "type": "int" + }, + { + "name": "metadataSchema", + "type": "SchemaMeta" + }, + { + "name": "timestampMetadataVersionId", + "type": "int", + "aliases": ["metadataVersionId"], + "default": -1 + } + ] + }, + { + "name": "ConfigureActiveActiveReplicationForCluster", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeType", + "type": "string" + }, + { + "name": "enabled", + "type": "boolean" + }, + { + "name": "regionsFilter", + "type": ["null", "string"], + "default": null + } + ] + }, { + "name": "ConfigureIncrementalPushForCluster", + "doc": "A command to migrate all incremental push stores in a cluster to a specific incremental push policy.", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "incrementalPushPolicyToFilter", + "doc": "If this batch update command is trying to configure existing incremental push store type, their incremental push policy should also match this filter before the batch update command applies any change to them. Default value is -1, meaning there is no filter.", + "type": "int", + "default": -1 + }, + { + "name": "incrementalPushPolicyToApply", + "doc": "This field will determine what incremental push policy will be applied to the selected stores. Default value is 1, which is the INCREMENTAL_PUSH_SAME_AS_REAL_TIME policy", + "type": "int", + "default": 1 + }, + { + "name": "regionsFilter", + "type": ["null", "string"], + "default": null + } + ] + }, { + "name": "MetaSystemStoreAutoCreationValidation", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + } + ] + }, { + "name": "PushStatusSystemStoreAutoCreationValidation", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + } + ] + }, { + "name": "CreateStoragePersona", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "name", + "type": "string" + }, + { + "name": "quotaNumber", + "type": "long" + }, + { + "name": "storesToEnforce", + "type": { + "type": "array", + "items": "string", + "default": [] + } + }, + { + "name": "owners", + "type": { + "type": "array", + "items": "string", + "default": [] + } + } + ] + }, { + "name": "DeleteStoragePersona", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "name", + "type": "string" + } + ] + }, { + "name": "UpdateStoragePersona", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, { + "name": "name", + "type": "string" + }, { + "name": "quotaNumber", + "type": ["null","long"], + "default": null + }, { + "name": "storesToEnforce", + "type": [ + "null", + { + "type": "array", + "items": "string" + } + ], + "default": null + }, { + "name": "owners", + "type": [ + "null", + { + "type": "array", + "items": "string" + } + ], + "default": null + } + ] + }, + { + "name": "DeleteUnusedValueSchemas", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + }, + { + "name": "schemaIds", + "type": { + "type": "array", + "items": "int", + "default": [] + } + } + ] + }, + { + "name": "RollbackCurrentVersion", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + }, + { + "name": "regionsFilter", + "doc": "A list of regions that will be impacted by the RollbackCurrentVersion command", + "type": ["null", "string"], + "default": null + } + ] + }, + { + "name": "RollForwardCurrentVersion", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + }, + { + "name": "regionsFilter", + "doc": "A list of regions that will be impacted by the RollForwardCurrentVersion command", + "type": ["null", "string"], + "default": null + } + ] + }, + { + "name": "MarkVersionRolledBack", + "type": "record", + "fields": [ + { + "name": "clusterName", + "type": "string" + }, + { + "name": "storeName", + "type": "string" + }, + { + "name": "versionNum", + "doc": "The version number to mark as ROLLED_BACK, without changing the current version.", + "type": "int" + }, + { + "name": "regionsFilter", + "doc": "A list of regions that will be impacted by the MarkVersionRolledBack command", + "type": ["null", "string"], + "default": null + } + ] + } + ] + } + ] +} diff --git a/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestDeferredVersionSwapServiceWithSequentialRollout.java b/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestDeferredVersionSwapServiceWithSequentialRollout.java index 87bd8720453..7ee316f554a 100644 --- a/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestDeferredVersionSwapServiceWithSequentialRollout.java +++ b/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestDeferredVersionSwapServiceWithSequentialRollout.java @@ -4,10 +4,13 @@ import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.anyDouble; +import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -39,6 +42,7 @@ import java.time.ZoneOffset; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -48,8 +52,11 @@ import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; +import org.mockito.ArgumentCaptor; +import org.mockito.InOrder; import org.testng.Assert; import org.testng.annotations.BeforeMethod; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -438,6 +445,7 @@ public void testSequentialRolloutFailurePath() throws Exception { TestUtils.waitForNonDeterministicAssertion(5, TimeUnit.SECONDS, () -> { // Verify error recording was called due to the failure verify(store, atLeastOnce()).updateVersionStatus(2, VersionStatus.PARTIALLY_ONLINE); + verify(admin, atLeastOnce()).markVersionRolledBack(eq(clusterName), eq(storeName), eq(versionTwo), anyString()); verify(admin, never()).rollForwardToFutureVersion(clusterName, storeName, region3); verify(admin, never()).truncateKafkaTopic(anyString()); }); @@ -513,6 +521,317 @@ public void testSequentialRolloutVersionValidationFails() throws Exception { verify(stats, never()).recordDeferredVersionSwapExceptionMetric(anyString()); } + /** + * When post-version-swap validation returns ROLLBACK, the target region(s) are rolled back and + * bootstrap-complete non-current child copies are reconciled. A child that still reports the failed + * version as current remains protected and keeps the reconciliation retriable. + */ + @Test + public void testSequentialRolloutPostSwapValidationRollbackMarksNonTargetRegions() throws Exception { + String storeName = "testStore"; + Store store = mockStore(versionOne, versionTwo, storeName); + + // Lifecycle hook that returns ROLLBACK during post-version-swap validation. + List lifecycleHooks = new ArrayList<>(); + Map params = new HashMap<>(); + params.put("outcome", StoreVersionLifecycleEventOutcome.ROLLBACK.toString()); + lifecycleHooks.add(new LifecycleHooksRecordImpl(MockStoreLifecycleHooks.class.getName(), params)); + doReturn(lifecycleHooks).when(store).getStoreLifecycleHooks(); + + // Wire the hooks cache to instantiate the mock hook. + StoreLifecycleHooksCache hooksCache = mock(StoreLifecycleHooksCache.class); + doReturn(new MockStoreLifecycleHooks(new VeniceProperties(new Properties()))).when(hooksCache) + .getOrInstantiateHook(MockStoreLifecycleHooks.class.getName()); + doReturn(hooksCache).when(veniceHelixAdmin).getStoreLifecycleHooksCache(); + + List storeList = new ArrayList<>(); + storeList.add(store); + doReturn(storeList).when(admin).getAllStores(clusterName); + doReturn(true).when(admin).isLeaderControllerFor(clusterName); + + Version versionOneImpl = new VersionImpl(storeName, versionOne); + Version versionTwoImpl = new VersionImpl(storeName, versionTwo); + versionTwoImpl.setStatus(VersionStatus.PUSHED); + List versionList = new ArrayList<>(); + versionList.add(versionOneImpl); + versionList.add(versionTwoImpl); + StoreResponse storeResponse = getStoreResponse(versionList); + + Map controllerClientMap = mockControllerClients(versionList); + for (Map.Entry entry: controllerClientMap.entrySet()) { + doReturn(storeResponse).when(entry.getValue()).getStore(anyString(), anyInt()); + } + + doReturn(store).when(repository).getStore(storeName); + + Long time = LocalDateTime.now().toEpochSecond(ZoneOffset.UTC); + Admin.OfflinePushStatusInfo completedPush = getOfflinePushStatusInfo( + ExecutionStatus.COMPLETED.toString(), + ExecutionStatus.COMPLETED.toString(), + ExecutionStatus.COMPLETED.toString(), + time - TimeUnit.MINUTES.toSeconds(90), + time - TimeUnit.MINUTES.toSeconds(30), + time - TimeUnit.MINUTES.toSeconds(30)); + String kafkaTopicName = Version.composeKafkaTopic(storeName, versionTwo); + doReturn(completedPush).when(admin).getOffLinePushStatus(clusterName, kafkaTopicName); + + DeferredVersionSwapService deferredVersionSwapService = + new DeferredVersionSwapService(admin, veniceControllerMultiClusterConfig, stats, metricsRepository); + deferredVersionSwapService.startInner(); + + ArgumentCaptor regionFilterCaptor = ArgumentCaptor.forClass(String.class); + TestUtils.waitForNonDeterministicAssertion(5, TimeUnit.SECONDS, () -> { + // Target region (region1, the prior rolled-forward region) is rolled back. + verify(admin, atLeastOnce()).rollbackToBackupVersion(clusterName, storeName, region1); + // Non-current child regions reconcile the abandoned version. + verify(admin, atLeastOnce()) + .markVersionRolledBack(eq(clusterName), eq(storeName), eq(versionTwo), regionFilterCaptor.capture()); + // No roll forward should happen for region2 after a ROLLBACK. + verify(admin, never()).rollForwardToFutureVersion(clusterName, storeName, region2); + }); + + Set reconciledRegions = new HashSet<>(Arrays.asList(regionFilterCaptor.getValue().split(","))); + Assert.assertEquals(reconciledRegions, new HashSet<>(Arrays.asList(region2, region3))); + } + + @DataProvider(name = "abandonedParentStatuses") + public Object[][] abandonedParentStatuses() { + return new Object[][] { + { VersionStatus.ERROR }, + { VersionStatus.PARTIALLY_ONLINE }, + { VersionStatus.ROLLED_BACK } }; + } + + @Test(dataProvider = "abandonedParentStatuses") + public void testAbandonedParentStatusReconcilesEligibleChildren(VersionStatus parentStatus) { + String storeName = "testStore"; + Store store = mockStore(versionOne, versionTwo, storeName); + doReturn(store).when(repository).getStore(storeName); + doReturn(ConcurrentPushDetectionStrategy.PARENT_VERSION_STATUS_ONLY).when(clusterConfig) + .getConcurrentPushDetectionStrategy(); + + DeferredVersionSwapService deferredVersionSwapService = + new DeferredVersionSwapService(admin, veniceControllerMultiClusterConfig, stats, metricsRepository); + deferredVersionSwapService.updateStore(clusterName, storeName, parentStatus, versionTwo); + + ArgumentCaptor regionFilterCaptor = ArgumentCaptor.forClass(String.class); + InOrder inOrder = inOrder(admin, store); + inOrder.verify(store).updateVersionStatus(versionTwo, parentStatus); + inOrder.verify(admin) + .markVersionRolledBack(eq(clusterName), eq(storeName), eq(versionTwo), regionFilterCaptor.capture()); + Assert.assertEquals( + new HashSet<>(Arrays.asList(regionFilterCaptor.getValue().split(","))), + new HashSet<>(Arrays.asList(region2, region3))); + } + + @Test + public void testAbandonedParentStatusPersistsWhenChildReconciliationFails() { + String storeName = "testStore"; + Store store = mockStore(versionOne, versionTwo, storeName); + doReturn(store).when(repository).getStore(storeName); + doThrow(new VeniceException("child reconciliation failed")).when(admin) + .markVersionRolledBack(eq(clusterName), eq(storeName), eq(versionTwo), anyString()); + + DeferredVersionSwapService deferredVersionSwapService = + new DeferredVersionSwapService(admin, veniceControllerMultiClusterConfig, stats, metricsRepository); + deferredVersionSwapService.updateStore(clusterName, storeName, VersionStatus.ERROR, versionTwo); + + verify(store).updateVersionStatus(versionTwo, VersionStatus.ERROR); + verify(admin).markVersionRolledBack(eq(clusterName), eq(storeName), eq(versionTwo), anyString()); + } + + @Test + public void testTerminalParentStatusRepairsStrandedChildVersions() throws Exception { + String storeName = "testStore"; + Store store = mockStore(versionOne, versionTwo, storeName); + Version targetVersion = store.getVersion(versionTwo); + doReturn(VersionStatus.ERROR).when(targetVersion).getStatus(); + doReturn(VersionStatus.ERROR).when(store).getVersionStatus(versionTwo); + doReturn(Arrays.asList(targetVersion)).when(store).getVersions(); + doReturn(Arrays.asList(store)).when(admin).getAllStores(clusterName); + doReturn(true).when(admin).isLeaderControllerFor(clusterName); + + DeferredVersionSwapService deferredVersionSwapService = + new DeferredVersionSwapService(admin, veniceControllerMultiClusterConfig, stats, metricsRepository); + deferredVersionSwapService.startInner(); + + ArgumentCaptor regionFilterCaptor = ArgumentCaptor.forClass(String.class); + TestUtils.waitForNonDeterministicAssertion(5, TimeUnit.SECONDS, () -> verify(admin, atLeastOnce()) + .markVersionRolledBack(eq(clusterName), eq(storeName), eq(versionTwo), regionFilterCaptor.capture())); + Assert.assertEquals( + new HashSet<>(Arrays.asList(regionFilterCaptor.getValue().split(","))), + new HashSet<>(Arrays.asList(region2, region3))); + } + + @Test + public void testTerminalParentStatusRetriesUntilInProgressChildCompletes() throws Exception { + String storeName = "testStore"; + Store store = mockStore(versionOne, versionTwo, storeName); + Version targetVersion = store.getVersion(versionTwo); + doReturn(VersionStatus.ERROR).when(targetVersion).getStatus(); + doReturn(VersionStatus.ERROR).when(store).getVersionStatus(versionTwo); + doReturn(Arrays.asList(targetVersion)).when(store).getVersions(); + doReturn(Arrays.asList(store)).when(admin).getAllStores(clusterName); + doReturn(true).when(admin).isLeaderControllerFor(clusterName); + + Version startedVersion = new VersionImpl(storeName, versionTwo); + startedVersion.setStatus(VersionStatus.STARTED); + Version pushedVersion = new VersionImpl(storeName, versionTwo); + pushedVersion.setStatus(VersionStatus.PUSHED); + Store regionStore = mockRegionalStore(versionOne, versionTwo, storeName); + StoreInfo startedStoreInfo = StoreInfo.fromStore(regionStore); + startedStoreInfo.setVersions(Arrays.asList(startedVersion)); + StoreResponse startedResponse = new StoreResponse(); + startedResponse.setStore(startedStoreInfo); + StoreInfo pushedStoreInfo = StoreInfo.fromStore(regionStore); + pushedStoreInfo.setVersions(Arrays.asList(pushedVersion)); + StoreResponse pushedResponse = new StoreResponse(); + pushedResponse.setStore(pushedStoreInfo); + + Map childControllers = veniceHelixAdmin.getControllerClientMap(clusterName); + doReturn(startedResponse, pushedResponse).when(childControllers.get(region2)) + .getStore(storeName, controllerTimeout); + + DeferredVersionSwapService deferredVersionSwapService = + new DeferredVersionSwapService(admin, veniceControllerMultiClusterConfig, stats, metricsRepository); + deferredVersionSwapService.startInner(); + + ArgumentCaptor regionFilterCaptor = ArgumentCaptor.forClass(String.class); + TestUtils.waitForNonDeterministicAssertion(5, TimeUnit.SECONDS, () -> { + verify(childControllers.get(region2), atLeastOnce()).getStore(storeName, controllerTimeout); + verify(admin, atLeastOnce()) + .markVersionRolledBack(eq(clusterName), eq(storeName), eq(versionTwo), regionFilterCaptor.capture()); + Assert.assertTrue( + regionFilterCaptor.getAllValues() + .stream() + .map(filter -> new HashSet<>(Arrays.asList(filter.split(",")))) + .anyMatch(regions -> regions.contains(region2))); + }); + } + + @Test + public void testTerminalParentStatusRetriesWhenChildVersionAppearsLater() throws Exception { + String storeName = "testStore"; + Store store = mockStore(versionOne, versionTwo, storeName); + Version targetVersion = store.getVersion(versionTwo); + doReturn(VersionStatus.ERROR).when(targetVersion).getStatus(); + doReturn(VersionStatus.ERROR).when(store).getVersionStatus(versionTwo); + doReturn(Arrays.asList(targetVersion)).when(store).getVersions(); + doReturn(Arrays.asList(store)).when(admin).getAllStores(clusterName); + doReturn(true).when(admin).isLeaderControllerFor(clusterName); + + Store regionStore = mockRegionalStore(versionOne, versionTwo, storeName); + StoreInfo missingStoreInfo = StoreInfo.fromStore(regionStore); + missingStoreInfo.setVersions(Collections.emptyList()); + StoreResponse missingResponse = new StoreResponse(); + missingResponse.setStore(missingStoreInfo); + Version pushedVersion = new VersionImpl(storeName, versionTwo); + pushedVersion.setStatus(VersionStatus.PUSHED); + StoreInfo pushedStoreInfo = StoreInfo.fromStore(regionStore); + pushedStoreInfo.setVersions(Arrays.asList(pushedVersion)); + StoreResponse pushedResponse = new StoreResponse(); + pushedResponse.setStore(pushedStoreInfo); + + Map childControllers = veniceHelixAdmin.getControllerClientMap(clusterName); + doReturn(missingResponse, pushedResponse).when(childControllers.get(region2)) + .getStore(storeName, controllerTimeout); + + DeferredVersionSwapService deferredVersionSwapService = + new DeferredVersionSwapService(admin, veniceControllerMultiClusterConfig, stats, metricsRepository); + deferredVersionSwapService.startInner(); + + ArgumentCaptor regionFilterCaptor = ArgumentCaptor.forClass(String.class); + TestUtils.waitForNonDeterministicAssertion(5, TimeUnit.SECONDS, () -> { + verify(childControllers.get(region2), atLeast(2)).getStore(storeName, controllerTimeout); + verify(admin, atLeastOnce()) + .markVersionRolledBack(eq(clusterName), eq(storeName), eq(versionTwo), regionFilterCaptor.capture()); + Assert.assertTrue( + regionFilterCaptor.getAllValues() + .stream() + .map(filter -> new HashSet<>(Arrays.asList(filter.split(",")))) + .anyMatch(regions -> regions.contains(region2))); + }); + } + + @Test + public void testTerminalParentStatusRetriesCurrentTargetAfterItBecomesNonCurrent() throws Exception { + String storeName = "testStore"; + Store store = mockStore(versionOne, versionTwo, storeName); + Version targetVersion = store.getVersion(versionTwo); + doReturn(VersionStatus.ERROR).when(targetVersion).getStatus(); + doReturn(VersionStatus.ERROR).when(store).getVersionStatus(versionTwo); + doReturn(Arrays.asList(targetVersion)).when(store).getVersions(); + doReturn(Arrays.asList(store)).when(admin).getAllStores(clusterName); + doReturn(true).when(admin).isLeaderControllerFor(clusterName); + + Version pushedVersion = new VersionImpl(storeName, versionTwo); + pushedVersion.setStatus(VersionStatus.PUSHED); + StoreInfo currentTargetStoreInfo = StoreInfo.fromStore(mockRegionalStore(versionTwo, versionTwo, storeName)); + currentTargetStoreInfo.setVersions(Arrays.asList(pushedVersion)); + StoreResponse currentTargetResponse = new StoreResponse(); + currentTargetResponse.setStore(currentTargetStoreInfo); + StoreInfo nonCurrentTargetStoreInfo = StoreInfo.fromStore(mockRegionalStore(versionOne, versionTwo, storeName)); + nonCurrentTargetStoreInfo.setVersions(Arrays.asList(pushedVersion)); + StoreResponse nonCurrentTargetResponse = new StoreResponse(); + nonCurrentTargetResponse.setStore(nonCurrentTargetStoreInfo); + + Map childControllers = veniceHelixAdmin.getControllerClientMap(clusterName); + doReturn(currentTargetResponse, nonCurrentTargetResponse).when(childControllers.get(region1)) + .getStore(storeName, controllerTimeout); + + DeferredVersionSwapService deferredVersionSwapService = + new DeferredVersionSwapService(admin, veniceControllerMultiClusterConfig, stats, metricsRepository); + deferredVersionSwapService.startInner(); + + ArgumentCaptor regionFilterCaptor = ArgumentCaptor.forClass(String.class); + TestUtils.waitForNonDeterministicAssertion(5, TimeUnit.SECONDS, () -> { + verify(childControllers.get(region1), atLeast(2)).getStore(storeName, controllerTimeout); + verify(admin, atLeastOnce()) + .markVersionRolledBack(eq(clusterName), eq(storeName), eq(versionTwo), regionFilterCaptor.capture()); + Assert.assertTrue( + regionFilterCaptor.getAllValues() + .stream() + .map(filter -> new HashSet<>(Arrays.asList(filter.split(",")))) + .anyMatch(regions -> regions.containsAll(Arrays.asList(region1, region2, region3)))); + }); + } + + @Test + public void testTerminalReconciliationIncludesSupersededParentVersions() throws Exception { + String storeName = "testStore"; + int latestVersionNumber = 3; + Store store = mockStore(versionOne, latestVersionNumber, storeName); + Version latestVersion = store.getVersion(latestVersionNumber); + Version supersededVersion = mock(Version.class); + doReturn(versionTwo).when(supersededVersion).getNumber(); + doReturn(VersionStatus.ERROR).when(supersededVersion).getStatus(); + doReturn(true).when(supersededVersion).isVersionSwapDeferred(); + doReturn(Arrays.asList(supersededVersion, latestVersion)).when(store).getVersions(); + doReturn(Arrays.asList(store)).when(admin).getAllStores(clusterName); + doReturn(true).when(admin).isLeaderControllerFor(clusterName); + + Version pushedVersion = new VersionImpl(storeName, versionTwo); + pushedVersion.setStatus(VersionStatus.PUSHED); + StoreInfo childStoreInfo = StoreInfo.fromStore(mockRegionalStore(versionOne, versionTwo, storeName)); + childStoreInfo.setVersions(Arrays.asList(pushedVersion)); + StoreResponse childStoreResponse = new StoreResponse(); + childStoreResponse.setStore(childStoreInfo); + for (ControllerClient childController: veniceHelixAdmin.getControllerClientMap(clusterName).values()) { + doReturn(childStoreResponse).when(childController).getStore(storeName, controllerTimeout); + } + + DeferredVersionSwapService deferredVersionSwapService = + new DeferredVersionSwapService(admin, veniceControllerMultiClusterConfig, stats, metricsRepository); + deferredVersionSwapService.startInner(); + + ArgumentCaptor regionFilterCaptor = ArgumentCaptor.forClass(String.class); + TestUtils.waitForNonDeterministicAssertion(5, TimeUnit.SECONDS, () -> verify(admin, atLeastOnce()) + .markVersionRolledBack(eq(clusterName), eq(storeName), eq(versionTwo), regionFilterCaptor.capture())); + Assert.assertEquals( + new HashSet<>(Arrays.asList(regionFilterCaptor.getValue().split(","))), + new HashSet<>(Arrays.asList(region1, region2, region3))); + } + /** * When the last region in rollout order is ONLINE, * parent version is marked as ONLINE diff --git a/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestStoreBackupVersionCleanupService.java b/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestStoreBackupVersionCleanupService.java index 2c6f6365261..33f07cb3eff 100644 --- a/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestStoreBackupVersionCleanupService.java +++ b/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestStoreBackupVersionCleanupService.java @@ -822,6 +822,55 @@ public void testRegularPushBackupNotDeletedEarlyWhenRepushChainBroken() { verify(admin, atLeast(1)).deleteOldVersionInStore(CLUSTER_NAME, storeExpired.getName(), 10); } + @Test + public void testPushedVersionIsNotDeletedAfterDefaultRetention() { + Map versions = new HashMap<>(); + versions.put(1, VersionStatus.PUSHED); + versions.put(2, VersionStatus.ONLINE); + long expiredRetention = mockTime.getMilliseconds() - DEFAULT_RETENTION_MS - 1; + Store store = mockStore(-1, expiredRetention, versions, 2); + Version currentVersion = store.getVersion(2); + doReturn(-1).when(currentVersion).getRepushSourceVersion(); + doReturn(currentVersion).when(store).getVersionOrThrow(2); + + Assert.assertFalse(service.cleanupBackupVersion(store, CLUSTER_NAME)); + verify(admin, never()).deleteOldVersionInStore(CLUSTER_NAME, store.getName(), 1); + } + + @Test + public void testPushedVersionIsNotDeletedAsRepushSource() { + Map versions = new HashMap<>(); + versions.put(2, VersionStatus.PUSHED); + versions.put(3, VersionStatus.ONLINE); + long pastMinimumRetention = mockTime.getMilliseconds() - TimeUnit.HOURS.toMillis(2); + Store store = mockStore(-1, pastMinimumRetention, versions, 3); + Version pushedVersion = store.getVersion(2); + doReturn(1).when(pushedVersion).getRepushSourceVersion(); + Version currentVersion = store.getVersion(3); + doReturn(2).when(currentVersion).getRepushSourceVersion(); + doReturn(currentVersion).when(store).getVersionOrThrow(3); + + Assert.assertFalse(service.cleanupBackupVersion(store, CLUSTER_NAME)); + verify(admin, never()).deleteOldVersionInStore(CLUSTER_NAME, store.getName(), 2); + } + + @Test + public void testPushedVersionDoesNotMakeOnlineBackupEligibleEarly() { + Map versions = new HashMap<>(); + versions.put(1, VersionStatus.ONLINE); + versions.put(2, VersionStatus.PUSHED); + versions.put(3, VersionStatus.ONLINE); + long pastMinimumRetention = mockTime.getMilliseconds() - TimeUnit.HOURS.toMillis(2); + Store store = mockStore(-1, pastMinimumRetention, versions, 3); + Version currentVersion = store.getVersion(3); + doReturn(-1).when(currentVersion).getRepushSourceVersion(); + doReturn(currentVersion).when(store).getVersionOrThrow(3); + + Assert.assertFalse(service.cleanupBackupVersion(store, CLUSTER_NAME)); + verify(admin, never()).deleteOldVersionInStore(CLUSTER_NAME, store.getName(), 1); + verify(admin, never()).deleteOldVersionInStore(CLUSTER_NAME, store.getName(), 2); + } + @org.testng.annotations.DataProvider(name = "rolledBackVersionCleanupParams") public Object[][] rolledBackVersionCleanupParams() { // { hoursElapsed, extraVersions (status map beyond v1 ONLINE + v2 ROLLED_BACK), expectCleanup, diff --git a/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestVeniceHelixAdmin.java b/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestVeniceHelixAdmin.java index d117d7c5493..f4458f94cf6 100644 --- a/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestVeniceHelixAdmin.java +++ b/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestVeniceHelixAdmin.java @@ -107,6 +107,7 @@ import org.mockito.MockedStatic; import org.mockito.Mockito; import org.testng.TestException; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -1292,6 +1293,63 @@ public void testRollForwardNoFutureVersions() { verify(mockVeniceHelixAdmin, never()).storeMetadataUpdate(any(), any(), any()); } + @DataProvider(name = "markVersionRolledBackStatuses") + public Object[][] markVersionRolledBackStatuses() { + return new Object[][] { + { VersionStatus.PUSHED, 1, true }, + { VersionStatus.ONLINE, 1, true }, + { VersionStatus.STARTED, 1, false }, + { VersionStatus.CREATED, 1, false }, + { VersionStatus.NOT_CREATED, 1, false }, + { VersionStatus.ERROR, 1, false }, + { VersionStatus.KILLED, 1, false }, + { VersionStatus.ROLLED_BACK, 1, false }, + { VersionStatus.PUSHED, 2, false } }; + } + + @Test(dataProvider = "markVersionRolledBackStatuses") + public void testMarkVersionRolledBackOnlyUpdatesCompletedNonCurrentVersions( + VersionStatus initialStatus, + int currentVersion, + boolean expectUpdate) { + VeniceHelixAdmin mockVeniceHelixAdmin = mock(VeniceHelixAdmin.class); + Store mockStore = mock(Store.class); + HelixVeniceClusterResources mockClusterResources = mock(HelixVeniceClusterResources.class); + doReturn("region1").when(mockVeniceHelixAdmin).getRegionName(); + doReturn(true).when(mockStore).containsVersion(2); + doReturn(currentVersion).when(mockStore).getCurrentVersion(); + doReturn(initialStatus).when(mockStore).getVersionStatus(2); + doAnswer(invocation -> { + VeniceHelixAdmin.StoreMetadataOperation updater = invocation.getArgument(2); + updater.update(mockStore, mockClusterResources); + return null; + }).when(mockVeniceHelixAdmin).storeMetadataUpdate(eq(clusterName), eq(storeName), any()); + doCallRealMethod().when(mockVeniceHelixAdmin) + .markVersionRolledBack(anyString(), anyString(), anyInt(), anyString()); + + mockVeniceHelixAdmin.markVersionRolledBack(clusterName, storeName, 2, "region1"); + + if (expectUpdate) { + verify(mockStore).updateVersionStatus(2, VersionStatus.ROLLED_BACK); + verify(mockStore).setLatestVersionPromoteToCurrentTimestamp(anyLong()); + } else { + verify(mockStore, never()).updateVersionStatus(anyInt(), any()); + verify(mockStore, never()).setLatestVersionPromoteToCurrentTimestamp(anyLong()); + } + } + + @Test + public void testMarkVersionRolledBackHonorsRegionFilter() { + VeniceHelixAdmin mockVeniceHelixAdmin = mock(VeniceHelixAdmin.class); + doReturn("region1").when(mockVeniceHelixAdmin).getRegionName(); + doCallRealMethod().when(mockVeniceHelixAdmin) + .markVersionRolledBack(anyString(), anyString(), anyInt(), anyString()); + + mockVeniceHelixAdmin.markVersionRolledBack(clusterName, storeName, 2, "region2"); + + verify(mockVeniceHelixAdmin, never()).storeMetadataUpdate(anyString(), anyString(), any()); + } + /** * isPartitionReadyToServe=>true: Future version exists and partitions are ready → success * isPartitionReadyToServe=>false: Future version exists but partitions aren’t ready → exception diff --git a/services/venice-controller/src/test/java/com/linkedin/venice/controller/kafka/consumer/AdminExecutionTaskTest.java b/services/venice-controller/src/test/java/com/linkedin/venice/controller/kafka/consumer/AdminExecutionTaskTest.java index 41ecf5e86ab..d4d8e576af2 100644 --- a/services/venice-controller/src/test/java/com/linkedin/venice/controller/kafka/consumer/AdminExecutionTaskTest.java +++ b/services/venice-controller/src/test/java/com/linkedin/venice/controller/kafka/consumer/AdminExecutionTaskTest.java @@ -690,4 +690,78 @@ private AdminOperationWrapper createAddVersionWrapper( System.currentTimeMillis(), System.currentTimeMillis()); } + + @Test + public void testHandleMarkVersionRolledBackWithRegionFilter() { + when(mockAdmin.isLeaderControllerFor(clusterName)).thenReturn(true); + + Queue queue = new ConcurrentLinkedQueue<>(); + queue.add(createMarkVersionRolledBackWrapper(1L, 42, "prod-lva1,prod-ltx1")); + + AdminExecutionTask task = new AdminExecutionTask( + mockLogger, + clusterName, + storeName, + lastSucceededExecutionIdMap, + lastPersistedExecutionId, + queue, + mockAdmin, + mockExecutionIdAccessor, + isParentController, + mockStats, + regionName, + inflightThreadsByStore); + task.call(); + + verify(mockAdmin).markVersionRolledBack(eq(clusterName), eq(storeName), eq(42), eq("prod-lva1,prod-ltx1")); + } + + @Test + public void testHandleMarkVersionRolledBackWithNullRegionFilter() { + when(mockAdmin.isLeaderControllerFor(clusterName)).thenReturn(true); + + Queue queue = new ConcurrentLinkedQueue<>(); + queue.add(createMarkVersionRolledBackWrapper(1L, 7, null)); + + AdminExecutionTask task = new AdminExecutionTask( + mockLogger, + clusterName, + storeName, + lastSucceededExecutionIdMap, + lastPersistedExecutionId, + queue, + mockAdmin, + mockExecutionIdAccessor, + isParentController, + mockStats, + regionName, + inflightThreadsByStore); + task.call(); + + // A null regionsFilter must not NPE and must be passed through as null. + verify(mockAdmin).markVersionRolledBack(eq(clusterName), eq(storeName), eq(7), eq(null)); + } + + private AdminOperationWrapper createMarkVersionRolledBackWrapper(long executionId, int versionNum, String regionFilter) { + AdminOperation adminOperation = new AdminOperation(); + adminOperation.operationType = AdminMessageType.MARK_VERSION_ROLLED_BACK.getValue(); + adminOperation.executionId = executionId; + + com.linkedin.venice.controller.kafka.protocol.admin.MarkVersionRolledBack markVersionRolledBack = + new com.linkedin.venice.controller.kafka.protocol.admin.MarkVersionRolledBack(); + markVersionRolledBack.clusterName = clusterName; + markVersionRolledBack.storeName = storeName; + markVersionRolledBack.versionNum = versionNum; + markVersionRolledBack.regionsFilter = regionFilter; + adminOperation.payloadUnion = markVersionRolledBack; + + PubSubPosition position = InMemoryPubSubPosition.of(1L); + return new AdminOperationWrapper( + adminOperation, + position, + executionId, + System.currentTimeMillis(), + System.currentTimeMillis(), + System.currentTimeMillis()); + } } From 6de769664b32721462cbd2b45385d71729cab0a5 Mon Sep 17 00:00:00 2001 From: Kai-Sern Lim Date: Tue, 21 Jul 2026 22:35:35 -0700 Subject: [PATCH 2/5] Update admin message dimension mapping Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../controller/kafka/protocol/enums/AdminMessageTypeTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/services/venice-controller/src/test/java/com/linkedin/venice/controller/kafka/protocol/enums/AdminMessageTypeTest.java b/services/venice-controller/src/test/java/com/linkedin/venice/controller/kafka/protocol/enums/AdminMessageTypeTest.java index 8541af26fc6..0f1bb4f5045 100644 --- a/services/venice-controller/src/test/java/com/linkedin/venice/controller/kafka/protocol/enums/AdminMessageTypeTest.java +++ b/services/venice-controller/src/test/java/com/linkedin/venice/controller/kafka/protocol/enums/AdminMessageTypeTest.java @@ -46,6 +46,7 @@ public void testDimensionInterface() { .put(AdminMessageType.DELETE_UNUSED_VALUE_SCHEMA, "delete_unused_value_schema") .put(AdminMessageType.ROLLBACK_CURRENT_VERSION, "rollback_current_version") .put(AdminMessageType.ROLLFORWARD_CURRENT_VERSION, "rollforward_current_version") + .put(AdminMessageType.MARK_VERSION_ROLLED_BACK, "mark_version_rolled_back") .build(); new VeniceDimensionTestFixture<>( AdminMessageType.class, From b810be510c6878677289b9fde5fb76eb3d7bc87f Mon Sep 17 00:00:00 2001 From: Kai-Sern Lim Date: Wed, 22 Jul 2026 00:00:21 -0700 Subject: [PATCH 3/5] Remove MARK_VERSION_ROLLED_BACK admin operation Replace cross-region protocol message with direct deleteOldVersion calls per region via controllerClientMap. Non-current PUSHED/ONLINE child copies of a terminal deferred version are deleted immediately rather than transitioned through ROLLED_BACK; the double-guard in VeniceHelixAdmin and controllerClientMap ensures current versions are never touched. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../data-management/version-lifecycle.md | 26 +- .../avro/AvroProtocolDefinition.java | 2 +- .../DeferredVersionSwapService.java | 28 +- .../venice/controller/VeniceHelixAdmin.java | 70 - .../controller/VeniceParentHelixAdmin.java | 29 - .../kafka/consumer/AdminExecutionTask.java | 13 - .../protocol/enums/AdminMessageType.java | 6 +- .../AdminOperation/v102/AdminOperation.avsc | 1370 ----------------- ...rsionSwapServiceWithSequentialRollout.java | 95 +- .../controller/TestVeniceHelixAdmin.java | 59 +- .../consumer/AdminExecutionTaskTest.java | 74 - .../protocol/enums/AdminMessageTypeTest.java | 1 - 12 files changed, 73 insertions(+), 1700 deletions(-) delete mode 100644 services/venice-controller/src/main/resources/avro/AdminOperation/v102/AdminOperation.avsc diff --git a/docs/operations/data-management/version-lifecycle.md b/docs/operations/data-management/version-lifecycle.md index 2b53b293a66..0a3db0db84e 100644 --- a/docs/operations/data-management/version-lifecycle.md +++ b/docs/operations/data-management/version-lifecycle.md @@ -8,9 +8,9 @@ backup, within configured retention, or part of a store migration. ## Deletion decision table The count-based sweep is implemented by `Store.retrieveVersionsToDelete`. The time-based sweep is -implemented by `StoreBackupVersionCleanupService`. Deferred version swap terminal transitions first -reconcile bootstrap-complete, non-current child copies to `ROLLED_BACK`, so they cannot remain -invisible to both sweeps. +implemented by `StoreBackupVersionCleanupService`. Deferred version swap terminal transitions +directly delete bootstrap-complete, non-current child copies via `ControllerClient.deleteOldVersion`, +so they cannot remain invisible to both sweeps. | Initial status | Role / initial condition | Count retention | Time / safety gate | Trigger | Deletion decision | | --- | --- | --- | --- | --- | --- | @@ -26,7 +26,7 @@ invisible to both sweeps. | `PUSHED` | Non-current version without a terminal parent decision | Excluded because it may be an active deferred or concurrent swap candidate | Not eligible | Count or time cleanup | **KEEP** | | `PUSHED` or `ONLINE` | Still current in a child after parent `ERROR` or `ROLLED_BACK` | Protected while serving; reconciliation remains incomplete | Not eligible | Parent terminal reconciliation | **DEFER: keep and retry** | | `PUSHED` or `ONLINE` | Still current in a child after parent `PARTIALLY_ONLINE` | Serving the intentional partial state | Not eligible | Parent terminal reconciliation | **KEEP** | -| `PUSHED` or non-current `ONLINE` | Parent deferred swap becomes `ERROR`, `PARTIALLY_ONLINE`, or `ROLLED_BACK` | Removed from count sweep | Rolled-back retention gate applies | Parent terminal transition | **DEFER: mark `ROLLED_BACK`** | +| `PUSHED` or non-current `ONLINE` | Parent deferred swap becomes `ERROR`, `PARTIALLY_ONLINE`, or `ROLLED_BACK` | Removed from count sweep | None — immediately deleted | Parent terminal transition | **DELETE** | | `ONLINE` | Backup within the configured preserved count | Within limit | Not considered | Count sweep | **KEEP** | | `ONLINE` | Backup beyond the configured preserved count | Exceeds limit | Not considered | Count sweep | **DELETE** | | `ONLINE` | Backup considered by retention cleanup | Not considered | Before minimum retention | Time sweep | **DEFER** | @@ -39,15 +39,11 @@ invisible to both sweeps. Count-based and time-based cleanup are independent triggers. The first applicable trigger may delete an eligible backup, but neither trigger may delete the current version. `PUSHED` versions are never deleted from status and version number alone because multiple deferred or concurrent swaps can be -active. A terminal parent decision explicitly converts bootstrap-complete non-current child copies -to `ROLLED_BACK`. The controller scans every deferred terminal parent version, including versions -superseded by a newer push, and retries while a child is unreachable, missing the target metadata, -in progress, or unexpectedly still current after parent `ERROR` or `ROLLED_BACK`. - -When a child copy becomes `ROLLED_BACK`, the controller resets the store-level latest-promotion -timestamp to start the rollback retention window. A subsequent promotion can reset that shared clock -again, so a rolled-back version may be retained longer than the configured duration; it cannot be -deleted immediately because the current version was promoted long before the rollback. +active. A terminal parent decision explicitly deletes bootstrap-complete non-current child copies +via per-region `ControllerClient.deleteOldVersion`. The controller scans every deferred terminal +parent version, including versions superseded by a newer push, and retries while a child is +unreachable, missing the target metadata, in progress, or unexpectedly still current after parent +`ERROR` or `ROLLED_BACK`. ## State machine @@ -60,7 +56,7 @@ stateDiagram-v2 STARTED --> ERROR: push fails STARTED --> KILLED: push is killed PUSHED --> ONLINE: version swap succeeds - PUSHED --> ROLLED_BACK: deferred swap terminates while non-current + PUSHED --> DELETED: deferred swap terminates while non-current (parent terminal sweep) ONLINE --> ROLLED_BACK: rollback or abandoned non-current copy NOT_CREATED --> DELETED: stale below-current metadata after time gate @@ -81,7 +77,7 @@ stateDiagram-v2 Before terminal parent decision: KEEP Current after ERROR/ROLLED_BACK: retry Current after PARTIALLY_ONLINE: KEEP - Parent terminal + non-current: ROLLED_BACK + Parent terminal + non-current: DELETE immediately end note note right of ONLINE diff --git a/internal/venice-common/src/main/java/com/linkedin/venice/serialization/avro/AvroProtocolDefinition.java b/internal/venice-common/src/main/java/com/linkedin/venice/serialization/avro/AvroProtocolDefinition.java index e87516a046c..0ca65d946c2 100644 --- a/internal/venice-common/src/main/java/com/linkedin/venice/serialization/avro/AvroProtocolDefinition.java +++ b/internal/venice-common/src/main/java/com/linkedin/venice/serialization/avro/AvroProtocolDefinition.java @@ -71,7 +71,7 @@ public enum AvroProtocolDefinition { * * TODO: Move AdminOperation to venice-common module so that we can properly reference it here. */ - ADMIN_OPERATION(102, SpecificData.get().getSchema(ByteBuffer.class), "AdminOperation"), + ADMIN_OPERATION(101, SpecificData.get().getSchema(ByteBuffer.class), "AdminOperation"), /** * Single chunk of a large multi-chunk value. Just a bunch of bytes. diff --git a/services/venice-controller/src/main/java/com/linkedin/venice/controller/DeferredVersionSwapService.java b/services/venice-controller/src/main/java/com/linkedin/venice/controller/DeferredVersionSwapService.java index 8be2d3035ee..9b0d06c28cf 100644 --- a/services/venice-controller/src/main/java/com/linkedin/venice/controller/DeferredVersionSwapService.java +++ b/services/venice-controller/src/main/java/com/linkedin/venice/controller/DeferredVersionSwapService.java @@ -1418,14 +1418,26 @@ private boolean reconcileAbandonedVersionInChildRegions( } if (!regionsToReconcile.isEmpty()) { - String regionsFilter = RegionUtils.composeRegionList(regionsToReconcile); - veniceParentHelixAdmin.markVersionRolledBack(clusterName, storeName, targetVersionNum, regionsFilter); - LOGGER.info( - "Reconciled version: {} for store: {} as ROLLED_BACK in child regions: {} after parent status: {}", - targetVersionNum, - storeName, - regionsFilter, - parentStatus); + for (String region: regionsToReconcile) { + try { + controllerClientMap.get(region).deleteOldVersion(storeName, targetVersionNum); + LOGGER.info( + "Deleted abandoned deferred version {} for store {} in region {} (parent status: {})", + targetVersionNum, + storeName, + region, + parentStatus); + } catch (Exception e) { + LOGGER.warn( + "Failed to delete abandoned deferred version {} for store {} in region {} (parent status: {})", + targetVersionNum, + storeName, + region, + parentStatus, + e); + allRegionsTerminal = false; + } + } } return allRegionsTerminal; } diff --git a/services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceHelixAdmin.java b/services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceHelixAdmin.java index 160be02620b..ab7e4505dfb 100644 --- a/services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceHelixAdmin.java +++ b/services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceHelixAdmin.java @@ -5124,76 +5124,6 @@ public void rollbackToBackupVersion(String clusterName, String storeName, String } } - /** - * Mark {@code versionNumber} as {@link VersionStatus#ROLLED_BACK} in the regions selected by - * {@code regionFilter}, WITHOUT changing the store's current version. - * - *

Deferred swaps can end globally while a bootstrap-complete copy is still non-current in some - * regions. Marking that copy ROLLED_BACK makes it eligible for time-based cleanup. In-progress - * versions and the current serving version remain untouched. - */ - public void markVersionRolledBack(String clusterName, String storeName, int versionNumber, String regionFilter) { - String currentRegion = getRegionName(); - if (!StringUtils.isEmpty(regionFilter)) { - Set regionsFilter = parseRegionsFilterList(regionFilter); - if (!regionsFilter.contains(currentRegion)) { - LOGGER.info( - "markVersionRolledBack will be skipped for store: {} version: {} in cluster: {}, because the region filter" - + " is {} which doesn't include the current region: {}", - storeName, - versionNumber, - clusterName, - regionsFilter, - currentRegion); - return; - } - } - - storeMetadataUpdate(clusterName, storeName, (store, resources) -> { - if (!store.containsVersion(versionNumber)) { - LOGGER.info( - "markVersionRolledBack skipped: version {} not found in store {} in cluster {}", - versionNumber, - storeName, - clusterName); - return store; - } - // Never override the current version's status; that version serves reads. - if (store.getCurrentVersion() == versionNumber) { - LOGGER.warn( - "markVersionRolledBack skipped: version {} is the current version of store {} in cluster {}", - versionNumber, - storeName, - clusterName); - return store; - } - VersionStatus currentStatus = store.getVersionStatus(versionNumber); - if (VersionStatus.isVersionRolledBack(currentStatus) || VersionStatus.canDelete(currentStatus)) { - // Already ROLLED_BACK, or already in a terminal cleanable status (KILLED/ERROR) — nothing to do. - return store; - } - if (!VersionStatus.isBootstrapCompleted(currentStatus)) { - LOGGER.info( - "markVersionRolledBack skipped: version {} of store {} in cluster {} has not completed bootstrap (status {})", - versionNumber, - storeName, - clusterName, - currentStatus); - return store; - } - LOGGER.info( - "Marking version {} of store {} in cluster {} as ROLLED_BACK (was {})", - versionNumber, - storeName, - clusterName, - currentStatus); - store.updateVersionStatus(versionNumber, ROLLED_BACK); - store.setLatestVersionPromoteToCurrentTimestamp( - Math.max(store.getLatestVersionPromoteToCurrentTimestamp(), System.currentTimeMillis())); - return store; - }); - } - /** * Update the largest used version number of a specified store. */ diff --git a/services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceParentHelixAdmin.java b/services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceParentHelixAdmin.java index dbc7ac4f28c..83010075846 100644 --- a/services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceParentHelixAdmin.java +++ b/services/venice-controller/src/main/java/com/linkedin/venice/controller/VeniceParentHelixAdmin.java @@ -63,7 +63,6 @@ import com.linkedin.venice.controller.kafka.protocol.admin.PushStatusSystemStoreAutoCreationValidation; import com.linkedin.venice.controller.kafka.protocol.admin.ResumeStore; import com.linkedin.venice.controller.kafka.protocol.admin.RollbackCurrentVersion; -import com.linkedin.venice.controller.kafka.protocol.admin.MarkVersionRolledBack; import com.linkedin.venice.controller.kafka.protocol.admin.SchemaMeta; import com.linkedin.venice.controller.kafka.protocol.admin.SetStoreOwner; import com.linkedin.venice.controller.kafka.protocol.admin.SetStorePartitionCount; @@ -2520,34 +2519,6 @@ void checkNewPushCapacityFromChildren(String clusterName, String storeName) { } } - /** - * Mark {@code versionNum} as ROLLED_BACK in the child regions selected by {@code regionFilter}, - * WITHOUT changing the current version. Used to reconcile the non-target regions after a deferred - * version swap rollback: those regions bootstrapped the target version but never swapped to it, so - * their copy is stranded in PUSHED status above their unchanged current version and leaks disk. - * Marking it ROLLED_BACK makes it visible to {@code StoreBackupVersionCleanupService}. - */ - public void markVersionRolledBack(String clusterName, String storeName, int versionNum, String regionFilter) { - acquireAdminMessageLock(clusterName, storeName); - try { - getVeniceHelixAdmin().checkPreConditionForUpdateStoreMetadata(clusterName, storeName); - - MarkVersionRolledBack markVersionRolledBack = - (MarkVersionRolledBack) AdminMessageType.MARK_VERSION_ROLLED_BACK.getNewInstance(); - markVersionRolledBack.clusterName = clusterName; - markVersionRolledBack.storeName = storeName; - markVersionRolledBack.versionNum = versionNum; - markVersionRolledBack.regionsFilter = regionFilter; - AdminOperation message = new AdminOperation(); - message.operationType = AdminMessageType.MARK_VERSION_ROLLED_BACK.getValue(); - message.payloadUnion = markVersionRolledBack; - - sendAdminMessageAndWaitForConsumed(clusterName, storeName, message); - } finally { - releaseAdminMessageLock(clusterName, storeName); - } - } - private void updateParentVersionStatusAfterRollback( String clusterName, String storeName, diff --git a/services/venice-controller/src/main/java/com/linkedin/venice/controller/kafka/consumer/AdminExecutionTask.java b/services/venice-controller/src/main/java/com/linkedin/venice/controller/kafka/consumer/AdminExecutionTask.java index 2c9e281898f..add09bc2407 100644 --- a/services/venice-controller/src/main/java/com/linkedin/venice/controller/kafka/consumer/AdminExecutionTask.java +++ b/services/venice-controller/src/main/java/com/linkedin/venice/controller/kafka/consumer/AdminExecutionTask.java @@ -30,7 +30,6 @@ import com.linkedin.venice.controller.kafka.protocol.admin.ResumeStore; import com.linkedin.venice.controller.kafka.protocol.admin.RollForwardCurrentVersion; import com.linkedin.venice.controller.kafka.protocol.admin.RollbackCurrentVersion; -import com.linkedin.venice.controller.kafka.protocol.admin.MarkVersionRolledBack; import com.linkedin.venice.controller.kafka.protocol.admin.SetStoreCurrentVersion; import com.linkedin.venice.controller.kafka.protocol.admin.SetStoreOwner; import com.linkedin.venice.controller.kafka.protocol.admin.SetStorePartitionCount; @@ -345,9 +344,6 @@ private void processMessage(AdminOperation adminOperation) { case ROLLFORWARD_CURRENT_VERSION: handleRollForwardToFutureVersion((RollForwardCurrentVersion) adminOperation.payloadUnion); break; - case MARK_VERSION_ROLLED_BACK: - handleMarkVersionRolledBack((MarkVersionRolledBack) adminOperation.payloadUnion); - break; default: throw new VeniceException("Unknown admin operation type: " + adminOperation.operationType); } @@ -939,15 +935,6 @@ private void handleRollbackCurrentVersion(RollbackCurrentVersion message) { admin.rollbackToBackupVersion(clusterName, storeName, regionFilter); } - private void handleMarkVersionRolledBack(MarkVersionRolledBack message) { - String clusterName = message.getClusterName().toString(); - String storeName = message.getStoreName().toString(); - int versionNum = message.getVersionNum(); - CharSequence regionsFilter = message.getRegionsFilter(); - String regionFilter = regionsFilter == null ? null : regionsFilter.toString(); - admin.markVersionRolledBack(clusterName, storeName, versionNum, regionFilter); - } - private void handleDeleteUnusedValueSchema(DeleteUnusedValueSchemas message) { String clusterName = message.getClusterName().toString(); String storeName = message.getStoreName().toString(); diff --git a/services/venice-controller/src/main/java/com/linkedin/venice/controller/kafka/protocol/enums/AdminMessageType.java b/services/venice-controller/src/main/java/com/linkedin/venice/controller/kafka/protocol/enums/AdminMessageType.java index 00d51c5554c..9ec42b29c3b 100644 --- a/services/venice-controller/src/main/java/com/linkedin/venice/controller/kafka/protocol/enums/AdminMessageType.java +++ b/services/venice-controller/src/main/java/com/linkedin/venice/controller/kafka/protocol/enums/AdminMessageType.java @@ -15,7 +15,6 @@ import com.linkedin.venice.controller.kafka.protocol.admin.DisableStoreRead; import com.linkedin.venice.controller.kafka.protocol.admin.EnableStoreRead; import com.linkedin.venice.controller.kafka.protocol.admin.KillOfflinePushJob; -import com.linkedin.venice.controller.kafka.protocol.admin.MarkVersionRolledBack; import com.linkedin.venice.controller.kafka.protocol.admin.MetaSystemStoreAutoCreationValidation; import com.linkedin.venice.controller.kafka.protocol.admin.MetadataSchemaCreation; import com.linkedin.venice.controller.kafka.protocol.admin.MigrateStore; @@ -66,8 +65,7 @@ public enum AdminMessageType implements VeniceDimensionInterface { CONFIGURE_INCREMENTAL_PUSH_FOR_CLUSTER(22, true), META_SYSTEM_STORE_AUTO_CREATION_VALIDATION(23, false), PUSH_STATUS_SYSTEM_STORE_AUTO_CREATION_VALIDATION(24, false), CREATE_STORAGE_PERSONA(25, false), DELETE_STORAGE_PERSONA(26, false), UPDATE_STORAGE_PERSONA(27, false), DELETE_UNUSED_VALUE_SCHEMA(28, false), - ROLLBACK_CURRENT_VERSION(29, false), ROLLFORWARD_CURRENT_VERSION(30, false), - MARK_VERSION_ROLLED_BACK(31, false); + ROLLBACK_CURRENT_VERSION(29, false), ROLLFORWARD_CURRENT_VERSION(30, false); private final int value; private final boolean batchUpdate; @@ -140,8 +138,6 @@ public Object getNewInstance() { return new RollForwardCurrentVersion(); case ROLLBACK_CURRENT_VERSION: return new RollbackCurrentVersion(); - case MARK_VERSION_ROLLED_BACK: - return new MarkVersionRolledBack(); default: throw new VeniceException("Unsupported " + getClass().getSimpleName() + " value: " + value); } diff --git a/services/venice-controller/src/main/resources/avro/AdminOperation/v102/AdminOperation.avsc b/services/venice-controller/src/main/resources/avro/AdminOperation/v102/AdminOperation.avsc deleted file mode 100644 index 051df79d210..00000000000 --- a/services/venice-controller/src/main/resources/avro/AdminOperation/v102/AdminOperation.avsc +++ /dev/null @@ -1,1370 +0,0 @@ -{ - "name": "AdminOperation", - "namespace": "com.linkedin.venice.controller.kafka.protocol.admin", - "type": "record", - "fields": [ - { - "name": "operationType", - "doc": "0 => StoreCreation, 1 => ValueSchemaCreation, 2 => PauseStore, 3 => ResumeStore, 4 => KillOfflinePushJob, 5 => DisableStoreRead, 6 => EnableStoreRead, 7=> DeleteAllVersions, 8=> SetStoreOwner, 9=> SetStorePartitionCount, 10=> SetStoreCurrentVersion, 11=> UpdateStore, 12=> DeleteStore, 13=> DeleteOldVersion, 14=> MigrateStore, 15=> AbortMigration, 16=>AddVersion, 17=> DerivedSchemaCreation, 18=>SupersetSchemaCreation, 19=>EnableNativeReplicationForCluster, 20=>MetadataSchemaCreation, 21=>EnableActiveActiveReplicationForCluster, 25=>CreatePersona, 26=>DeletePersona, 27=>UpdatePersona, 28=>RollbackCurrentVersion, 29=>RollforwardCurrentVersion, 31=>MarkVersionRolledBack", - "type": "int" - }, { - "name": "executionId", - "doc": "ID of a command execution which is used to query the status of this command.", - "type": "long", - "default": 0 - }, { - "name": "payloadUnion", - "doc": "This contains the main payload of the admin operation", - "type": [ - { - "name": "StoreCreation", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - }, - { - "name": "owner", - "type": "string" - }, - { - "name": "keySchema", - "type": { - "type": "record", - "name": "SchemaMeta", - "fields": [ - {"name": "schemaType", "type": "int", "doc": "0 => Avro-1.4, and we can add more if necessary"}, - {"name": "definition", "type": "string"} - ] - } - }, - { - "name": "valueSchema", - "type": "SchemaMeta" - } - ] - }, - { - "name": "ValueSchemaCreation", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - }, - { - "name": "schema", - "type": "SchemaMeta" - }, - { - "name": "schemaId", - "type": "int" - }, - { - "name": "doUpdateSupersetSchemaID", - "type": "boolean", - "doc": "Whether this superset schema ID should be updated to be the value schema ID for this store.", - "default": false - } - ] - }, - { - "name": "PauseStore", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - } - ] - }, - { - "name": "ResumeStore", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - } - ] - }, - { - "name": "KillOfflinePushJob", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "kafkaTopic", - "type": "string" - } - ] - }, - { - "name": "DisableStoreRead", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - } - ] - }, - { - "name": "EnableStoreRead", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - } - ] - }, - { - "name": "DeleteAllVersions", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - } - ] - }, - { - "name": "SetStoreOwner", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - }, - { - "name": "owner", - "type": "string" - } - ] - }, - { - "name": "SetStorePartitionCount", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - }, - { - "name": "partitionNum", - "type": "int" - } - ] - }, - { - "name": "SetStoreCurrentVersion", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - }, - { - "name": "currentVersion", - "type": "int" - } - ] - }, - { - "name": "UpdateStore", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - }, - { - "name": "owner", - "type": "string" - }, - { - "name": "partitionNum", - "type": "int" - }, - { - "name": "currentVersion", - "type": "int" - }, - { - "name": "enableReads", - "type": "boolean" - }, - { - "name": "enableWrites", - "type": "boolean" - }, - { - "name": "storageQuotaInByte", - "type": "long", - "default": 21474836480 - }, - { - "name": "readQuotaInCU", - "type": "long", - "default": 1800 - }, - { - "name": "hybridStoreConfig", - "type": [ - "null", - { - "name": "HybridStoreConfigRecord", - "type": "record", - "fields": [ - { - "name": "rewindTimeInSeconds", - "type": "long" - }, - { - "name": "offsetLagThresholdToGoOnline", - "type": "long" - }, - { - "name": "producerTimestampLagThresholdToGoOnlineInSeconds", - "type": "long", - "default": -1 - }, - { - "name": "dataReplicationPolicy", - "doc": "Real-time Samza job data replication policy. Using int because Avro Enums are not evolvable 0 => NON_AGGREGATE, 1 => AGGREGATE, 2 => NONE, 3 => ACTIVE_ACTIVE", - "type": "int", - "default": 0 - }, - { - "name": "bufferReplayPolicy", - "type": "int", - "doc": "Policy that will be used during buffer replay. rewindTimeInSeconds defines the delta. 0 => REWIND_FROM_EOP (replay from 'EOP - rewindTimeInSeconds'), 1 => REWIND_FROM_SOP (replay from 'SOP - rewindTimeInSeconds')", - "default": 0 - }, - {"name": "realTimeTopicName", "type": "string", "default": "", "doc": "Name of the real time topic this store/version uses"} - ] - } - ], - "default": null - }, - { - "name": "accessControlled", - "type": "boolean", - "default": false - }, - { - "name": "compressionStrategy", - "doc": "Using int because Avro Enums are not evolvable", - "type": "int", - "default": 0 - }, - { - "name": "chunkingEnabled", - "type": "boolean", - "default": false - }, - { - "name": "rmdChunkingEnabled", - "type": "boolean", - "default": false - }, - { - "name": "singleGetRouterCacheEnabled", - "aliases": ["routerCacheEnabled"], - "type": "boolean", - "default": false - }, - { - "name": "batchGetRouterCacheEnabled", - "type": "boolean", - "default": false - }, - { - "name": "batchGetLimit", - "doc": "The max key number allowed in batch get request, and Venice will use cluster-level config if the limit (not positive) is not valid", - "type": "int", - "default": -1 - }, - { - "name": "numVersionsToPreserve", - "doc": "The max number of versions the store should preserve. Venice will use cluster-level config if the number is 0 here.", - "type": "int", - "default": 0 - }, - { - "name": "incrementalPushEnabled", - "doc": "a flag to see if the store supports incremental push or not", - "type": "boolean", - "default": false - }, - { - "name": "separateRealTimeTopicEnabled", - "doc": "Flag to see if the store supports separate real-time topic for incremental push.", - "type": "boolean", - "default": false - }, - { - "name": "isMigrating", - "doc": "Whether or not the store is in the process of migration", - "type": "boolean", - "default": false - }, - { - "name": "writeComputationEnabled", - "doc": "Whether write-path computation feature is enabled for this store", - "type": "boolean", - "default": false - }, - { - "name": "replicationMetadataVersionID", - "doc": "RMD (Replication metadata) version ID on the store-level. Default -1 means NOT_SET and the cluster-level RMD version ID should be used for stores.", - "type": "int", - "default": -1 - }, - { - "name": "readComputationEnabled", - "doc": "Whether read-path computation feature is enabled for this store", - "type": "boolean", - "default": false - }, - { - "name": "bootstrapToOnlineTimeoutInHours", - "doc": "Maximum number of hours allowed for the store to transition from bootstrap to online state", - "type": "int", - "default": 24 - }, - { - "name": "leaderFollowerModelEnabled", - "doc": "Whether or not to use leader follower state transition model for upcoming version", - "type": "boolean", - "default": false - }, - { - "name": "backupStrategy", - "doc": "Strategies to store backup versions.", - "type": "int", - "default": 0 - }, - { - "name": "clientDecompressionEnabled", - "type": "boolean", - "default": true - }, - { - "name": "schemaAutoRegisterFromPushJobEnabled", - "type": "boolean", - "default": false - }, - { - "name": "hybridStoreOverheadBypass", - "type": "boolean", - "default": false - }, - { - "name": "hybridStoreDiskQuotaEnabled", - "doc": "Whether or not to enable disk storage quota for a hybrid store", - "type": "boolean", - "default": false - }, - { - "name": "ETLStoreConfig", - "type": [ - "null", - { - "name": "ETLStoreConfigRecord", - "type": "record", - "fields": [ - { - "name": "etledUserProxyAccount", - "type": ["null", "string"] - }, - { - "name": "regularVersionETLEnabled", - "type": "boolean" - }, - { - "name": "futureVersionETLEnabled", - "type": "boolean" - }, - { - "name": "etlStrategy", - "type": "int", - "default": 1 - }, - { - "name": "etlActiveFabrics", - "type": ["null", {"type": "array", "items": "string"}], - "default": null, - "doc": "Allowlist of fabric names where ETL onboard/offboard fires. null = fire in every fabric (default behavior). When set, only listed fabrics' child controllers trigger the ExternalETLService." - } - ] - } - ], - "default": null - }, - { - "name": "partitionerConfig", - "type": [ - "null", - { - "name": "PartitionerConfigRecord", - "type": "record", - "fields": [ - { - "name": "partitionerClass", - "type": "string" - }, - { - "name": "partitionerParams", - "type": { - "type": "map", - "values": "string" - } - }, - { - "name": "amplificationFactor", - "type": "int" - } - ] - } - ], - "default": null - }, - { - "name": "nativeReplicationEnabled", - "type": "boolean", - "default": false - }, - { - "name": "pushStreamSourceAddress", - "type": ["null", "string"], - "default": null - }, - { - "name": "largestUsedVersionNumber", - "type": ["null", "int"], - "default": null - }, - { - "name": "largestUsedRTVersionNumber", - "type": ["null", "int"], - "doc": "Largest used RT version number used by this store. This is used to create real time topic name while creating a new store-version", - "default": null - }, - { - "name": "incrementalPushPolicy", - "doc": "Incremental Push Policy to reconcile with real time pushes. Using int because Avro Enums are not evolvable 0 => PUSH_TO_VERSION_TOPIC, 1 => INCREMENTAL_PUSH_SAME_AS_REAL_TIME", - "type": "int", - "default": 0 - }, - { - "name": "backupVersionRetentionMs", - "type": "long", - "doc": "Backup version retention time after a new version is promoted to the current version, if not specified, Venice will use the configured retention as the default policy", - "default": -1 - }, - { - "name": "replicationFactor", - "doc": "number of replica each store version will have", - "type": "int", - "default": 3 - }, - { - "name": "migrationDuplicateStore", - "doc": "Whether or not the store is a duplicate store in the process of migration", - "type": "boolean", - "default": false - }, - { - "name": "nativeReplicationSourceFabric", - "doc": "The source fabric to be used when the store is running in Native Replication mode.", - "type": ["null", "string"], - "default": null - }, - { - "name": "activeActiveReplicationEnabled", - "doc": "A command option to enable/disable Active/Active replication feature for a store", - "type": "boolean", - "default": false - }, - { - "name": "disableMetaStore", - "doc": "An UpdateStore command option to disable the companion meta system store", - "type": "boolean", - "default": false - }, - { - "name": "disableDavinciPushStatusStore", - "doc": "An UpdateStore command option to disable the companion davinci push status store", - "type": "boolean", - "default": false - }, - { - "name": "applyTargetVersionFilterForIncPush", - "doc": "An UpdateStore command option to enable/disable applying the target version filter for incremental pushes", - "type": "boolean", - "default": false - }, - { - "name": "updatedConfigsList", - "doc": "The list that contains all updated configs by the UpdateStore command. Most of the fields in UpdateStore are not optional, and changing those fields to Optional (Union) is not a backward compatible change, so we have to add an addition array field to record all updated configs in parent controller.", - "type": { - "type": "array", - "items": "string" - }, - "default": [] - }, - { - "name": "replicateAllConfigs", - "doc": "A flag to indicate whether all store configs in parent cluster will be replicated to child clusters; true by default, so that existing UpdateStore messages in Admin topic will behave the same as before.", - "type": "boolean", - "default": true - }, - { - "name": "regionsFilter", - "doc": "A list of regions that will be impacted by the UpdateStore command", - "type": ["null", "string"], - "default": null - }, - { - "name": "storagePersona", - "doc": "The name of the StoragePersona to add to the store", - "type": ["null", "string"], - "default": null - }, - { - "name": "views", - "doc": "A map of views which describe and configure a downstream view of a venice store. Keys in this map are for convenience of managing configs.", - "type": ["null", - { - "type":"map", - "java-key-class": "java.lang.String", - "avro.java.string": "String", - "values": { - "name": "StoreViewConfigRecord", - "type": "record", - "doc": "A configuration for a particular view. This config should inform Venice leaders how to transform and transmit data to destination views.", - "fields": [ - { - "name": "viewClassName", - "type": "string", - "doc": "This informs what kind of view we are materializing. This then informs what kind of parameters are passed to parse this input. This is expected to be a fully formed class path name for materialization.", - "default": "" - }, - { - "name": "viewParameters", - "doc": "Optional parameters to be passed to the given view config.", - "type": ["null", - { - "type": "map", - "java-key-class": "java.lang.String", - "avro.java.string": "String", - "values": { "type": "string", "avro.java.string": "String" } - } - ], - "default": null - } - ] - } - }], - "default": null - }, - { - "name": "latestSuperSetValueSchemaId", - "doc": "The schema id for the latest superset schema", - "type" : "int", - "default": -1 - }, - { - "name": "storageNodeReadQuotaEnabled", - "doc": "Whether storage node read quota is enabled for this store", - "type": "boolean", - "default": false - }, - { - "name": "compactionEnabled", - "doc": "Whether compaction is enabled for this store", - "type": "boolean", - "default": true - }, - { - "name": "compactionThresholdMilliseconds", - "doc": "Store-level compaction threshold in milliseconds", - "type": "long", - "default": -1 - }, - { - "name": "encryptionEnabled", - "doc": "Whether encryption is enabled for this store", - "type": "boolean", - "default": false - }, - { - "name": "minCompactionLagSeconds", - "doc": "Store-level version topic min compaction lag", - "type": "long", - "default": -1 - }, - { - "name": "maxCompactionLagSeconds", - "doc": "Store-level version topic max compaction lag", - "type": "long", - "default": -1 - }, - { - "name": "maxRecordSizeBytes", - "doc": "Store-level maximum size of any record in bytes for batch push jobs", - "type": "int", - "default": -1 - }, - { - "name": "maxNearlineRecordSizeBytes", - "doc": "Store-level maximum size of any record in bytes for nearline jobs with partial updates", - "type": "int", - "default": -1 - }, - { - "name": "unusedSchemaDeletionEnabled", - "doc": "Whether unused schema deletion is enabled or not.", - "type": "boolean", - "default": false - }, - { - "name": "blobTransferEnabled", - "doc": "Flag to indicate if the blob transfer is allowed or not", - "type": "boolean", - "default": false - }, - { - "name": "nearlineProducerCompressionEnabled", - "doc": "Flag to control whether the producer in Server for nearline workload will enable compression or not", - "type": "boolean", - "default": true - }, - { - "name": "nearlineProducerCountPerWriter", - "doc": "How many producers will be used for the nearline producer in Server to improve producing throughput", - "type": "int", - "default": 1 - }, - { - "name": "targetSwapRegion", - "doc": "Controls what region to swap in the current version during target colo push", - "type": ["null","string"], - "default": null - }, - { - "name": "targetSwapRegionWaitTime", - "doc": "Controls how long to wait in minutes before swapping the version on the regions", - "type": "int", - "default": 60 - }, - { - "name": "isDaVinciHeartBeatReported", - "doc": "Flag to indicate whether DVC is bootstrapping and sending heartbeats", - "type": "boolean", - "default": false - }, - { - "name": "globalRtDivEnabled", - "doc": "Flag to indicate whether the Global RT DIV feature is on. The DIV will be centralized in the ConsumptionTask, and leaders will periodically replicate the RT DIV to followers via VT.", - "type": "boolean", - "default": false - }, - { - "name": "enumSchemaEvolutionAllowed", - "doc": "Flag to control whether a certain store is allowed to evolve enum schema since the readers need to use Avro-1.9+", - "type": "boolean", - "default": false - }, - { - "name": "storeLifecycleHooks", - "doc": "List of store lifecycle hooks", - "type": { - "type": "array", - "items": { - "name": "StoreLifecycleHooksRecord", - "type": "record", - "fields": [ - {"name": "storeLifecycleHooksClassName", "type": "string", "doc": "FQCN of the hook implementation"}, - {"name": "storeLifecycleHooksParams", "type": {"type": "map", "values": "string"}, "doc": "Bag of properties to pass into the hook implementation"} - ] - } - }, - "default": [] - }, - { - "name": "blobTransferInServerEnabled", - "doc": "Flag to indicate if the blob transfer is allowed or not in server. Values can be 'NOT_SPECIFIED' as default, 'ENABLED', or 'DISABLED'.", - "type": "string", - "default": "NOT_SPECIFIED" - }, - { - "name": "keyUrnCompressionEnabled", - "doc": "Whether key URN compression is enabled for this store", - "type": "boolean", - "default": false - }, - { - "name": "keyUrnFields", - "doc": "List of fields in the key schema that will be eligible for key urn compression", - "type": { - "type": "array", - "items": "string" - }, - "default": [] - }, - { - "name": "flinkVeniceViewsEnabled", - "doc": "Whether this store is enabled to use Flink-based Venice Views", - "type": "boolean", - "default": false - }, - { - "name": "throughputQuotaInBytes", - "doc": "The maximum throughput measured in bytes that clients can produce into one store. Default -1 means no limit.", - "type": "long", - "default": -1 - }, - { - "name": "throughputQuotaInRecords", - "doc": "The maximum throughput measured in records that clients can produce into one store. Default -1 means no limit.", - "type": "long", - "default": -1 - }, - { - "name": "blobDbEnabled", - "doc": "Flag to indicate if the RocksDB BlobDB feature is enabled or not. Values can be 'NOT_SPECIFIED' (default, follows cluster level config), 'ENABLED', or 'DISABLED'.", - "type": "string", - "default": "NOT_SPECIFIED" - }, - { - "name": "previousCurrentVersion", - "doc": "Int representing the previous current version before the current version was marked current", - "type": "int", - "default": -1 - }, - { - "name": "transientRecordCacheEnabled", - "doc": "Whether the bounded hot transient record cache is enabled for this store to retain frequently accessed large records across consumer poll boundaries.", - "type": "boolean", - "default": false - }, - { - "name": "mergedValueRmdColumnFamilyEnabled", - "doc": "Whether to store value and RMD in a single column family to reduce read amplification during A/A ingestion.", - "type": "boolean", - "default": false - }, - { - "name": "ingestionPauseMode", - "doc": "Ingestion pause mode. 0 => NOT_PAUSED, 1 => CURRENT_VERSION, 2 => ALL_VERSIONS", - "type": "int", - "default": 0 - }, - { - "name": "ingestionPausedRegions", - "doc": "List of regions where pause is applied. Empty list means all regions.", - "type": {"type": "array", "items": "string"}, - "default": [] - }, - { - "name": "targetRegionPromoted", - "doc": "Flag set by the parent controller when the target region has promoted the future version to current. Propagated to child controllers so DaVinci clients can resume paused ingestion.", - "type": "boolean", - "default": false - }, - { - "name": "storageMode", - "doc": "Store-level default storage mode. The controller persists this value on the store record (StoreProperties.storageMode) and also copies it into StoreVersion.storageMode when a new store version is created; existing versions are unaffected. Controls where version data is persisted in addition to (or instead of) Venice local storage. 0 => INTERNAL (default, Venice-only); 1 => DUAL_WRITE (data is written to both Venice local storage and the configured external storage; the specific dual-write implementation -- leader-consumer-pipeline vs Venice Push Job -- is selected by separate server/VPJ configuration); 2 => EXTERNAL (external-storage-only; Venice's data partition becomes NoOp while metadata partitions are still persisted locally for checkpointing).", - "type": "int", - "default": 0 - }, - { - "name": "externalStorageReadMode", - "doc": "Store-level read routing applied to the store as a whole (not per-version). Controls how clients route reads between Venice local storage and the configured external storage. 0 => VENICE_ONLY (default, reads served from Venice local storage only -- current behavior); 1 => DUAL_MODE_CONSISTENCY_CHECK (the client reads from both Venice and the external storage and verifies/reports divergence; used to validate dual-write correctness before cutover); 2 => DUAL_MODE_EARLY_RETURN (the client issues reads against both Venice and the external storage in parallel and returns the first response, falling back to the slower one on miss); 3 => EXTERNAL_ONLY (reads served from the external storage only, with Venice acting as the metadata/CDC path).", - "type": "int", - "default": 0 - } - ] - }, - { - "name": "DeleteStore", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - }, - { - "name": "largestUsedVersionNumber", - "type": "int" - } - ] - }, - { - "name": "DeleteOldVersion", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - }, - { - "name": "versionNum", - "type": "int" - } - ] - }, - { - "name": "MigrateStore", - "type": "record", - "fields": [ - { - "name": "srcClusterName", - "type": "string" - }, - { - "name": "destClusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - } - ] - }, - { - "name": "AbortMigration", - "type": "record", - "fields": [ - { - "name": "srcClusterName", - "type": "string" - }, - { - "name": "destClusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - } - ] - }, - { - "name": "AddVersion", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - }, - { - "name": "pushJobId", - "type": "string" - }, - { - "name": "versionNum", - "type": "int" - }, - { - "name": "numberOfPartitions", - "type": "int" - }, - { - "name": "pushType", - "doc": "The push type of the new version, 0 => BATCH, 1 => STREAM_REPROCESSING. Previous add version messages will default to BATCH and this is a safe because they were created when BATCH was the only version type", - "type": "int", - "default": 0 - }, - { - "name": "pushStreamSourceAddress", - "type": ["null", "string"], - "default": null - }, - { - "name": "rewindTimeInSecondsOverride", - "doc": "The overridable rewind time config for this specific version of a hybrid store, and if it is not specified, the new version will use the store-level rewind time config", - "type": "long", - "default": -1 - }, - { - "name": "timestampMetadataVersionId", - "doc": "The A/A metadata schema version ID that will be used to deserialize metadataPayload.", - "type": "int", - "default": -1 - }, - { - "name": "versionSwapDeferred", - "doc": "Indicates if swapping this version to current version after push completion should be initiated or not", - "type": "boolean", - "default": false - }, - { - "name": "targetedRegions", - "doc": "The list of regions that is separated by comma for targeted region push. If set, this admin message should only be consumed by the targeted regions", - "type": [ - "null", - { - "type": "array", - "items": "string" - } - ], - "default": null - }, - { - "name": "repushSourceVersion", - "doc": "Indicates the source version from which a repush version is created", - "type": "int", - "default": -1 - }, - { - "name": "currentRTVersionNumber", - "type": "int", - "doc": "current RT version number that should be used to formulate real time topic name during add version", - "default": 0 - }, - { - "name": "repushTtlSeconds", - "type": "int", - "doc": "For store version created from repush, indicates the time-to-live in seconds set during the repush", - "default": -1 - }, - { - "name": "degradedDatacenters", - "doc": "List of datacenter names marked as degraded at the time of version creation. Child controllers use this in AdminExecutionTask to enforce skipConsumption for degraded DCs even when versionSwapDeferred=true, preventing ghost versions in degraded DCs.", - "type": [ - "null", - { - "type": "array", - "items": "string" - } - ], - "default": null - } - ] - }, - { - "name": "DerivedSchemaCreation", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - }, - { - "name": "schema", - "type": "SchemaMeta" - }, - { - "name": "valueSchemaId", - "type": "int" - }, - { - "name": "derivedSchemaId", - "type": "int" - } - ] - }, - { - "name": "SupersetSchemaCreation", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - }, - { - "name": "valueSchema", - "type": "SchemaMeta" - }, - { - "name": "valueSchemaId", - "type": "int" - }, - { - "name": "supersetSchema", - "type": "SchemaMeta" - }, - { - "name": "supersetSchemaId", - "type": "int" - } - ] - }, - { - "name": "ConfigureNativeReplicationForCluster", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeType", - "type": "string" - }, - { - "name": "enabled", - "type": "boolean" - }, - { - "name": "nativeReplicationSourceRegion", - "doc": "The source region to be used when the store is running in Native Replication mode.", - "type": ["null", "string"], - "default": null - }, - { - "name": "regionsFilter", - "type": ["null", "string"], - "default": null - } - ] - }, - { - "name": "MetadataSchemaCreation", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - }, - { - "name": "valueSchemaId", - "type": "int" - }, - { - "name": "metadataSchema", - "type": "SchemaMeta" - }, - { - "name": "timestampMetadataVersionId", - "type": "int", - "aliases": ["metadataVersionId"], - "default": -1 - } - ] - }, - { - "name": "ConfigureActiveActiveReplicationForCluster", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeType", - "type": "string" - }, - { - "name": "enabled", - "type": "boolean" - }, - { - "name": "regionsFilter", - "type": ["null", "string"], - "default": null - } - ] - }, { - "name": "ConfigureIncrementalPushForCluster", - "doc": "A command to migrate all incremental push stores in a cluster to a specific incremental push policy.", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "incrementalPushPolicyToFilter", - "doc": "If this batch update command is trying to configure existing incremental push store type, their incremental push policy should also match this filter before the batch update command applies any change to them. Default value is -1, meaning there is no filter.", - "type": "int", - "default": -1 - }, - { - "name": "incrementalPushPolicyToApply", - "doc": "This field will determine what incremental push policy will be applied to the selected stores. Default value is 1, which is the INCREMENTAL_PUSH_SAME_AS_REAL_TIME policy", - "type": "int", - "default": 1 - }, - { - "name": "regionsFilter", - "type": ["null", "string"], - "default": null - } - ] - }, { - "name": "MetaSystemStoreAutoCreationValidation", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - } - ] - }, { - "name": "PushStatusSystemStoreAutoCreationValidation", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - } - ] - }, { - "name": "CreateStoragePersona", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "name", - "type": "string" - }, - { - "name": "quotaNumber", - "type": "long" - }, - { - "name": "storesToEnforce", - "type": { - "type": "array", - "items": "string", - "default": [] - } - }, - { - "name": "owners", - "type": { - "type": "array", - "items": "string", - "default": [] - } - } - ] - }, { - "name": "DeleteStoragePersona", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "name", - "type": "string" - } - ] - }, { - "name": "UpdateStoragePersona", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, { - "name": "name", - "type": "string" - }, { - "name": "quotaNumber", - "type": ["null","long"], - "default": null - }, { - "name": "storesToEnforce", - "type": [ - "null", - { - "type": "array", - "items": "string" - } - ], - "default": null - }, { - "name": "owners", - "type": [ - "null", - { - "type": "array", - "items": "string" - } - ], - "default": null - } - ] - }, - { - "name": "DeleteUnusedValueSchemas", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - }, - { - "name": "schemaIds", - "type": { - "type": "array", - "items": "int", - "default": [] - } - } - ] - }, - { - "name": "RollbackCurrentVersion", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - }, - { - "name": "regionsFilter", - "doc": "A list of regions that will be impacted by the RollbackCurrentVersion command", - "type": ["null", "string"], - "default": null - } - ] - }, - { - "name": "RollForwardCurrentVersion", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - }, - { - "name": "regionsFilter", - "doc": "A list of regions that will be impacted by the RollForwardCurrentVersion command", - "type": ["null", "string"], - "default": null - } - ] - }, - { - "name": "MarkVersionRolledBack", - "type": "record", - "fields": [ - { - "name": "clusterName", - "type": "string" - }, - { - "name": "storeName", - "type": "string" - }, - { - "name": "versionNum", - "doc": "The version number to mark as ROLLED_BACK, without changing the current version.", - "type": "int" - }, - { - "name": "regionsFilter", - "doc": "A list of regions that will be impacted by the MarkVersionRolledBack command", - "type": ["null", "string"], - "default": null - } - ] - } - ] - } - ] -} diff --git a/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestDeferredVersionSwapServiceWithSequentialRollout.java b/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestDeferredVersionSwapServiceWithSequentialRollout.java index 7ee316f554a..115da274676 100644 --- a/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestDeferredVersionSwapServiceWithSequentialRollout.java +++ b/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestDeferredVersionSwapServiceWithSequentialRollout.java @@ -52,7 +52,6 @@ import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; -import org.mockito.ArgumentCaptor; import org.mockito.InOrder; import org.testng.Assert; import org.testng.annotations.BeforeMethod; @@ -445,7 +444,9 @@ public void testSequentialRolloutFailurePath() throws Exception { TestUtils.waitForNonDeterministicAssertion(5, TimeUnit.SECONDS, () -> { // Verify error recording was called due to the failure verify(store, atLeastOnce()).updateVersionStatus(2, VersionStatus.PARTIALLY_ONLINE); - verify(admin, atLeastOnce()).markVersionRolledBack(eq(clusterName), eq(storeName), eq(versionTwo), anyString()); + Map childControllers = veniceHelixAdmin.getControllerClientMap(clusterName); + verify(childControllers.get(region2), atLeastOnce()).deleteOldVersion(storeName, versionTwo); + verify(childControllers.get(region3), atLeastOnce()).deleteOldVersion(storeName, versionTwo); verify(admin, never()).rollForwardToFutureVersion(clusterName, storeName, region3); verify(admin, never()).truncateKafkaTopic(anyString()); }); @@ -579,19 +580,16 @@ public void testSequentialRolloutPostSwapValidationRollbackMarksNonTargetRegions new DeferredVersionSwapService(admin, veniceControllerMultiClusterConfig, stats, metricsRepository); deferredVersionSwapService.startInner(); - ArgumentCaptor regionFilterCaptor = ArgumentCaptor.forClass(String.class); TestUtils.waitForNonDeterministicAssertion(5, TimeUnit.SECONDS, () -> { // Target region (region1, the prior rolled-forward region) is rolled back. verify(admin, atLeastOnce()).rollbackToBackupVersion(clusterName, storeName, region1); - // Non-current child regions reconcile the abandoned version. - verify(admin, atLeastOnce()) - .markVersionRolledBack(eq(clusterName), eq(storeName), eq(versionTwo), regionFilterCaptor.capture()); + // Non-current child regions have their orphaned version deleted. + Map childControllers = veniceHelixAdmin.getControllerClientMap(clusterName); + verify(childControllers.get(region2), atLeastOnce()).deleteOldVersion(storeName, versionTwo); + verify(childControllers.get(region3), atLeastOnce()).deleteOldVersion(storeName, versionTwo); // No roll forward should happen for region2 after a ROLLBACK. verify(admin, never()).rollForwardToFutureVersion(clusterName, storeName, region2); }); - - Set reconciledRegions = new HashSet<>(Arrays.asList(regionFilterCaptor.getValue().split(","))); - Assert.assertEquals(reconciledRegions, new HashSet<>(Arrays.asList(region2, region3))); } @DataProvider(name = "abandonedParentStatuses") @@ -614,14 +612,12 @@ public void testAbandonedParentStatusReconcilesEligibleChildren(VersionStatus pa new DeferredVersionSwapService(admin, veniceControllerMultiClusterConfig, stats, metricsRepository); deferredVersionSwapService.updateStore(clusterName, storeName, parentStatus, versionTwo); - ArgumentCaptor regionFilterCaptor = ArgumentCaptor.forClass(String.class); - InOrder inOrder = inOrder(admin, store); + Map childControllers = veniceHelixAdmin.getControllerClientMap(clusterName); + InOrder inOrder = inOrder(store, childControllers.get(region2)); inOrder.verify(store).updateVersionStatus(versionTwo, parentStatus); - inOrder.verify(admin) - .markVersionRolledBack(eq(clusterName), eq(storeName), eq(versionTwo), regionFilterCaptor.capture()); - Assert.assertEquals( - new HashSet<>(Arrays.asList(regionFilterCaptor.getValue().split(","))), - new HashSet<>(Arrays.asList(region2, region3))); + inOrder.verify(childControllers.get(region2)).deleteOldVersion(storeName, versionTwo); + verify(childControllers.get(region3)).deleteOldVersion(storeName, versionTwo); + verify(childControllers.get(region1), never()).deleteOldVersion(anyString(), anyInt()); } @Test @@ -629,15 +625,21 @@ public void testAbandonedParentStatusPersistsWhenChildReconciliationFails() { String storeName = "testStore"; Store store = mockStore(versionOne, versionTwo, storeName); doReturn(store).when(repository).getStore(storeName); - doThrow(new VeniceException("child reconciliation failed")).when(admin) - .markVersionRolledBack(eq(clusterName), eq(storeName), eq(versionTwo), anyString()); + + Map childControllers = veniceHelixAdmin.getControllerClientMap(clusterName); + doThrow(new VeniceException("child reconciliation failed")) + .when(childControllers.get(region2)) + .deleteOldVersion(anyString(), anyInt()); + doThrow(new VeniceException("child reconciliation failed")) + .when(childControllers.get(region3)) + .deleteOldVersion(anyString(), anyInt()); DeferredVersionSwapService deferredVersionSwapService = new DeferredVersionSwapService(admin, veniceControllerMultiClusterConfig, stats, metricsRepository); deferredVersionSwapService.updateStore(clusterName, storeName, VersionStatus.ERROR, versionTwo); verify(store).updateVersionStatus(versionTwo, VersionStatus.ERROR); - verify(admin).markVersionRolledBack(eq(clusterName), eq(storeName), eq(versionTwo), anyString()); + verify(childControllers.get(region2)).deleteOldVersion(storeName, versionTwo); } @Test @@ -655,12 +657,11 @@ public void testTerminalParentStatusRepairsStrandedChildVersions() throws Except new DeferredVersionSwapService(admin, veniceControllerMultiClusterConfig, stats, metricsRepository); deferredVersionSwapService.startInner(); - ArgumentCaptor regionFilterCaptor = ArgumentCaptor.forClass(String.class); - TestUtils.waitForNonDeterministicAssertion(5, TimeUnit.SECONDS, () -> verify(admin, atLeastOnce()) - .markVersionRolledBack(eq(clusterName), eq(storeName), eq(versionTwo), regionFilterCaptor.capture())); - Assert.assertEquals( - new HashSet<>(Arrays.asList(regionFilterCaptor.getValue().split(","))), - new HashSet<>(Arrays.asList(region2, region3))); + TestUtils.waitForNonDeterministicAssertion(5, TimeUnit.SECONDS, () -> { + Map childControllers = veniceHelixAdmin.getControllerClientMap(clusterName); + verify(childControllers.get(region2), atLeastOnce()).deleteOldVersion(storeName, versionTwo); + verify(childControllers.get(region3), atLeastOnce()).deleteOldVersion(storeName, versionTwo); + }); } @Test @@ -696,16 +697,9 @@ public void testTerminalParentStatusRetriesUntilInProgressChildCompletes() throw new DeferredVersionSwapService(admin, veniceControllerMultiClusterConfig, stats, metricsRepository); deferredVersionSwapService.startInner(); - ArgumentCaptor regionFilterCaptor = ArgumentCaptor.forClass(String.class); TestUtils.waitForNonDeterministicAssertion(5, TimeUnit.SECONDS, () -> { - verify(childControllers.get(region2), atLeastOnce()).getStore(storeName, controllerTimeout); - verify(admin, atLeastOnce()) - .markVersionRolledBack(eq(clusterName), eq(storeName), eq(versionTwo), regionFilterCaptor.capture()); - Assert.assertTrue( - regionFilterCaptor.getAllValues() - .stream() - .map(filter -> new HashSet<>(Arrays.asList(filter.split(",")))) - .anyMatch(regions -> regions.contains(region2))); + verify(childControllers.get(region2), atLeast(2)).getStore(storeName, controllerTimeout); + verify(childControllers.get(region2), atLeastOnce()).deleteOldVersion(storeName, versionTwo); }); } @@ -740,16 +734,9 @@ public void testTerminalParentStatusRetriesWhenChildVersionAppearsLater() throws new DeferredVersionSwapService(admin, veniceControllerMultiClusterConfig, stats, metricsRepository); deferredVersionSwapService.startInner(); - ArgumentCaptor regionFilterCaptor = ArgumentCaptor.forClass(String.class); TestUtils.waitForNonDeterministicAssertion(5, TimeUnit.SECONDS, () -> { verify(childControllers.get(region2), atLeast(2)).getStore(storeName, controllerTimeout); - verify(admin, atLeastOnce()) - .markVersionRolledBack(eq(clusterName), eq(storeName), eq(versionTwo), regionFilterCaptor.capture()); - Assert.assertTrue( - regionFilterCaptor.getAllValues() - .stream() - .map(filter -> new HashSet<>(Arrays.asList(filter.split(",")))) - .anyMatch(regions -> regions.contains(region2))); + verify(childControllers.get(region2), atLeastOnce()).deleteOldVersion(storeName, versionTwo); }); } @@ -783,16 +770,12 @@ public void testTerminalParentStatusRetriesCurrentTargetAfterItBecomesNonCurrent new DeferredVersionSwapService(admin, veniceControllerMultiClusterConfig, stats, metricsRepository); deferredVersionSwapService.startInner(); - ArgumentCaptor regionFilterCaptor = ArgumentCaptor.forClass(String.class); TestUtils.waitForNonDeterministicAssertion(5, TimeUnit.SECONDS, () -> { verify(childControllers.get(region1), atLeast(2)).getStore(storeName, controllerTimeout); - verify(admin, atLeastOnce()) - .markVersionRolledBack(eq(clusterName), eq(storeName), eq(versionTwo), regionFilterCaptor.capture()); - Assert.assertTrue( - regionFilterCaptor.getAllValues() - .stream() - .map(filter -> new HashSet<>(Arrays.asList(filter.split(",")))) - .anyMatch(regions -> regions.containsAll(Arrays.asList(region1, region2, region3)))); + // After region1 becomes non-current, all 3 regions should have deleteOldVersion called. + verify(childControllers.get(region1), atLeastOnce()).deleteOldVersion(storeName, versionTwo); + verify(childControllers.get(region2), atLeastOnce()).deleteOldVersion(storeName, versionTwo); + verify(childControllers.get(region3), atLeastOnce()).deleteOldVersion(storeName, versionTwo); }); } @@ -824,12 +807,12 @@ public void testTerminalReconciliationIncludesSupersededParentVersions() throws new DeferredVersionSwapService(admin, veniceControllerMultiClusterConfig, stats, metricsRepository); deferredVersionSwapService.startInner(); - ArgumentCaptor regionFilterCaptor = ArgumentCaptor.forClass(String.class); - TestUtils.waitForNonDeterministicAssertion(5, TimeUnit.SECONDS, () -> verify(admin, atLeastOnce()) - .markVersionRolledBack(eq(clusterName), eq(storeName), eq(versionTwo), regionFilterCaptor.capture())); - Assert.assertEquals( - new HashSet<>(Arrays.asList(regionFilterCaptor.getValue().split(","))), - new HashSet<>(Arrays.asList(region1, region2, region3))); + Map childControllers = veniceHelixAdmin.getControllerClientMap(clusterName); + TestUtils.waitForNonDeterministicAssertion(5, TimeUnit.SECONDS, () -> { + verify(childControllers.get(region1), atLeastOnce()).deleteOldVersion(storeName, versionTwo); + verify(childControllers.get(region2), atLeastOnce()).deleteOldVersion(storeName, versionTwo); + verify(childControllers.get(region3), atLeastOnce()).deleteOldVersion(storeName, versionTwo); + }); } /** diff --git a/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestVeniceHelixAdmin.java b/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestVeniceHelixAdmin.java index f4458f94cf6..b7ad290c0eb 100644 --- a/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestVeniceHelixAdmin.java +++ b/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestVeniceHelixAdmin.java @@ -1293,66 +1293,9 @@ public void testRollForwardNoFutureVersions() { verify(mockVeniceHelixAdmin, never()).storeMetadataUpdate(any(), any(), any()); } - @DataProvider(name = "markVersionRolledBackStatuses") - public Object[][] markVersionRolledBackStatuses() { - return new Object[][] { - { VersionStatus.PUSHED, 1, true }, - { VersionStatus.ONLINE, 1, true }, - { VersionStatus.STARTED, 1, false }, - { VersionStatus.CREATED, 1, false }, - { VersionStatus.NOT_CREATED, 1, false }, - { VersionStatus.ERROR, 1, false }, - { VersionStatus.KILLED, 1, false }, - { VersionStatus.ROLLED_BACK, 1, false }, - { VersionStatus.PUSHED, 2, false } }; - } - - @Test(dataProvider = "markVersionRolledBackStatuses") - public void testMarkVersionRolledBackOnlyUpdatesCompletedNonCurrentVersions( - VersionStatus initialStatus, - int currentVersion, - boolean expectUpdate) { - VeniceHelixAdmin mockVeniceHelixAdmin = mock(VeniceHelixAdmin.class); - Store mockStore = mock(Store.class); - HelixVeniceClusterResources mockClusterResources = mock(HelixVeniceClusterResources.class); - doReturn("region1").when(mockVeniceHelixAdmin).getRegionName(); - doReturn(true).when(mockStore).containsVersion(2); - doReturn(currentVersion).when(mockStore).getCurrentVersion(); - doReturn(initialStatus).when(mockStore).getVersionStatus(2); - doAnswer(invocation -> { - VeniceHelixAdmin.StoreMetadataOperation updater = invocation.getArgument(2); - updater.update(mockStore, mockClusterResources); - return null; - }).when(mockVeniceHelixAdmin).storeMetadataUpdate(eq(clusterName), eq(storeName), any()); - doCallRealMethod().when(mockVeniceHelixAdmin) - .markVersionRolledBack(anyString(), anyString(), anyInt(), anyString()); - - mockVeniceHelixAdmin.markVersionRolledBack(clusterName, storeName, 2, "region1"); - - if (expectUpdate) { - verify(mockStore).updateVersionStatus(2, VersionStatus.ROLLED_BACK); - verify(mockStore).setLatestVersionPromoteToCurrentTimestamp(anyLong()); - } else { - verify(mockStore, never()).updateVersionStatus(anyInt(), any()); - verify(mockStore, never()).setLatestVersionPromoteToCurrentTimestamp(anyLong()); - } - } - - @Test - public void testMarkVersionRolledBackHonorsRegionFilter() { - VeniceHelixAdmin mockVeniceHelixAdmin = mock(VeniceHelixAdmin.class); - doReturn("region1").when(mockVeniceHelixAdmin).getRegionName(); - doCallRealMethod().when(mockVeniceHelixAdmin) - .markVersionRolledBack(anyString(), anyString(), anyInt(), anyString()); - - mockVeniceHelixAdmin.markVersionRolledBack(clusterName, storeName, 2, "region2"); - - verify(mockVeniceHelixAdmin, never()).storeMetadataUpdate(anyString(), anyString(), any()); - } - /** * isPartitionReadyToServe=>true: Future version exists and partitions are ready → success - * isPartitionReadyToServe=>false: Future version exists but partitions aren’t ready → exception + * isPartitionReadyToServe=>false: Future version exists but partitions aren't ready → exception */ @Test(dataProvider = "True-and-False", dataProviderClass = DataProviderUtils.class) public void testRollForwardPartitionNotReady(boolean isPartitionReadyToServe) throws Exception { diff --git a/services/venice-controller/src/test/java/com/linkedin/venice/controller/kafka/consumer/AdminExecutionTaskTest.java b/services/venice-controller/src/test/java/com/linkedin/venice/controller/kafka/consumer/AdminExecutionTaskTest.java index d4d8e576af2..41ecf5e86ab 100644 --- a/services/venice-controller/src/test/java/com/linkedin/venice/controller/kafka/consumer/AdminExecutionTaskTest.java +++ b/services/venice-controller/src/test/java/com/linkedin/venice/controller/kafka/consumer/AdminExecutionTaskTest.java @@ -690,78 +690,4 @@ private AdminOperationWrapper createAddVersionWrapper( System.currentTimeMillis(), System.currentTimeMillis()); } - - @Test - public void testHandleMarkVersionRolledBackWithRegionFilter() { - when(mockAdmin.isLeaderControllerFor(clusterName)).thenReturn(true); - - Queue queue = new ConcurrentLinkedQueue<>(); - queue.add(createMarkVersionRolledBackWrapper(1L, 42, "prod-lva1,prod-ltx1")); - - AdminExecutionTask task = new AdminExecutionTask( - mockLogger, - clusterName, - storeName, - lastSucceededExecutionIdMap, - lastPersistedExecutionId, - queue, - mockAdmin, - mockExecutionIdAccessor, - isParentController, - mockStats, - regionName, - inflightThreadsByStore); - task.call(); - - verify(mockAdmin).markVersionRolledBack(eq(clusterName), eq(storeName), eq(42), eq("prod-lva1,prod-ltx1")); - } - - @Test - public void testHandleMarkVersionRolledBackWithNullRegionFilter() { - when(mockAdmin.isLeaderControllerFor(clusterName)).thenReturn(true); - - Queue queue = new ConcurrentLinkedQueue<>(); - queue.add(createMarkVersionRolledBackWrapper(1L, 7, null)); - - AdminExecutionTask task = new AdminExecutionTask( - mockLogger, - clusterName, - storeName, - lastSucceededExecutionIdMap, - lastPersistedExecutionId, - queue, - mockAdmin, - mockExecutionIdAccessor, - isParentController, - mockStats, - regionName, - inflightThreadsByStore); - task.call(); - - // A null regionsFilter must not NPE and must be passed through as null. - verify(mockAdmin).markVersionRolledBack(eq(clusterName), eq(storeName), eq(7), eq(null)); - } - - private AdminOperationWrapper createMarkVersionRolledBackWrapper(long executionId, int versionNum, String regionFilter) { - AdminOperation adminOperation = new AdminOperation(); - adminOperation.operationType = AdminMessageType.MARK_VERSION_ROLLED_BACK.getValue(); - adminOperation.executionId = executionId; - - com.linkedin.venice.controller.kafka.protocol.admin.MarkVersionRolledBack markVersionRolledBack = - new com.linkedin.venice.controller.kafka.protocol.admin.MarkVersionRolledBack(); - markVersionRolledBack.clusterName = clusterName; - markVersionRolledBack.storeName = storeName; - markVersionRolledBack.versionNum = versionNum; - markVersionRolledBack.regionsFilter = regionFilter; - adminOperation.payloadUnion = markVersionRolledBack; - - PubSubPosition position = InMemoryPubSubPosition.of(1L); - return new AdminOperationWrapper( - adminOperation, - position, - executionId, - System.currentTimeMillis(), - System.currentTimeMillis(), - System.currentTimeMillis()); - } } diff --git a/services/venice-controller/src/test/java/com/linkedin/venice/controller/kafka/protocol/enums/AdminMessageTypeTest.java b/services/venice-controller/src/test/java/com/linkedin/venice/controller/kafka/protocol/enums/AdminMessageTypeTest.java index 0f1bb4f5045..8541af26fc6 100644 --- a/services/venice-controller/src/test/java/com/linkedin/venice/controller/kafka/protocol/enums/AdminMessageTypeTest.java +++ b/services/venice-controller/src/test/java/com/linkedin/venice/controller/kafka/protocol/enums/AdminMessageTypeTest.java @@ -46,7 +46,6 @@ public void testDimensionInterface() { .put(AdminMessageType.DELETE_UNUSED_VALUE_SCHEMA, "delete_unused_value_schema") .put(AdminMessageType.ROLLBACK_CURRENT_VERSION, "rollback_current_version") .put(AdminMessageType.ROLLFORWARD_CURRENT_VERSION, "rollforward_current_version") - .put(AdminMessageType.MARK_VERSION_ROLLED_BACK, "mark_version_rolled_back") .build(); new VeniceDimensionTestFixture<>( AdminMessageType.class, From 5e3d9cc685ad1d17c8e6579b05a4a886586cf61a Mon Sep 17 00:00:00 2001 From: Kai-Sern Lim Date: Wed, 22 Jul 2026 01:16:50 -0700 Subject: [PATCH 4/5] Fix Spotless formatting violations in controller tests Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...estDeferredVersionSwapServiceWithSequentialRollout.java | 7 ++----- .../linkedin/venice/controller/TestVeniceHelixAdmin.java | 1 - 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestDeferredVersionSwapServiceWithSequentialRollout.java b/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestDeferredVersionSwapServiceWithSequentialRollout.java index 115da274676..9cc78a9fe76 100644 --- a/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestDeferredVersionSwapServiceWithSequentialRollout.java +++ b/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestDeferredVersionSwapServiceWithSequentialRollout.java @@ -4,7 +4,6 @@ import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.anyDouble; import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.atLeastOnce; @@ -627,11 +626,9 @@ public void testAbandonedParentStatusPersistsWhenChildReconciliationFails() { doReturn(store).when(repository).getStore(storeName); Map childControllers = veniceHelixAdmin.getControllerClientMap(clusterName); - doThrow(new VeniceException("child reconciliation failed")) - .when(childControllers.get(region2)) + doThrow(new VeniceException("child reconciliation failed")).when(childControllers.get(region2)) .deleteOldVersion(anyString(), anyInt()); - doThrow(new VeniceException("child reconciliation failed")) - .when(childControllers.get(region3)) + doThrow(new VeniceException("child reconciliation failed")).when(childControllers.get(region3)) .deleteOldVersion(anyString(), anyInt()); DeferredVersionSwapService deferredVersionSwapService = diff --git a/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestVeniceHelixAdmin.java b/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestVeniceHelixAdmin.java index b7ad290c0eb..594d23503de 100644 --- a/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestVeniceHelixAdmin.java +++ b/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestVeniceHelixAdmin.java @@ -107,7 +107,6 @@ import org.mockito.MockedStatic; import org.mockito.Mockito; import org.testng.TestException; -import org.testng.annotations.DataProvider; import org.testng.annotations.Test; From f2e7273fb3ef557380f90d61f1e9b773dc7fa731 Mon Sep 17 00:00:00 2001 From: Kai-Sern Lim Date: Wed, 22 Jul 2026 01:38:38 -0700 Subject: [PATCH 5/5] fix: resolve CI failures from spotless, metric count, and branch coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - DeferredVersionSwapService.java: wrap long isAbandonedDeferredVersion return at the && operator and collapse split childVersion assignment to one line (spotlessJavaCheck violations) - TestDeferredVersionSwapServiceWithSequentialRollout.java: condense abandonedParentStatuses() array to match google-java-format style (spotlessJavaCheck violation) - ServerMetricEntityTest: update expected count 185 → 186 to reflect the new VERSION_COUNT entry added to StorageEngineOtelMetricEntity - AsyncMetricEntityStateOneEnumTest / AsyncMetricEntityStateTwoEnumsTest: add testCloseUnregistersObservableDoubleGauge and testCloseOnDisabledInstanceIsNoOp to cover the two missing branches in the new close() method (fixes venice-client-common diffCoverage < 50%) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../davinci/stats/ServerMetricEntityTest.java | 2 +- .../AsyncMetricEntityStateOneEnumTest.java | 34 +++++++++++++++++- .../AsyncMetricEntityStateTwoEnumsTest.java | 35 +++++++++++++++++++ .../DeferredVersionSwapService.java | 6 ++-- ...rsionSwapServiceWithSequentialRollout.java | 4 +-- 5 files changed, 73 insertions(+), 8 deletions(-) diff --git a/clients/da-vinci-client/src/test/java/com/linkedin/davinci/stats/ServerMetricEntityTest.java b/clients/da-vinci-client/src/test/java/com/linkedin/davinci/stats/ServerMetricEntityTest.java index 8fee846e59d..2c1ae4978be 100644 --- a/clients/da-vinci-client/src/test/java/com/linkedin/davinci/stats/ServerMetricEntityTest.java +++ b/clients/da-vinci-client/src/test/java/com/linkedin/davinci/stats/ServerMetricEntityTest.java @@ -22,7 +22,7 @@ public class ServerMetricEntityTest { @Test public void testServerMetricEntitiesCount() { - assertEquals(SERVER_METRIC_ENTITIES.size(), 185, "Expected 185 unique metric entities"); + assertEquals(SERVER_METRIC_ENTITIES.size(), 186, "Expected 186 unique metric entities"); } /** diff --git a/internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateOneEnumTest.java b/internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateOneEnumTest.java index 59141b88cbb..9c6bdf82d49 100644 --- a/internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateOneEnumTest.java +++ b/internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateOneEnumTest.java @@ -19,6 +19,7 @@ import com.linkedin.venice.stats.metrics.AsyncMetricResolvers.LiveStateResolverOneEnum; import com.linkedin.venice.stats.metrics.AsyncMetricResolvers.ValueResolverOneEnum; import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.ObservableDoubleGauge; import io.opentelemetry.api.metrics.ObservableDoubleMeasurement; import io.opentelemetry.api.metrics.ObservableLongGauge; import io.opentelemetry.api.metrics.ObservableLongMeasurement; @@ -102,9 +103,40 @@ public void testCloseUnregistersObservableGauge() { verify(gauge).close(); } + @Test + public void testCloseUnregistersObservableDoubleGauge() { + when(mockMetricEntity.getMetricType()).thenReturn(MetricType.ASYNC_DOUBLE_GAUGE); + ObservableDoubleGauge gauge = mock(ObservableDoubleGauge.class); + when(mockOtelRepository.registerObservableDoubleGauge(eq(mockMetricEntity), any())).thenReturn(gauge); + AsyncMetricEntityStateOneEnum metricState = AsyncMetricEntityStateOneEnum.create( + mockMetricEntity, + mockOtelRepository, + baseDimensionsMap, + DimensionEnum1.class, + e -> e, + (state, e) -> 1L); + + metricState.close(); + + verify(gauge).close(); + } + + @Test + public void testCloseOnDisabledInstanceIsNoOp() { + AsyncMetricEntityStateOneEnum metricState = AsyncMetricEntityStateOneEnum.create( + mockMetricEntity, + null /* OTel disabled */, + baseDimensionsMap, + DimensionEnum1.class, + e -> e, + (state, e) -> 1L); + + // Must not throw when instrument is null. + metricState.close(); + } + @Test public void testCallbackEmitsOnlyWhenLiveStateResolverReturnsNonNull() { - // liveStateResolver returns state for DIMENSION_ONE only; DIMENSION_TWO is dormant. LiveStateResolverOneEnum liveStateResolver = e -> e == DimensionEnum1.DIMENSION_ONE ? "live" : null; ValueResolverOneEnum valueResolver = (state, e) -> 42L; diff --git a/internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateTwoEnumsTest.java b/internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateTwoEnumsTest.java index 0343d9bbfb5..04a3060ee11 100644 --- a/internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateTwoEnumsTest.java +++ b/internal/venice-client-common/src/test/java/com/linkedin/venice/stats/metrics/AsyncMetricEntityStateTwoEnumsTest.java @@ -20,6 +20,7 @@ import com.linkedin.venice.stats.metrics.AsyncMetricResolvers.LiveStateResolverTwoEnums; import com.linkedin.venice.stats.metrics.AsyncMetricResolvers.ValueResolverTwoEnums; import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.metrics.ObservableDoubleGauge; import io.opentelemetry.api.metrics.ObservableDoubleMeasurement; import io.opentelemetry.api.metrics.ObservableLongGauge; import io.opentelemetry.api.metrics.ObservableLongMeasurement; @@ -113,6 +114,40 @@ public void testCloseUnregistersObservableGauge() { verify(gauge).close(); } + @Test + public void testCloseUnregistersObservableDoubleGauge() { + when(mockMetricEntity.getMetricType()).thenReturn(MetricType.ASYNC_DOUBLE_GAUGE); + ObservableDoubleGauge gauge = mock(ObservableDoubleGauge.class); + when(mockOtelRepository.registerObservableDoubleGauge(eq(mockMetricEntity), any())).thenReturn(gauge); + AsyncMetricEntityStateTwoEnums metricState = AsyncMetricEntityStateTwoEnums.create( + mockMetricEntity, + mockOtelRepository, + baseDimensionsMap, + DimensionEnum1.class, + DimensionEnum2.class, + (e1, e2) -> e1, + (state, e1, e2) -> 1L); + + metricState.close(); + + verify(gauge).close(); + } + + @Test + public void testCloseOnDisabledInstanceIsNoOp() { + AsyncMetricEntityStateTwoEnums metricState = AsyncMetricEntityStateTwoEnums.create( + mockMetricEntity, + null /* OTel disabled */, + baseDimensionsMap, + DimensionEnum1.class, + DimensionEnum2.class, + (e1, e2) -> e1, + (state, e1, e2) -> 1L); + + // Must not throw when instrument is null. + metricState.close(); + } + @Test public void testCallbackEmitsOnlyWhenLiveStateResolverReturnsNonNull() { LiveStateResolverTwoEnums liveStateResolver = (e1, e2) -> { diff --git a/services/venice-controller/src/main/java/com/linkedin/venice/controller/DeferredVersionSwapService.java b/services/venice-controller/src/main/java/com/linkedin/venice/controller/DeferredVersionSwapService.java index 9b0d06c28cf..561f17174ac 100644 --- a/services/venice-controller/src/main/java/com/linkedin/venice/controller/DeferredVersionSwapService.java +++ b/services/venice-controller/src/main/java/com/linkedin/venice/controller/DeferredVersionSwapService.java @@ -971,7 +971,8 @@ private static String getVersionProcessingKey(String clusterName, String storeNa } private static boolean isAbandonedDeferredVersion(Version version) { - return version != null && version.isVersionSwapDeferred() && ABANDONED_VERSION_STATUSES.contains(version.getStatus()); + return version != null && version.isVersionSwapDeferred() + && ABANDONED_VERSION_STATUSES.contains(version.getStatus()); } private void submitTerminalVersionReconciliationTasks( @@ -1393,8 +1394,7 @@ private boolean reconcileAbandonedVersionInChildRegions( } StoreInfo childStore = storeResponse.getStore(); - Version childVersion = - getVersionFromStoreInRegion(region, storeName, targetVersionNum, storeResponse); + Version childVersion = getVersionFromStoreInRegion(region, storeName, targetVersionNum, storeResponse); if (childVersion == null) { allRegionsTerminal = false; continue; diff --git a/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestDeferredVersionSwapServiceWithSequentialRollout.java b/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestDeferredVersionSwapServiceWithSequentialRollout.java index 9cc78a9fe76..dad9c24d45a 100644 --- a/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestDeferredVersionSwapServiceWithSequentialRollout.java +++ b/services/venice-controller/src/test/java/com/linkedin/venice/controller/TestDeferredVersionSwapServiceWithSequentialRollout.java @@ -593,9 +593,7 @@ public void testSequentialRolloutPostSwapValidationRollbackMarksNonTargetRegions @DataProvider(name = "abandonedParentStatuses") public Object[][] abandonedParentStatuses() { - return new Object[][] { - { VersionStatus.ERROR }, - { VersionStatus.PARTIALLY_ONLINE }, + return new Object[][] { { VersionStatus.ERROR }, { VersionStatus.PARTIALLY_ONLINE }, { VersionStatus.ROLLED_BACK } }; }