Add wait for cache expiry #3040

KodaiD · 2025-10-09T06:42:01Z

Description

This PR adds waits for cache expiry in integration test cases for administrative operations.

Currently, some admin test cases fail due to stale table metadata. And it is difficult to invalidate them, especially in a ScalarDB Cluster.

I and @brfrn169 discussed it, and decided to make the expiration time short and insert waits for expiry.

Related issues and/or PRs

Add alter column type #3028

Changes made

Set "1" for the table metadata cache expiration time in Core integration tests.
Insert waits (1-second sleep) for table metadata cache expiry into the following cases:
- alterColumnType_WideningConversion_ShouldAlterColumnTypesCorrectly
- alterColumnType_Oracle_WideningConversion_ShouldAlterColumnTypesCorrectly

Checklist

I have commented my code, particularly in hard-to-understand areas.
I have updated the documentation to reflect the changes.
I have considered whether similar issues could occur in other products, components, or modules if this PR is for bug fixes.
Any remaining open issues linked to this PR are documented and up-to-date (Jira, GitHub, etc.).
Tests (unit, integration, etc.) have been added for the changes.
My changes generate no new warnings.
Any dependent changes in other PRs have been merged and published.

Additional notes (optional)

N/A

Release notes

N/A

KodaiD · 2025-10-09T06:42:19Z

/gemini review

gemini-code-assist

Code Review

This is a great pull request that addresses the flakiness in integration tests by waiting for cache expiry. The approach of shortening the cache expiration time and adding a wait is a practical solution for the problem of stale metadata in a clustered environment. The refactoring in the test methods to use a single try-with-resources block is also a nice improvement that simplifies the code. I have a couple of suggestions to make the tests even more robust and the code more maintainable.

...est/java/com/scalar/db/storage/jdbc/ConsensusCommitAdminIntegrationTestWithJdbcDatabase.java

core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcAdminIntegrationTest.java

...alar/db/storage/jdbc/SingleCrudOperationTransactionAdminIntegrationTestWithJdbcDatabase.java

...ntegration-test/java/com/scalar/db/transaction/jdbc/JdbcTransactionAdminIntegrationTest.java

...gration-test/src/main/java/com/scalar/db/api/DistributedStorageAdminIntegrationTestBase.java

...ion-test/src/main/java/com/scalar/db/api/DistributedTransactionAdminIntegrationTestBase.java

core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraEnv.java

KodaiD · 2025-10-09T06:53:45Z

...est/java/com/scalar/db/storage/jdbc/ConsensusCommitAdminIntegrationTestWithJdbcDatabase.java

-      throws ExecutionException, IOException, TransactionException {
-    try {
+      throws ExecutionException, TransactionException {
+    try (DistributedTransactionManager manager = transactionFactory.getTransactionManager()) {


Remove unnecessary IOException.

KodaiD · 2025-10-09T06:56:34Z

...est/java/com/scalar/db/storage/jdbc/ConsensusCommitAdminIntegrationTestWithJdbcDatabase.java

              .addClusteringKey("c2", Scan.Ordering.Order.ASC);
      TableMetadata expectedTableMetadata = expectedTableMetadataBuilder.build();
      assertThat(admin.getTableMetadata(namespace1, TABLE4)).isEqualTo(expectedTableMetadata);



Fix to use the same DistributedTransactionManager. In the previous code, a different manager is used to expire metadata cache. But this technique cannot resolve the issue in ScalarDB Cluster side.

KodaiD · 2025-10-09T07:00:10Z

core/src/integration-test/java/com/scalar/db/storage/cassandra/CassandraEnv.java

+    // Metadata cache expiration time
+    properties.setProperty(DatabaseConfig.METADATA_CACHE_EXPIRATION_TIME_SECS, "1");


The short expiration time (i.e., 1) can test both where the data is in cache and where it is not.

Copilot

Pull Request Overview

This PR addresses test failures caused by stale table metadata in administrative operations by setting short cache expiration times and adding explicit waits for cache expiry in integration tests.

Sets metadata cache expiration time to 1 second across all Core integration test environments
Adds 1-second sleep waits in specific test methods after column type alterations to allow cache expiry
Refactors resource management to use try-with-resources pattern for proper cleanup

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
DistributedTransactionAdminIntegrationTestBase.java	Adds cache expiry wait and refactors to try-with-resources
DistributedStorageAdminIntegrationTestBase.java	Adds cache expiry wait and refactors to try-with-resources
JdbcTransactionAdminIntegrationTest.java	Adds cache expiry wait and refactors to try-with-resources
SingleCrudOperationTransactionAdminIntegrationTestWithJdbcDatabase.java	Adds cache expiry wait and refactors to try-with-resources
JdbcAdminIntegrationTest.java	Adds cache expiry wait and refactors to try-with-resources
ConsensusCommitAdminIntegrationTestWithJdbcDatabase.java	Adds cache expiry wait and refactors to try-with-resources
CassandraEnv.java	Sets metadata cache expiration time to 1 second
CosmosEnv.java	Sets metadata cache expiration time to 1 second
DynamoEnv.java	Sets metadata cache expiration time to 1 second
JdbcEnv.java	Sets metadata cache expiration time to 1 second
MultiStorageEnv.java	Sets metadata cache expiration time to 1 second
MultiStorageAdminIntegrationTest.java	Sets metadata cache expiration time to 1 second
MultiStorageIntegrationTest.java	Sets metadata cache expiration time to 1 second
MultiStorageMutationAtomicityUnitIntegrationTest.java	Sets metadata cache expiration time to 1 second
MultiStorageSchemaLoaderIntegrationTest.java	Sets metadata cache expiration time to 1 second
ConsensusCommitSpecificIntegrationTestWithMultiStorage.java	Sets metadata cache expiration time to 1 second
ConsensusCommitNullMetadataIntegrationTestWithMultiStorage.java	Sets metadata cache expiration time to 1 second

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

...ntegration-test/java/com/scalar/db/transaction/jdbc/JdbcTransactionAdminIntegrationTest.java

...alar/db/storage/jdbc/SingleCrudOperationTransactionAdminIntegrationTestWithJdbcDatabase.java

...est/java/com/scalar/db/storage/jdbc/ConsensusCommitAdminIntegrationTestWithJdbcDatabase.java

brfrn169

LGTM! Thank you!

KodaiD · 2025-10-09T09:41:05Z

core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlite.java

    throw new UnsupportedOperationException(
-        CoreError.JDBC_SQLITE_ALTER_COLUMN_TYPE_NOT_SUPPORTED.buildMessage(
-            from.toString(), to.toString()));
+        CoreError.JDBC_SQLITE_ALTER_COLUMN_TYPE_NOT_SUPPORTED.buildMessage());


Not directly related to this PR, but these arguments are unnecessary.

komamitsu

LGTM! 👍

Add wait for cache expiry

5e3a93e

KodaiD self-assigned this Oct 9, 2025

gemini-code-assist bot reviewed Oct 9, 2025

View reviewed changes

KodaiD commented Oct 9, 2025

View reviewed changes

KodaiD marked this pull request as ready for review October 9, 2025 07:00

Copilot AI review requested due to automatic review settings October 9, 2025 07:00

Copilot AI reviewed Oct 9, 2025

View reviewed changes

KodaiD requested review from a team, Torch3333, brfrn169, feeblefakie and komamitsu and removed request for a team October 9, 2025 07:03

brfrn169 approved these changes Oct 9, 2025

View reviewed changes

Fix to remove unnecessary arguments

bbd0c31

KodaiD commented Oct 9, 2025

View reviewed changes

komamitsu approved these changes Oct 10, 2025

View reviewed changes

		// Metadata cache expiration time
		properties.setProperty(DatabaseConfig.METADATA_CACHE_EXPIRATION_TIME_SECS, "1");

Add wait for cache expiry #3040

Are you sure you want to change the base?

Add wait for cache expiry #3040

Uh oh!

Conversation

KodaiD commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues and/or PRs

Changes made

Checklist

Additional notes (optional)

Release notes

Uh oh!

KodaiD commented Oct 9, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

KodaiD Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

KodaiD Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

KodaiD Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brfrn169 left a comment

Choose a reason for hiding this comment

Uh oh!

KodaiD Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

komamitsu left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

KodaiD commented Oct 9, 2025 •

edited

Loading