Dual Storage Architecture

[malinosqui]

Based on the provided code changes, here is a precise description for the pull request:

Pull Request Description

Summary
This pull request enhances the "Mode 3" dual-writer architecture by optimizing how operations interact with legacy storage. It introduces asynchronous writes to the legacy store, implements missing read functionality, adds observability metrics, and expands test coverage.

Functional Changes & Enhancements:

• Asynchronous Legacy Writes: Modified Create, Update, Delete, and DeleteCollection operations in Mode 3 to execute legacy storage updates asynchronously in the background with a 10-second timeout. This ensures that primary storage operations are no longer blocked by legacy storage latency.
• Implemented List Operation: Added the previously missing List method for Mode 3, which now correctly retrieves collections directly from the primary storage (Unified Store).
• Observability & Metrics: Integrated execution duration and error tracking metrics (recordStorageDuration, recordLegacyDuration) across all CRUD operations to monitor the performance and reliability of both primary and legacy storage backends.
• Testing Expansion:
  • Completely rewrote the unit tests for DualWriterMode3 using table-driven mock tests to validate all storage operations and error handling.
  • Added new integration tests for the Playlist API to verify Mode 3 dual-writing behavior across different storage backends (file, unified, and etcd).
• Dependency Updates: Updated go.work.sum to include new OpenTelemetry (tracing/metrics), Azure SDK, and Alertmanager dependencies required for the observability enhancements.

[malinosqui]

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration

Interacting with Kody

• Request a Review: Ask Kody to review your PR manually by adding a comment with the @kody start-review command at the root of your PR.

• Validate Business Logic: Ask Kody to validate your code against business rules by adding a comment with the @kody -v business-logic command.

• Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Current Kody Configuration

Review Options

The following review options are enabled or disabled:

Options	Enabled
Bug	✅
Performance	✅
Security	✅
Business Logic	✅

Access your configuration settings here.

​

[malinosqui]

Data race occurs in legacy Create because the obj pointer is shared concurrently between the background goroutine and the main serialization goroutine. Perform a deep copy using obj.DeepCopyObject() to prevent crashes from in-place mutations common in k8s.io/apiserver storage layers.

objCopy := obj.DeepCopyObject()
go func() {
	ctx, cancel := context.WithTimeoutCause(context.WithoutCancel(ctx), time.Second*10, errors.New("legacy create timeout"))
	defer cancel()

	startLegacy := time.Now()
	_, errObjectSt := d.Legacy.Create(ctx, objCopy, createValidation, options)
	d.recordLegacyDuration(errObjectSt != nil, mode3Str, options.Kind, method, startLegacy)
}()

Prompt for LLM

File pkg/apiserver/rest/dualwriter_mode3.go:

Line 50 to 57:

WHAT: The `obj` pointer is passed to the legacy `Create` background goroutine concurrently while the API server returns it to the main goroutine for serialization.
WHY: `k8s.io/apiserver` storage layers commonly mutate the object in-place (e.g. setting ResourceVersion or defaults); concurrent reads/writes to `obj` across goroutines will cause a data race and potential process crashes.
HOW: Perform a deep copy of the object (e.g., `objCopy := obj.DeepCopyObject()`) before passing it to the background goroutine.

Suggested Code:

	objCopy := obj.DeepCopyObject()
	go func() {
		ctx, cancel := context.WithTimeoutCause(context.WithoutCancel(ctx), time.Second*10, errors.New("legacy create timeout"))
		defer cancel()

		startLegacy := time.Now()
		_, errObjectSt := d.Legacy.Create(ctx, objCopy, createValidation, options)
		d.recordLegacyDuration(errObjectSt != nil, mode3Str, options.Kind, method, startLegacy)
	}()

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[malinosqui]

Cardinality explosion in d.recordStorageDuration occurs because unique object names are used as metric labels instead of options.Kind, exhausting memory and crashing the metrics pipeline. Replace name with options.Kind here and in pkg/apiserver/rest/dualwriter_mode3.go:101-106.

d.recordStorageDuration(false, mode3Str, options.Kind, method, startStorage)

Prompt for LLM

File pkg/apiserver/rest/dualwriter_mode3.go:

Line 106:

WHAT: `d.recordStorageDuration` is called with the object's `name` instead of `options.Kind` on successful deletes.
WHY: Object names are unique per item and unbounded; using them as metric labels will cause a cardinality explosion that exhausts memory and crashes Prometheus or the metrics pipeline.
HOW: Replace `name` with `options.Kind` to bound the metric cardinality.

Suggested Code:

	d.recordStorageDuration(false, mode3Str, options.Kind, method, startStorage)

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​

[malinosqui]

Context cancellation aborts background legacy storage operations in Create, Update, Delete, and DeleteCollection because the parent HTTP request ctx is passed directly to goroutines that outlive the handler. Use context.WithoutCancel(ctx) to detach these operations from the parent request lifecycle.

go func() {
	// Use context.WithoutCancel to prevent immediate cancellation when the parent HTTP handler returns.
	ctx, cancel := context.WithTimeoutCause(context.WithoutCancel(ctx), time.Second*10, errors.New("legacy create timeout"))
	defer cancel()

	startLegacy := time.Now()
	_, errObjectSt := d.Legacy.Create(ctx, obj, createValidation, options)
	d.recordLegacyDuration(errObjectSt != nil, mode3Str, options.Kind, method, startLegacy)
}()

Prompt for LLM

File pkg/apiserver/rest/dualwriter_mode3.go:

Line 50 to 57:

WHAT: The parent HTTP request context `ctx` is passed directly into fire-and-forget goroutines for legacy storage operations.
WHY: When the primary synchronous storage operation completes, the HTTP handler returns and cancels `ctx`, instantly aborting the background legacy storage operations before they can complete.
HOW: Use `context.WithoutCancel(ctx)` (if Go 1.21+) or `context.Background()` to detach the context from the parent request lifecycle. This applies to Create, Update, Delete, and DeleteCollection.

Suggested Code:

	go func() {
		// Use context.WithoutCancel to prevent immediate cancellation when the parent HTTP handler returns.
		ctx, cancel := context.WithTimeoutCause(context.WithoutCancel(ctx), time.Second*10, errors.New("legacy create timeout"))
		defer cancel()

		startLegacy := time.Now()
		_, errObjectSt := d.Legacy.Create(ctx, obj, createValidation, options)
		d.recordLegacyDuration(errObjectSt != nil, mode3Str, options.Kind, method, startLegacy)
	}()

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​