[protocol][build] Stage PushJobDetails v6

[ymuppala]

Summary

Stage PushJobDetails v6 with an extensible additionalPushMetrics map.

The map stores optional long metrics by name. The first consumers are:

• externalStorageWriteTimeMs: summed task time for external writes, including throttling, retries, flush, and close.
• veniceWriteTimeMs: summed task time for Venice writes, flush, and close.

These values are sums across successful data-writer task outputs, not the push job's overall duration. Missing keys mean the metric was not reported, and null means no additional metrics were reported.

Staged rollout

This PR adds the v6 schema and pins generated PushJobDetails classes to v5 through versionOverrides. It does not activate v6 or change runtime serialization.

Follow-up PR #2972 removes the pin, activates v6, and adds VPJ collection and controller metric emission. Controllers must deploy with v6 support before upgraded push jobs send v6 payloads.

Testing Done

./gradlew :internal:venice-common:test --tests "com.linkedin.venice.status.protocol.TestPushJobDetailsSchemaCompatibility"
BUILD SUCCESSFUL

No existing tests were changed.

[Copilot]

Pull request overview

Stages a new PushJobDetails Avro schema version (v6) intended to add wall-clock timing fields so push-job status reporting can later attribute time to external storage vs Venice write paths, while keeping the active/serialized protocol pinned to v5 during rollout.

Changes:

• Adds PushJobDetails v6 schema that appends two defaulted timing fields (externalStorageWriteTimeMs, veniceWriteTimeMs).
• Pins generated PushJobDetails classes to v5 via versionOverrides in the root build.gradle (staged protocol pattern).
• Extends schema compatibility tests to validate staged-v6 shape and v5-writer → v6-reader default resolution.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File	Description
internal/venice-common/src/main/resources/avro/PushJobDetails/v6/PushJobDetails.avsc	Introduces v6 schema with two new defaulted timing fields.
internal/venice-common/src/test/java/com/linkedin/venice/status/protocol/TestPushJobDetailsSchemaCompatibility.java	Adds tests for staged v6 registration/activation assumptions and compatibility/default resolution.
build.gradle	Pins PushJobDetails compilation to v5 via versionOverrides while shipping v6 resources.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

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

Suppressed comments (1)

internal/venice-common/src/test/java/com/linkedin/venice/status/protocol/TestPushJobDetailsSchemaCompatibility.java:61

• PR description says this change lets controllers register PushJobDetails v6 while code still serializes v5, but this test explicitly asserts the opposite: v6 is staged/inert and not visible to runtime discovery/serializer (so controllers won’t register v6 until the follow-up PR bumps PUSH_JOB_DETAILS to 6). Please update the PR description/rollout section to match the actual behavior to avoid incorrect deployment assumptions.

   * v6 is staged, which means it is inert: the {@code versionOverrides} pin in the root build.gradle keeps the
   * generated classes on v5, and {@link Utils#getAllSchemasFromResources(AvroProtocolDefinition)} stops at the
   * compiled version, so no runtime path in this build reads, writes or registers v6. Controllers register value
   * schemas only up to {@link AvroProtocolDefinition#currentProtocolVersion}, and the serializer only caches
   * readers for the versions resource discovery returned, so both stop at v5 too.

[Copilot]

Pull request overview

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

Suppressed comments (1)

internal/venice-common/src/main/resources/avro/PushJobDetails/v6/PushJobDetails.avsc:93

• This PR introduces PushJobDetails v6 with two new fields and relies on a build pin to keep v6 staged/inert, but there are no unit tests in the repo asserting the staging behavior (v6 not visible via schema discovery/serializer) or v5↔v6 compatibility for the new fields. Add/extend schema compatibility tests so the staging contract and default resolution (-1) are enforced.

    {
      "name": "externalStorageWriteTimeMs",
      "doc": "Total elapsed time in milliseconds, summed across all successful data writer task outputs of this push, spent in the external storage (Spaniel/TiKV) write path: throttling wait, batchPut calls including retries and retry backoff, external flush and external close. This is a sum of per-task wall-clock durations, NOT the push job's overall wall-clock duration, so with N parallel tasks it can be up to N times the push duration. -1 when the push did not report it (no dual write configured, or an older push job version).",
      "type": "long",
      "default": -1
    },
    {
      "name": "veniceWriteTimeMs",
      "doc": "Total elapsed time in milliseconds, summed across all successful data writer task outputs of this push, spent invoking the Venice/Kafka writes and flushing/closing the Venice writer. This is a sum of per-task wall-clock durations, NOT the push job's overall wall-clock duration. -1 when the push did not report it.",
      "type": "long",
      "default": -1

[Copilot]

Pull request overview

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

Suppressed comments (1)

internal/venice-common/src/main/resources/avro/PushJobDetails/v6/PushJobDetails.avsc:87

• The PR description says PushJobDetails v6 appends two top-level fields (externalStorageWriteTimeMs / veniceWriteTimeMs, default -1). However, the v6 schema being added here appends a single nullable map field (additionalPushMetrics) and encodes those metrics as map keys (with “missing key” semantics), and the two top-level fields do not exist. Please align the implementation and the stated rollout/compatibility story by either (a) changing the schema to add the two explicit fields as described, or (b) updating the PR description (and any related docs) to match the map-based schema evolution and its defaults/"not reported" semantics.

    {
      "name": "additionalPushMetrics",
      "doc": "Extensible map of additional push-job metrics keyed by metric name, so new metrics (e.g. per-destination write timings, error-retry counts, per-batch latency) can be added without evolving this schema again. Values reported so far: 'externalStorageWriteTimeMs' - total elapsed time in milliseconds, summed across all successful data writer task outputs of this push, spent in the external storage write path (throttling wait, batch write calls including retries and retry backoff, flush and close); 'veniceWriteTimeMs' - total elapsed time in milliseconds, summed across all successful data writer task outputs of this push, spent invoking the Venice/Kafka writes and flushing/closing the Venice writer. Both are sums of per-task wall-clock durations, NOT the push job's overall wall-clock duration, so with N parallel tasks they can be up to N times the push duration. A missing key means the push did not report that metric (no dual write configured, or an older push job version). Null when no additional metrics were reported.",
      "type": ["null", {"type": "map", "values": "long"}],
      "default": null

[Copilot]

Pull request overview

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