Track `elapsed_compute` for `AsyncFuncExec`

[2010YOUY01]

Is your feature request related to a problem or challenge?

This is a follow on work for #19626

The above PR implemented BaselineMetrics tracking for AsyncFuncExec, except elapsed_compute, the reason is we have found it requires some additional change to make it work as expected, see the original discussion for details: #19626 (comment)

Describe the solution you'd like

No response

Describe alternatives you've considered

No response

Additional context

No response

[feniljain]

take

[feniljain]

Hey @2010YOUY01 👋🏻

I have been exploring this ticket, and I believe this would require a change to AsyncScalarUDFImpl itself? At the end of the chain, invoke_async_with_args is an async function too, so any driver code would await on it. As we don't want to include the polling time in our metrics, we would have to change the API of invoke_async_with_args to pass in a timer, which the user would have to manually start and stop in their implementation. If they don't, they would miss out on time.

I currently don't see any other way to force the user to make use of that. For measuring time spent in our driver code, I believe we can pass the timer around and keep taking measurements using intermediate method.

What do you think? Am I missing something? If we need to change the API, what according to you would be a good way to migrate all downstream consumers to the same?

[2010YOUY01]

Perhaps we can keep the existing API, and add an optional new API invoke_with_timer(), and async UDFs can optionally migrate to a compute time tracked version manually

[feniljain]

Yup, that's one way! In general, as it seems to be a bigger change, do you think we should involve someone else in the discussion too?

[2010YOUY01]

I'm not sure who might know this better, or we could leave this ticket for someone who has real usage for this feature 🤔 , if that’s not the case for you.

[feniljain]

Sure, that makes more sense! Will unassign myself :)

[feniljain]

untake