Skip to content

[Yield] Support invocations to yield on transient errors#4725

Open
AhmedSoliman wants to merge 1 commit into
mainfrom
pr4725
Open

[Yield] Support invocations to yield on transient errors#4725
AhmedSoliman wants to merge 1 commit into
mainfrom
pr4725

Conversation

@AhmedSoliman
Copy link
Copy Markdown
Contributor

@AhmedSoliman AhmedSoliman commented May 12, 2026

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 12, 2026

Test Results

  8 files  ± 0    8 suites  ±0   2m 43s ⏱️ - 2m 6s
 43 tests  - 17   43 ✅  - 17  0 💤 ±0  0 ❌ ±0 
186 runs   - 81  186 ✅  - 81  0 💤 ±0  0 ❌ ±0 

Results for commit 96b03fe. ± Comparison against base commit 5ea7c39.

This pull request removes 17 tests.
dev.restate.sdktesting.tests.Combinators ‑ awakeableOrTimeoutUsingAwaitAny(Client)
dev.restate.sdktesting.tests.Combinators ‑ firstSuccessfulCompletedAwakeable(Client)
dev.restate.sdktesting.tests.Combinators ‑ mixedAllCompletedRunSignalAndFailedRun(Client)
dev.restate.sdktesting.tests.Combinators ‑ mixedAllSucceededRunAndSignal(Client)
dev.restate.sdktesting.tests.Combinators ‑ mixedRunWinsRaceAgainstSignal(Client)
dev.restate.sdktesting.tests.Combinators ‑ mixedSignalWinsAfterRunFails(Client)
dev.restate.sdktesting.tests.Combinators ‑ signalAllCompleted(Client)
dev.restate.sdktesting.tests.Combinators ‑ signalAllSuccessfulOrFirstFailed(Client)
dev.restate.sdktesting.tests.Combinators ‑ signalFirstCompleted(Client)
dev.restate.sdktesting.tests.Combinators ‑ signalFirstSuccessfulOrAllFailed(Client)
…

♻️ This comment has been updated with latest results.

@AhmedSoliman AhmedSoliman changed the title [Yield][1/N] Introduce Effect Yield for errors [Yield] Support invocations to yield on transient errors May 13, 2026
@AhmedSoliman AhmedSoliman marked this pull request as ready for review May 13, 2026 15:03
@AhmedSoliman AhmedSoliman requested a review from tillrohrmann May 13, 2026 16:41
@AhmedSoliman AhmedSoliman force-pushed the pr4725 branch 2 times, most recently from b824fb7 to c8ebcb2 Compare May 13, 2026 17:09
@AhmedSoliman AhmedSoliman force-pushed the pr4725 branch 2 times, most recently from 1b4b644 to 3c4e111 Compare May 14, 2026 10:24
@AhmedSoliman AhmedSoliman force-pushed the pr4725 branch 4 times, most recently from a0d3f06 to 90f61fc Compare May 18, 2026 08:06
Copy link
Copy Markdown
Contributor

@tillrohrmann tillrohrmann left a comment

Choose a reason for hiding this comment

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

Great work @AhmedSoliman 🚀 Looking forward to finally releasing invoker slots when retrying for prolonged times :-) LGTM. +1 for merging.


use restate_util_time::{FriendlyDuration, NonZeroFriendlyDuration};

const DEFAULT_INVOCATION_YIELD_THRESHOLD: FriendlyDuration = FriendlyDuration::from_secs(2);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A comment in the invocation state machine said that the threshold is 1 second.

bilrost::empty_state_via_default!(ByteCount<true>);

bilrost::delegate_proxied_encoding!(
use encoding (bilrost::encoding::Varint)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are we encoding ByteCount as a varint? Is the space saving benefit big enough for the added decoding cycles? Or is it because of wire compatibility with another format?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

bilrost::empty_state_via_for_overwrite!(ByteCount<false>);

bilrost::delegate_proxied_encoding!(
use encoding (bilrost::encoding::Varint)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same question here.

Full implementation for yielding invocations from the invoker back to the vqueues scheduler. This unifies with the existing support for memory-budget-based invocation yielding.
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