search

temporalio / features

Behavior and history compatibility testing for Temporal SDKs
13 stars 16 forks source link

SDKs should expose history length and size via Workflow info #16

Open bergundy opened 2 years ago

bergundy commented 2 years ago

History length:

History size and "should continue as new":

lorensr commented 2 years ago

Related:

We should give Workflow.isContinueAsNewNeeded flag to the SDKs. And it should be coming from the service.

https://temporaltechnologies.slack.com/archives/C01FT8U10GK/p1643400086752909?thread_ts=1643398281.925129&cid=C01FT8U10GK

lorensr commented 1 year ago

I think we should expose shouldContinueAsNew as well in all SDKs.

  1. If server is able to take into account other things, then it's better than history length/size
  2. This:
if (workflowInfo().shouldContinueAsNew) {
  cAN()
}

is shorter & easier to read/understand compared to:

if (workflowInfo().historyLength < 10_000 && workflowInfo().historySize < 10_000_000) {
  cAN()
}
  1. We'll no longer need to teach people the right numbers to use
cretz commented 1 year ago

I think we should expose shouldContinueAsNew as well in all SDKs.

Concur, when available in server (no benefit in supporting before server). Or is it already available in server? What is the logic there and is it configurable?

mjameswh commented 1 year ago

It will be important to mention in documentation the fact that relying on these properties is useless and dangerous if using an older server.

cretz commented 1 year ago

We could make it a capability and error (i.e. wft failure) if invoked for older server. But I think this is a good enough reason to not even expose this until it is available on cloud.

cretz commented 1 year ago

Note, we should call this "continue as new suggested" across all SDKs I think.

cretz commented 1 year ago

A caveat that SDKs need to note here: these three values may be delayed if viewed via query. Since the value is on wft start, a query-only poll response won't have updated history. So if you, say, sent a signal to start far-out 50 timers, and then, even after already in history stored, sent just a query, there is no new wft and therefore no value updates even though current event count is actually quite different on the server.

Basically every SDK needs to document that these values may be delayed if accessed in a query.