Decide if plugin functions should have a context.Context

codefromthecrypt commented 11 months ago

What would you like to be added:

We should consider if context parameters should exist on plugin callbacks before formalizing the SDK.

Notes :

This is an SDK only issue because context does not pass through the wasm sandbox, nor is there any language independent way to propagate go's context (e.g. from the scheduler to wasm written in C).
This shouldn't start until we have all plugin functionality created, as otherwise we can't see holistic performance impact.

Why is this needed:

End users should comment directly what they want, so we can verify the expectation will work in TinyGo.

Completion requirements:

The completion of this issue depends one the decision:

If yes, we add context params and document how they are different than normal go context. We also have test cases and benchmarks around at least one user required application of context.
If no, we add into RATIONALE.md why we didn't, for example to reduce chance of mis-use at the cost of lack of parity between the framework and wasm SDKs.

This enhancement requires the following artifacts:

The artifacts should be linked in subsequent comments.

kerthcet commented 11 months ago

Some context here, for upstream kubernetes scheduler plugins, we general use context for three things, IIRC

parallelism to speed up the filtering process --> not a big deal I think because of the concurrency limitation of wasm
contextual logging --> Can't handle here
http request --> like binding here, but we can use a plain context instead. More often, we have plugins implementing filter & score extension points.

k8s-triage-robot commented 3 months ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

sanposhiho commented 3 months ago

/remove-lifecycle stale