Skip to content

[WIP][protocol][common][vpj] Activate StoreMetaValue v44 and add VPJ dual-write gating predicate#2821

Draft
sixpluszero wants to merge 1 commit into
linkedin:mainfrom
sixpluszero:jlliu/external-storage-dual-write-gating
Draft

[WIP][protocol][common][vpj] Activate StoreMetaValue v44 and add VPJ dual-write gating predicate#2821
sixpluszero wants to merge 1 commit into
linkedin:mainfrom
sixpluszero:jlliu/external-storage-dual-write-gating

Conversation

@sixpluszero
Copy link
Copy Markdown
Contributor

Problem Statement

PR #2814 staged storageMode and externalStorageReadMode on StoreMetaValue v44 / AdminOperation v99, but pinned both via versionOverrides so neither field is readable from Java today. The Venice Batch-Only Store Dual-Write design (separate doc) needs VPJ to consult the per-version storageMode at job-setup time and decide whether to engage the dual-write code path. Until v44 is active and there is a small SPI-style config knob on the VPJ side, no follow-up wiring can land.

This PR is the minimal scaffolding: activate v44, expose storageMode through the Java Version model, define a VPJ config key for the external-storage writer impl, and add a single static predicate that combines the two halves of the gate. The actual writer interface and AbstractVeniceWriter wrapper are deliberately deferred to the follow-up PR — this change is gating-only and has no runtime effect until that wiring lands.

Solution

StoreMetaValue v44 is now active (AdminOperation v99 stays pinned)

build.gradle removes the StoreMetaValue v43 override. AdminOperation v98 stays pinned — admin-tool / controller wiring for the store-level setter is a separate follow-up. Activating v44 makes three new fields readable in Java: targetRegionPromoted and externalStorageReadMode stay dormant (no Java reads them yet); only storageMode is consumed by this PR.

Java StorageMode enum

com.linkedin.venice.meta.StorageModeINTERNAL(0), DUAL_WRITE(1), EXTERNAL(2). Implements VeniceEnumValue with the strict-throw valueOf(int) pattern, mirroring CompressionStrategy.

Version.getStorageMode() / setStorageMode()

Added to the Version interface and implemented on VersionImpl (delegates to storeVersion.storageMode) and ReadOnlyStore.ReadOnlyVersion (setter throws, matching the existing setCompressionStrategy pattern). ReadOnlyStore.convertVersion() also writes the value back into StoreVersion so the field round-trips through Store ↔ StoreVersion serialization.

VPJ config key

push.job.external.storage.writer.class — fully-qualified class name of the external-storage writer impl. Defined in VenicePushJobConstants. Presence of a non-empty value is the VPJ-side half of the gating predicate. The class itself will be loaded reflectively by the follow-up PR; this PR only checks the config string is non-empty.

Gating predicate

ExternalStorageWriteUtils.isDualWriteToExternalStorageFromVpjEnabled(VeniceProperties, Version) returns true iff:

  • the VPJ config is set to a non-empty string, AND
  • version.getStorageMode() == StorageMode.DUAL_WRITE.

Null-safe on both arguments. Nothing calls this helper yet — the follow-up PR will plug it into the Spark-mode createBasicVeniceWriter() path.

Code changes

  • Added new code behind a config. New config: push.job.external.storage.writer.class (default empty/unset — gating predicate stays false). Plus the per-version storageMode field whose default is INTERNAL(0). With both at defaults today's behavior is unchanged.
  • Introduced new log lines. (None.)

Concurrency-Specific Checks

N/A — this PR only adds an enum, an immutable static predicate, and accessors on existing model classes. No new threads, locks, or shared state.

How was this PR tested?

  • New unit tests added.
    • StorageModeTest extends VeniceEnumValueTest<StorageMode> (mirrors CompressionStrategyTest): verifies the int↔enum mapping is stable and out-of-bound values throw.
    • ExternalStorageWriteUtilsTest: 8 cases covering both halves set, writer-class missing / empty, mode INTERNAL / EXTERNAL / default, null Version, null VeniceProperties.
  • Verified backward compatibility — storageMode defaults to 0 (INTERNAL), so existing stores' behavior is unchanged. targetRegionPromoted (false) and externalStorageReadMode (0, VENICE_ONLY) also default-zero; no Java code reads them yet.
  • ./gradlew compileJava compileTestJava — clean across all modules; no downstream breakage from the v44 unpin or the Version interface extension.
  • ./gradlew :internal:venice-common:test --tests 'com.linkedin.venice.meta.StorageModeTest' — passes.
  • ./gradlew :clients:venice-push-job:test --tests 'com.linkedin.venice.vpj.ExternalStorageWriteUtilsTest' — 8/8 pass.
  • Regression spot-checks on Version/Store serialization tests — all pass: TestVersion, ReadOnlyStoreTest, TestZKStore, VersionJsonSerializerTest, ReadOnlyViewStoreTest, StoreVersionInfoTest, NameRepositoryTest.

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

  • No. Every new behavior is gated behind a config that defaults to off. The VPJ predicate is defined but not yet called from any production code path. Activating StoreMetaValue v44 exposes three new fields whose defaults preserve current behavior.

…write

gating predicate

Removes the StoreMetaValue v43 versionOverride so storageMode is readable in
Java; AdminOperation v98 stays pinned (admin-tool wiring lands in a
follow-up).
Adds the StorageMode enum (INTERNAL/DUAL_WRITE/EXTERNAL), exposes
getStorageMode/setStorageMode on the Version interface and its two
implementations, and wires the field into ReadOnlyStore.convertVersion() so it
round-trips through serialization.

Adds push.job.external.storage.writer.class and a static helper
ExternalStorageWriteUtils.isDualWriteToExternalStorageFromVpjEnabled that
returns true iff the config is non-empty AND the target version's storageMode
is DUAL_WRITE. The writer SPI and BufferedDualWriter integration are
intentionally deferred to a follow-up PR; nothing calls this predicate yet.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant