Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] Simplify proper usage of AsyncLocalStorage in Workflow context #1432

Open
mjameswh opened this issue May 30, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@mjameswh
Copy link
Contributor

Description

AsyncLocalStorage are frequently used inside of Workflow context, for example to implement correlation/tracing/context propagation mechanisms. However, most users don't know (or forget) that ALS must be "disabled" when they are no longer required (i.e. when the workflow execution gets evicted from cache). Otherwise, they do retain heap and still get called for every single AsyncResource (e.g. mostly Promise in this context) creation.

Given that we already control access to the AsyncLocalStorage in Workflow context, it would be easy for the SDK to track instantiation of AsyncLocalStorage, and automatically dispose them when the execution context is about to be destroyed.

We also need to investigate either usage of ALS inside of Workflow Context is efficient. Since ALS hooks work at the Worker Thread level, each ALS instance “tags” (i.e. adds a [kResourceStore] property on) every single AsyncResource that gets created in that Worker Thread, without regard to which VM the ALS or the AsyncResource was created in.

This could possibly mean that, even when ALS get disposed properly, using ALS inside workflows may currently add a memory and performance penalty that grows exponentially with the number of currently cached workflows. Based on memory snapshots provided by a customer, the constant multiplier here would be relatively small (48 bytes per “tag” in >99% of cases), and therefore, this problem may not be immediately apparent, but could still have sizable impact on Workflow Worker where maxCachedWorkflow is high.

Again, given that we already control access to the AsyncLocalStorage in Workflow context, it would be possible for the SDK to provide a custom implementation of ALS that is workflow-aware and/or that scales better.

@mjameswh mjameswh added the enhancement New feature or request label May 30, 2024
@bergundy bergundy changed the title [Feature Request] Simply proper usage of AsyncLocalStorage in Workflow context [Feature Request] Simplify proper usage of AsyncLocalStorage in Workflow context May 30, 2024
@neelance
Copy link

neelance commented Nov 3, 2024

Please see #1557.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants