[shruthi713]

What

Four failure-path fixes in the task worker client and handler. None of them changes behaviour on the success path.

1. RemoteTaskExecutor restores the user credential when a call fails. runTask swaps the caller's credential for an encrypted copy before each attempt, but it only put the original back after a response arrived. If execute threw (connection refused, timeout, a 503 from RemoteClient), the encrypted copy stayed in SecurityRequestContext. The next retry then encrypted the ciphertext again, and the caller was left holding the wrong credential. The restore now happens in a finally.

2. TaskWorkerHttpHandlerInternal#run releases the slot exactly once. There were three bugs here:

   • the slot leaked when the task threw an Error;
   • it leaked when sending the error response threw;
   • it was released a second time when clearing the GCP metadata context threw after the task had already been reported.

   Reading the request and launching the task are now separate blocks, and every failure goes through one helper (failTask) that releases the slot even if the response can't be sent.

3. An isolated worker restarts after a task that threw. The handler's comment says a task that throws may have run user code and the runner must terminate. But the failure was reported with a null request, so the completion consumer had no class name and skipped the restart. The failure metrics were also tagged with an empty class. The request is now passed through.

4. The worker restarts when the credential wipe fails. If clearGcpMetadataTaskContext fails, the namespace's service-account credential stays on the pod, and the next task, from any namespace, runs with it. The pod is now marked for restart. If no task is running, it's stopped straight away: a failed task has already run its completion check, so nothing else would act on the flag.

Why now

We found (1) in a load test. The RBAC task worker stack (#16225, #16226, #16229, #16230, #16231, #16232) has the proxy answer 503 while a task worker restarts, and RemoteClient throws on 503. On 2026-10-07, all 25 task workers on a test instance did their periodic restart within about 20 minutes. One 50-user run hit 168 x 400 on pipeline deploy. AppFabric logged 309 x:

GET /v3Internal/namespaces/ns1/credentials/workloadIdentity/provision
AccessException: Failed to decrypt credential in principle: Failed to decrypt: decryption failed

That is the double-encrypted credential reaching the worker. The bug is on develop today too: any execute failure that gets retried (for example NoRouteToHostException) triggers it. The new 503 path just makes it common.

(2)-(4) turned up while reviewing the same handler.

Merge order

This should merge before or together with the RBAC task worker stack. Without (1), the stack's 503-on-restart behaviour turns every worker restart under load into a burst of credential failures.

Testing

• RemoteTaskExecutorTest#testUserCredentialIsRestoredAfterFailedAttempts: discovery points at a closed port, so every attempt fails inside execute() after encryption. The test asserts that the caller gets back the same credential object, that there was more than one attempt, and that every attempt encrypted the plaintext, never the ciphertext.
• TaskWorkerFailureHandlingTest (new):
  • testErrorFromTaskReleasesTheSlot
  • testRepeatedErrorsDoNotWedgeThePod
  • testUnparseableRequestReleasesTheSlot
  • testFailedErrorResponseReleasesTheSlotOnce
  • testFailedCredentialWipeReleasesTheSlotOnce
  • testTaskThatThrowsCountsTowardTheRestart
  • testFailedCredentialWipeRestartsThePod
  • testFailedCredentialWipeDefersStopWhileATaskRuns
• Load test (2026-10-07): this change merged into the RBAC stack's integration branch, on the same instance and settings as the run quoted above (50 users, 25 task workers):
  • 6 regular runs: 0 Failed to decrypt credential, 0 failed credentials/workloadIdentity/provision calls. Same setup without the fix: 309 decryption failures in one run.
  • 1 extra run that deleted all 25 task workers one by one, every 15s, mid-run: also 0 decryption failures.
  • The remaining deploy failures are two errors on fresh task workers: concurrent artifact unpacks colliding (Directory not empty) and the first GCS read timing out. In regular runs they only hit the first ~30s of the first run; in the restart-wave run they followed the restarted workers. The non-RBAC instance shows the same first-run burst, so they aren't related to this change.

Behaviour change

• Success path: unchanged.
• Failure paths, intentionally changed:
  • slot accounting is now correct;
  • an isolated worker now restarts after a task that threw, as its comment always said it should;
  • a failed credential wipe now restarts the pod instead of leaving the credential in place.

[gemini-code-assist]

Code Review

This pull request improves task execution robustness and security by ensuring user credentials are consistently restored on failures, task slots are reliably released across all failure modes (including JVM Errors), and the GCP metadata context is safely cleared or triggers a pod restart. Comprehensive unit tests are added to verify these failure paths. The review feedback suggests adding a null check for runnableTaskRequest.getParam() to prevent a potential NullPointerException if the parameter is omitted in the request payload.

[gemini-code-assist]

Add a null check for runnableTaskRequest.getParam() before calling getEmbeddedTaskRequest() to prevent a potential NullPointerException if the parameter is omitted or null in the request payload.

Suggested change

	if (runnableTaskRequest.getParam().getEmbeddedTaskRequest() != null) {
	if (runnableTaskRequest.getParam() != null && runnableTaskRequest.getParam().getEmbeddedTaskRequest() != null) {

[shruthi713]

I'll leave this as is. The line is unchanged from develop (it's at L233 there), and a null param can't come from our own callers: RunnableTaskRequest's only constructor always sets this.param = new RunnableTaskParam(param, embeddedTaskRequest), so it's only null for a hand-built JSON payload. That case is already handled. This line sits inside the request-reading try, whose catch (Exception ex) at L260 answers 500 and releases the slot through failTask. testUnparseableRequestReleasesTheSlot checks that path, including the 500.

[shruthi713]

Closing in favour of #16231. The RBAC task worker stack changes the same code in RemoteTaskExecutor and TaskWorkerHttpHandlerInternal, so these four fixes are now the first five commits of #16231 (unchanged, including the review fixes), with the description and load-test results carried over under "Bug fixes". They stay ungated there, so they still apply with RBAC or the task worker manager off.