Skip to content

Conversation

@wxing1292
Copy link
Contributor

What changed?

  • Set Cassandra record max TTL to be 10 years

Why?
cadence-workflow/cadence#3627

How did you test it?
DB integration tests

Potential risks
N/A

@wxing1292 wxing1292 requested review from a team and samarabbas October 12, 2020 23:26
@CLAassistant
Copy link

CLAassistant commented Oct 12, 2020

CLA assistant check
All committers have signed the CLA.


maxCassandraTTL = int64(630720000) // Cassandra TTL maximum, 20 years in second
// ref: https://docs.datastax.com/en/dse-trblshoot/doc/troubleshooting/recoveringTtlYear2038Problem.html
maxCassandraTTL = int64(315360000) // Cassandra max support time is 2038-01-19T03:14:06+00:00. Updated this to 10 years to support until year 2028
Copy link
Contributor

@alexshtin alexshtin Oct 13, 2020

Choose a reason for hiding this comment

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

Why not:

func maxCassandraTTL() int64 { 
    return (time.Date(2038,01,19,03,14,06,0,time.UTC).Sub(time.Now().UTC())).Milliseconds()
}

Copy link
Contributor

Choose a reason for hiding this comment

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

We have default timeouts of 10 years.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

depending on whether we are still going to support Cassandra after 10 years
the solution from uber is much more simpler.

@samarabbas what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed with @wxing1292. The main issue is we need a concept of infinite timeout in Temporal. Using a default of 10 years introduces a scalability bottleneck. Here are couple of issues tracking this:
#358
#573

Copy link
Contributor

Choose a reason for hiding this comment

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

This is not even 10 but 8 years. I don't understand why they picked up some random number instead of properly set to the real limit Cassandra gives us. It is used in 2 places only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well, I guess as long as the time is really long (far in the future), then it is ok? BTW, cadence picked 5 years...

@samarabbas samarabbas merged commit ec220ca into temporalio:master Oct 19, 2020
@wxing1292 wxing1292 deleted the ttl branch October 21, 2020 00:08
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.

5 participants