Skip to content

Async function execution internals#1355

Open
TristonianJones wants to merge 5 commits into
cel-expr:masterfrom
TristonianJones:async-internals
Open

Async function execution internals#1355
TristonianJones wants to merge 5 commits into
cel-expr:masterfrom
TristonianJones:async-internals

Conversation

@TristonianJones

Copy link
Copy Markdown
Collaborator

Internal support for async function evaluation.

  • Declaration support with guard functions just like sync functions
  • The framework manages async function launches according to configurable concurrency limits
  • Async functions are provided the context.Context object and expected to block
    until a result is available and return it. The framework manages go routines

The implementation supports minimal call deduplication logic which users
will be able to extend with functionality introduced into later PRs.

@TristonianJones TristonianJones requested a review from l46kok June 22, 2026 22:35
Comment thread interpreter/async.go
Comment thread interpreter/async.go Outdated
Comment thread interpreter/async_test.go
Comment thread interpreter/async.go
@TristonianJones

Copy link
Copy Markdown
Collaborator Author

PTAL, thanks for the detailed review. I also added a NaN case to the async function call equality check.

Comment thread interpreter/async.go
Comment thread interpreter/async.go
Comment thread interpreter/async.go Outdated
go func() {
if semaphore != nil {
// Release the concurrency slot when this call finishes or is cancelled.
defer func() { <-semaphore }()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to also release the semaphore right before sending to the completion channel? defer runs at the very end of the goroutine right? If the semaphore is at capacity, there might be a small window where the next async call is rejected because defer hasn't ran.

@TristonianJones TristonianJones Jun 25, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we do. The completions signal indicates when a result is ready for re-evaluation (this will be more obvious in later PRs). The idea is that a batch of results are being accumulated and some logic is trying to decide whether to wait for all pending completions, a certain number of results, or a certain amount of time. The semaphore release at the end is, I think, the right thing to do since it should block new calls until the decision about whether re-evaluate is made.

@l46kok l46kok Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the semaphore release at the end, here's the scenario I'm wondering about, let me know if this sounds realistic to you. Suppose we have asyncA() + asyncB(), with semaphore capacity of 1. Rough timeline:

First evaluation pass:

  • asyncA() starts, acquires the semaphore.
  • asyncB() fails to acquire it then backs off. Evaluator now waits for completions channel.
  • asyncA() finishes. completions <- stateA is notified. Evaluator gets woken up. Note that semaphore isn't released at this time.

Second evaluation pass:

  • asyncA() is now cached so it's skipped, asyncB() starts, attempts to acquire the semaphore
  • asyncA()'s goroutine hasn't executed the defer <- semaphore yet. asyncB() fails to acquire the slot.
  • asyncA() gets done, asyncB() was rejected. The pass yields an unknown. At this point, frame.PendingAsyncCalls == 0. The evaluator prematurely aborts the evaluation.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the behavior was even worse than you thought-- turns out it would evaluate twice and hang. Yikes.

I made a mistake in how I thought about the flow. I introduced an asyncGate to async.go that groups the concepts together - acquire the semaphore, increment a count. complete the task, release the semaphore, decrement a count, notify on completions with the call id. Seems much cleaner now, but I may still have something off.

Comment thread interpreter/async.go
Comment thread interpreter/async.go Outdated
@TristonianJones

Copy link
Copy Markdown
Collaborator Author

PTAL and let me know what you think regarding the last open comment about the semaphore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants