[ChickenBenny]

Tracking issue

Closes #7916

Why are the changes needed?

When a dynamic workflow's compiled CRD (futures_compiled.pb) is larger than the storage cache's per-entry limit (1/1024 of storage.cache.max_size_mbs), propeller recompiles the dynamic workflow on every evaluation round.

cachedRawStore.ReadRaw returns the data together with CACHE_WRITE_FAILED when the blob store read succeeds but the cache write fails. ReadProtobuf treats that as a soft failure, but getRawBytes in remote_workflow_store.go returns early on any error and drops the data. RetrieveCache then fails, and buildContextualDynamicWorkflow falls back to recompiling. The blob is the same size every round, so this keeps happening until the dynamic node finishes.

What changes were proposed in this pull request?

• getRawBytes: ignore CACHE_WRITE_FAILED like ReadProtobuf does, and return an error if the reader is nil.
• PutFlyteWorkflowCRD: ignore CACHE_WRITE_FAILED like WriteProtobuf does. The blob is already written at that point, but the error caused a misleading Failed to cache Dynamic workflow log every round.

How was this patch tested?

Added Test_cacheFlyteWorkflowLargerThanCacheEntry. It uses a 1MB storage cache and a 100-node CRD that is larger than a cache entry:

• put and get succeed and return the same workflow
• getting a missing reference still returns an error

The new test fails on the base commit with [CACHE_WRITE_FAILED] Failed to Cache the metadata and passes with this change:

cd flytepropeller
go test ./pkg/controller/nodes/task/ -run 'Test_cacheFlyteWorkflow' -v
go test ./pkg/controller/nodes/...

I also reproduced it end to end on the flytectl demo sandbox (v1.16.9) with a @dynamic that fans out 20 tasks, each sleeping 20s. With storage.cache.max_size_mbs: 1, the "this will cause the dynamic workflow to be recompiled" warning showed up 31 times in one run. With 100 it showed up 0 times.

Check all the applicable boxes

• I updated the documentation accordingly.
• All new and existing tests passed.
• All commits are signed-off.

Related PRs

None.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.34%. Comparing base (f4d63a7) to head (4452ea1).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8130      +/-   ##
==========================================
+ Coverage   57.33%   57.34%   +0.01%     
==========================================
  Files         931      931              
  Lines       58331    58336       +5     
==========================================
+ Hits        33445    33454       +9     
+ Misses      21833    21831       -2     
+ Partials     3053     3051       -2     

Flag	Coverage Δ
unittests-datacatalog	53.62% <ø> (ø)
unittests-flyteadmin	53.26% <ø> (ø)
unittests-flytecopilot	48.05% <ø> (ø)
unittests-flytectl	64.11% <ø> (ø)
unittests-flyteidl	76.63% <ø> (ø)
unittests-flyteplugins	60.64% <ø> (ø)
unittests-flytepropeller	53.88% <100.00%> (+0.03%)	⬆️
unittests-flytestdlib	64.41% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[ChickenBenny]

Hi @Sovietaced @pingsutw, could one of you take a look when you have a moment?

This fixes #7916: dynamic workflows whose compiled CRD is larger than a storage-cache entry get recompiled on every evaluation round, because getRawBytes drops the data when the read succeeds but the cache write fails (CACHE_WRITE_FAILED). The fix aligns getRawBytes / PutFlyteWorkflowCRD with how ReadProtobuf / WriteProtobuf already treat that error, and adds a regression test that fails on the base commit.

It's a small change (2 files in flytepropeller/pkg/controller/nodes/task). Thanks!

[AdilFayyaz]

LGTM