Release executor slot if not executing

[yashnevatia]

Previously:

• (F+1)th request would execute and send responses to (F+1) requesters
• (F+2)nd request would have been blocked on the above. After acquiring lock, (F+2)nd request would have added itself to requester list and sent the requester the response set above.

Now:

• Use stateMux around requesterList, to let other requests add to requester list and exit (so that executing request can sendResponses to all requesters) and blocked requests dont occupy slots just to send response.
• Use executionClaimed atomic variable to enable only one request to execute.
• Use stateMux around response and error to avoid race when writing/reading between requests.
• So (F+2)nd comes in now:
  • if execution in progress: adds requester and releases slot, once execution is finished (F+1)th request will send response to all requesters
  • if execution is complete: it will send response to its own requester

How to handle Cancel():

• Previously:
  • If execution is in progress and Cancel() is called, it will be blocked on executing request. And there will also be 2F requests waiting on executing request. If response is set, it will be sent to all requesters, if not, cancel will send error to all requesters.
  • To replicate this, we use a execDone channel. If execution is in progress (i.e executionClaimed flag is true), we wait for it to finish (execDone channel will be closed by executing request once execution completes) and then we send timeout errors to requesters if response was not set by executing request.

[github-actions]

👋 yashnevatia, thanks for creating this pull request!

To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team.

Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks!

[github-actions]

✅ No conflicts with other open PRs targeting develop

[github-actions]

I see you updated files related to core. Please run make gocs in the root directory to add a changeset as well as in the text include at least one of the following tags:

• #added For any new functionality added.
• #breaking_change For any functionality that requires manual action for the node to boot.
• #bugfix For bug fixes.
• #changed For any change to the existing functionality.
• #db_update For any feature that introduces updates to database schema.
• #deprecation_notice For any upcoming deprecation functionality.
• #internal For changesets that need to be excluded from the final changelog.
• #nops For any feature that is NOP facing and needs to be in the official Release Notes for the release.
• #removed For any functionality/config that is removed.
• #updated For any functionality that is updated.
• #wip For any change that is not ready yet and external communication about it should be held off till it is feature complete.

[trunk-io]

   

Failed Test	Failure Summary	Logs
Test_CCIPProgrammableTokenTransfer_EVM2Sui_BurnMintTokenPool		Logs ↗︎

View Full Report ↗︎ ⋅ Docs

[dhaidashenko]

Introduced changes maintain the original behavior, but the original behavior does not make sense to me.

1. Cancel is expected to just send a signal for cancellation, not wait for completion of the corresponding task. If execution started before Cancel is called, we'll just wait for its completion; if it was successful, noop; if it failed, we override the error with a different one.
2. When we send cancel, we just notify requestors but do not cancel the capability call itself.

[dhaidashenko]

Same as above. nit: related to the original code.
We should minimize the places where setError is called. executeRequest should return the result and the error, and those should be set at a higher level

[cl-sonarqube-production]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube