Skip to content

Conversation

@lucymcnatt
Copy link
Contributor

Description

Release Notes Confirmation

CHANGELOG.md

  • I updated CHANGELOG.md in this PR
  • I assert that this change shouldn't be included in CHANGELOG.md because it doesn't impact community users

Terra Release Notes

  • I added a suggested release notes entry in this Jira ticket
  • I assert that this change doesn't need Jira release notes because it doesn't impact Terra users

@lucymcnatt lucymcnatt marked this pull request as ready for review December 9, 2025 19:40
@lucymcnatt lucymcnatt requested a review from a team as a code owner December 9, 2025 19:40
@lucymcnatt lucymcnatt changed the title [CTM-133] Reverse call-caching order and delete old entries [CTM-133] Reverse call-caching order and not checking old entries Dec 10, 2025
Copy link
Contributor

@LizBaldo LizBaldo left a comment

Choose a reason for hiding this comment

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

The changes look good to me, I only have a couple of minor comments. I am guessing that a few integration tests will need to be updated if they expected older cache hits? I suspect the AWS job queue is stuck by a misconfigured resource job as well if the tests are timing out

@lucymcnatt lucymcnatt changed the title [CTM-133] Reverse call-caching order and not checking old entries [CTM-133] Reverse call-caching order and not check old entries Dec 12, 2025
@LizBaldo LizBaldo self-requested a review December 15, 2025 22:01
Copy link
Collaborator

@aednichols aednichols left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

Deployment note:

We should benchmark how long this Liquibase takes in production.

I believe recent versions of MySQL support instant add columns, but we should verify that it works in practice on our instance, with this particular changeset.

My clone is still around, so that would be a good place to try it.

@aednichols
Copy link
Collaborator

It looks like at least one call caching test is failing, I would recommend running it locally to observe how it interacts with the database.

Metadata mismatch for calls.call_cache_cha_cha.read_files_failOnStderr_expression.callCaching.hit - expected: true but got: false

As well as some sort of column type semantics in the DBMS suite:

[info] LiquibaseComparisonSpec:
[info] Liquibase Comparison for Engine MySQL 
[info] - should match the Slick schema for column CALL_CACHING_ENTRY.CREATED_AT *** FAILED *** (1 millisecond)
[info]   for type TIMESTAMP(default = CURRENT_TIMESTAMP) vs. TIMESTAMP(default = null): ColumnType("TIMESTAMP", None) was not equal to ColumnType("DATETIME", None) (LiquibaseComparisonSpec.scala:93)

@aednichols aednichols mentioned this pull request Dec 16, 2025
4 tasks
@lucymcnatt
Copy link
Contributor Author

Looks good to me 👍

Deployment note:

We should benchmark how long this Liquibase takes in production.

I believe recent versions of MySQL support instant add columns, but we should verify that it works in practice on our instance, with this particular changeset.

My clone is still around, so that would be a good place to try it.

Tried it on your clone and it took ~0.5 secs, so I think we should be good!

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.

3 participants