jiangzho opened a new pull request, #973:
URL: https://github.com/apache/spark-kubernetes-operator/pull/973

   ### What changes were proposed in this pull request?
   
   This adds a new configuration option,
   
`spark.kubernetes.operator.api.secondaryResourceCreateMaxBlockingDelayMillis` 
(API_SECONDARY_RESOURCE_CREATE_MAX_BLOCKING_DELAY_MILLIS, default 3000ms): a 
computed delay at or below it is still slept inline as before, but a longer 
delay now throws a new SecondaryResourceBackoffException instead of blocking. 
AppInitStep catches it ahead of its existing catch (Exception e) and requeues 
the reconcile after the delay via ReconcileProgress.completeAndRequeueAfter, 
without moving the application to SchedulingFailure. A small in-memory 
SecondaryResourceRetryTracker (keyed per secondary resource, lazily evicted 
after 30 minutes) carries the attempt count across the parked reconcile so the 
exponential backoff keeps escalating instead of resetting to attempt 1 each 
time.
   
   As part of this change, AppInitStep.reconcile's resource-creation loop 
(pre-resources, driver pod, driver resources) was extracted into a new private 
requestDriverAndResources method to keep the method within the project's 
statement-count (PMD NcssCount) limit.
   
   ### Why are the changes needed?
   
   Under sustained API server pressure (e.g. a 429 storm), several 
SparkApplications can hit a retryable error at the same time. Previously, each 
one could tie up one of the operator's shared reconciler threads for the full 
backoff duration, which can starve reconciliation of unrelated 
SparkApplication/SparkCluster resources exactly when the operator most needs 
free threads to back off cleanly.
   
   Under default backoff settings (initialBackoff=1000ms, multiplier=2.0, 
maxAttempts=3), the exponential schedule never exceeds ~2500ms (including 
jitter), which stays below the new 3000ms default threshold -- so this change 
is a no-op for the common case and only takes effect for a genuinely long wait: 
a server-requested Retry-After, or a non-default maxAttempts/initialBackoff/ 
multiplier configuration that pushes the exponential schedule past the 
threshold.
   
   `API_SECONDARY_RESOURCE_CREATE_MAX_BACKOFF_MILLIS` (default 40s), which caps 
the computed delay itself before this new threshold is applied, is unchanged 
and still bounds unbounded exponential growth and a server-supplied Retry-After 
regardless of how the delay is applied. It is not retuned in this PR, but a 
long wait is now much cheaper than before, since it parks the reconcile instead 
of occupying one of the operator's shared reconciler threads -- raising its 
default could be considered separately in a future change.
   
   ### Does this PR introduce any user-facing change?
   
   Yes, but not a change to any existing default behavior: it adds one new 
configuration option 
(spark.kubernetes.operator.api.secondaryResourceCreateMaxBlockingDelayMillis, 
default 3000ms) and, only when the computed backoff delay exceeds it, the 
application now stays in its current state and reconciles again after that 
delay instead of blocking the reconciler thread in place.
   
   ### How was this patch tested?
   
   Updated 
SparkAppReconcilerTest#secondaryResourceCreationRetriesWithBackoffOnConflict: 
its 5000ms initial backoff now exceeds the default threshold, so it asserts 
that ReconcilerUtils.getOrCreateSecondaryResource throws 
SecondaryResourceBackoffException with the expected requeue delay instead of 
blocking and retrying in place.
   
   Existing ReconcilerUtilsTest and BackoffUtilsTest wall-clock/backoff tests 
continue to pass unmodified, since their configured delays (~1000ms) stay below 
the new default threshold.
   
   Added new tests:
   - 
ReconcilerUtilsTest#throwsBackoffExceptionInsteadOfSleepingAboveTheBlockingThreshold:
 a delay above the threshold throws immediately (no sleep) with the expected 
resource, cause, attemptCount and requeueAfter.
   - 
ReconcilerUtilsTest#parkedAttemptCountEscalatesAcrossReconcilesInsteadOfResetting:
 calling getOrCreateSecondaryResource again for the same resource after a park 
resumes the attempt count instead of restarting at 1.
   - 
ReconcilerUtilsTest#parkedAttemptCountDoesNotCarryOverAfterASubsequentSuccess: 
a success in between two parked attempts clears the tracked count, so the next 
failure starts over at 1.
   - AppInitStepTest#driverCreationBackoffAboveThresholdIsParkedNotFailed: 
exercises the real integration path (not just ReconcilerUtils in isolation) — a 
driver pod creation backoff above the threshold is caught by AppInitStep ahead 
of its generic catch (Exception e) and returned as a plain requeue, with the 
application left in Submitted rather than moved to SchedulingFailure.
   
   Each new backoff/park test builds its own uniquely-named Pod, since 
SecondaryResourceRetryTracker is a static, process-wide map and a shared 
literal pod identity (e.g. "test-pod") across test classes was observed to leak 
a parked attempt count into unrelated tests depending on run order.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Co-authored-by: Claude Code (claude-sonnet-5-5)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to