Skip to content

Fix SQL Server 2017 CI #2599

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 23, 2025
+70 −51
Merged

Fix SQL Server 2017 CI #2599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cb170fd
Fix
Torch3333 Apr 17, 2025
e27606a
Fix format [skip ci]
Torch3333 Apr 22, 2025
407c806
Add comments and fix ordering of condition
Torch3333 Apr 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 40 additions & 26 deletions .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ env:
ORG_GRADLE_PROJECT_integrationTestJavaRuntimeVersion: "${{ github.event_name != 'workflow_dispatch' && '8' || inputs.INT_TEST_JAVA_RUNTIME_VERSION }}"
ORG_GRADLE_PROJECT_integrationTestJavaRuntimeVendor: "${{ github.event_name != 'workflow_dispatch' && 'temurin' || inputs.INT_TEST_JAVA_RUNTIME_VENDOR }}"
# This variable evaluates to: if {!(Temurin JDK 8) && !(Oracle JDK)} then {true} else {false}
# Oracle JDK that are linux compatible and publicly available through direct download exist for all LTS versions
SET_UP_INT_TEST_RUNTIME_NON_ORACLE_JDK: "${{ (github.event_name == 'workflow_dispatch' && !(inputs.INT_TEST_JAVA_RUNTIME_VERSION == '8' && inputs.INT_TEST_JAVA_RUNTIME_VENDOR == 'temurin') && !(inputs.INT_TEST_JAVA_RUNTIME_VENDOR == 'oracle')) && 'true' || 'false' }}"
INT_TEST_GRADLE_OPTIONS_FOR_GROUP_COMMIT: '"-Dscalardb.consensus_commit.coordinator.group_commit.enabled=true" "-Dscalardb.consensus_commit.coordinator.group_commit.old_group_abort_timeout_millis=15000" --tests "**.ConsensusCommit**"'

Expand Down Expand Up @@ -244,9 +245,10 @@ jobs:
name: Cosmos DB integration test (${{ matrix.mode.label }})
runs-on: windows-latest
env:
# Official Oracle JDK images that are windows compatible and publicly available through direct download do not exist for JDK 8 and 11 so we use instead cached versions hosted on the Scalar container registry.
# This variable evaluates to: if {!(Temurin JDK 8) && !(Oracle JDK 8 or 11)} then {true} else {false}
SET_UP_INT_TEST_RUNTIME_JDK_WHEN_NOT_ORACLE_8_OR_11: "${{ (github.event_name == 'workflow_dispatch' && !(inputs.INT_TEST_JAVA_RUNTIME_VERSION == '8' && inputs.INT_TEST_JAVA_RUNTIME_VENDOR == 'temurin') && !(inputs.INT_TEST_JAVA_RUNTIME_VENDOR == 'oracle' && (inputs.INT_TEST_JAVA_RUNTIME_VERSION == '8' || inputs.INT_TEST_JAVA_RUNTIME_VERSION == '11'))) && 'true' || 'false' }}"
SET_UP_INT_TEST_RUNTIME_ORACLE_JDK_8_OR_11: "${{ (inputs.INT_TEST_JAVA_RUNTIME_VENDOR == 'oracle' && (inputs.INT_TEST_JAVA_RUNTIME_VERSION == '8' || inputs.INT_TEST_JAVA_RUNTIME_VERSION == '11')) && 'true' || 'false' }}"
SET_UP_INT_TEST_RUNTIME_ORACLE_JDK_8_OR_11: "${{ ((inputs.INT_TEST_JAVA_RUNTIME_VERSION == '8' || inputs.INT_TEST_JAVA_RUNTIME_VERSION == '11') && inputs.INT_TEST_JAVA_RUNTIME_VENDOR == 'oracle') && 'true' || 'false' }}"

strategy:
matrix:
Expand Down Expand Up @@ -1120,18 +1122,12 @@ jobs:

integration-test-for-jdbc-sqlserver-2017:
name: SQL Server 2017 integration test (${{ matrix.mode.label }})
runs-on: ubuntu-20.04

services:
sqlserver:
image: mcr.microsoft.com/mssql/server:2017-latest
env:
MSSQL_PID: "Express"
SA_PASSWORD: "SqlServer17"
ACCEPT_EULA: "Y"
ports:
- 1433:1433
options: --name sqlserver17
runs-on: windows-latest
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switch to a windows runner for SQL Server 2017

env:
# Official Oracle JDK images that are windows compatible and publicly available through direct download do not exist for JDK 8 and 11 so we use instead cached versions hosted on the Scalar container registry.
# This variable evaluates to: if {!(Temurin JDK 8) && !(Oracle JDK 8 or 11)} then {true} else {false}
SET_UP_INT_TEST_RUNTIME_JDK_WHEN_NOT_ORACLE_8_OR_11: "${{ (github.event_name == 'workflow_dispatch' && !(inputs.INT_TEST_JAVA_RUNTIME_VERSION == '8' && inputs.INT_TEST_JAVA_RUNTIME_VENDOR == 'temurin') && !(inputs.INT_TEST_JAVA_RUNTIME_VENDOR == 'oracle' && (inputs.INT_TEST_JAVA_RUNTIME_VERSION == '8' || inputs.INT_TEST_JAVA_RUNTIME_VERSION == '11'))) && 'true' || 'false' }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

[very trivial] The order of the first condition is VERSION and then VENDOR, but the order of the second condition is VENDER and then VERSION. It might be better if they're consistent.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Torch3333 Could you add a comment to explain why this condition is needed instead of SET_UP_INT_TEST_RUNTIME_NON_ORACLE_JDK ?

Copy link
Contributor Author

@Torch3333 Torch3333 Apr 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I fixed both, thank you.
407c806

Please refer to this PR #1978 description for more details about the special handling for Oracle JDK 8 and 11 on Windows.

Triggering the CI with Oracle JDK 8 or 11 to run the integration test currently fails because these versions are unavailable with the Setup Java Github action and more generally through public direct download. To retrieve these JDK versions, you must create a free Oracle account and accept the end-user license agreement.

For these reasons, we need to store these versions internally so that our CI can install them when required.
To store the JDK installer, we decided to package it as Docker image and publish it on the Scalar Github registry. The advantage is that it is cost-effective and we don't need to manage additional credentials compared to a storage option on AWS or Azure.

SET_UP_INT_TEST_RUNTIME_ORACLE_JDK_8_OR_11: "${{ ((inputs.INT_TEST_JAVA_RUNTIME_VERSION == '8' || inputs.INT_TEST_JAVA_RUNTIME_VERSION == '11') && inputs.INT_TEST_JAVA_RUNTIME_VENDOR == 'oracle') && 'true' || 'false' }}"

strategy:
matrix:
Expand All @@ -1144,6 +1140,13 @@ jobs:
steps:
- uses: actions/checkout@v4

- name: Setup Sql Server 2017
uses: rails-sqlserver/setup-mssql@v1
with:
components: sqlcmd,sqlengine
sa-password: "SqlServer17"
version: 2017
Comment on lines +1143 to +1148
Copy link
Contributor Author

Choose a reason for hiding this comment

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

SQL Server 2017 is now installed locally instead of using a Docker container on WSL 2, the latter performing worse, as the CI lasted twice as long compared to a local SQL Server installation.
The CI takes about 30 minutes now, compared to less than 15 minutes on Ubuntu.

Copy link
Contributor

Choose a reason for hiding this comment

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

30 mins for SQL Server 2017?

Anyway, we need to think about running only one version (probably the latest version) for each database in CI of a PR and running with other versions regularly, like weekly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the wrong reporting, the SQL Server 2017 took 30 minutes with some modifications I added for debugging.
In reality, it takes 18 minutes for the regular commit job and 20 minutes for the group commit job to complete. Which is a couple of minutes longer compared to the previous duration observed on the Ubuntu runner.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Anyway, we need to think about running only one version (probably the latest version) for each database in CI of a PR and running with other versions regularly, like weekly.

I agree.


- name: Set up JDK ${{ env.JAVA_VERSION }} (${{ env.JAVA_VENDOR }})
uses: actions/setup-java@v4
with:
Expand All @@ -1152,34 +1155,45 @@ jobs:

- name: Set up JDK ${{ env.INT_TEST_JAVA_RUNTIME_VERSION }} (${{ env.INT_TEST_JAVA_RUNTIME_VENDOR }}) to run integration test
uses: actions/setup-java@v4
if: ${{ env.SET_UP_INT_TEST_RUNTIME_NON_ORACLE_JDK == 'true'}}
if: ${{ env.SET_UP_INT_TEST_RUNTIME_JDK_WHEN_NOT_ORACLE_8_OR_11 == 'true'}}
with:
java-version: ${{ env.INT_TEST_JAVA_RUNTIME_VERSION }}
distribution: ${{ env.INT_TEST_JAVA_RUNTIME_VENDOR }}

- name: Login to Oracle container registry
- name: Login to GitHub Container Registry
uses: docker/login-action@v3
if: ${{ env.INT_TEST_JAVA_RUNTIME_VENDOR == 'oracle' }}
if: ${{ env.SET_UP_INT_TEST_RUNTIME_ORACLE_JDK_8_OR_11 == 'true'}}
with:
registry: container-registry.oracle.com
username: ${{ secrets.OCR_USERNAME }}
password: ${{ secrets.OCR_TOKEN }}
registry: ghcr.io
username: ${{ github.repository_owner }}
password: ${{ secrets.CR_PAT }}

- name: Set up JDK ${{ env.INT_TEST_JAVA_RUNTIME_VERSION }} (oracle) to run the integration test
if: ${{ env.INT_TEST_JAVA_RUNTIME_VENDOR == 'oracle' }}
if: ${{ env.SET_UP_INT_TEST_RUNTIME_ORACLE_JDK_8_OR_11 == 'true'}}
run: |
container_id=$(docker create "container-registry.oracle.com/java/jdk:${{ env.INT_TEST_JAVA_RUNTIME_VERSION }}")
docker cp -L "$container_id:/usr/java/default" /usr/lib/jvm/oracle-jdk && docker rm "$container_id"
$container_id=$(docker create "ghcr.io/scalar-labs/oracle/jdk:${{ env.INT_TEST_JAVA_RUNTIME_VERSION }}-windows")
docker cp "${container_id}:oracle-jdk.exe" .
docker rm "$container_id"
Write-Host "Install Oracle JDK"
Start-Process "oracle-jdk.exe" -NoNewWindow -Wait -ArgumentList "/s"
Write-Host "Oracle JDK installation successful"
if ( ${env:INT_TEST_JAVA_RUNTIME_VERSION} -eq '8' ) {
$jdk_root_dir = "jdk-1.8"
} else {
$jdk_root_dir = "jdk-11"
}
echo "JAVA_HOME=C:\Program Files\Java\${jdk_root_dir}" >> ${env:GITHUB_ENV}
Comment on lines -1160 to +1185
Copy link
Contributor Author

@Torch3333 Torch3333 Apr 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I copied the JDK setup from the Cosmos DB job, which runs on Windows too.


- name: Create no superuser
run: ./ci/no-superuser/create-no-superuser-sqlserver.sh sqlserver17 SqlServer17 10 3
run: ./ci/no-superuser/create-no-superuser-sqlserver.sh SqlServer17 10 3
shell: bash
timeout-minutes: 1

- name: Setup Gradle
uses: gradle/actions/setup-gradle@v4

- name: Execute Gradle 'integrationTestJdbc' task
run: ./gradlew integrationTestJdbc "-Dscalardb.jdbc.url=jdbc:sqlserver://localhost:1433;databaseName=test_db;encrypt=true;trustServerCertificate=true" -Dscalardb.jdbc.username=no_superuser -Dscalardb.jdbc.password=no_superuser_password ${{ matrix.mode.group_commit_enabled && env.INT_TEST_GRADLE_OPTIONS_FOR_GROUP_COMMIT || '' }}
run: ./gradlew.bat integrationTestJdbc "-Dscalardb.jdbc.url=jdbc:sqlserver://localhost:1433;databaseName=test_db;encrypt=true;trustServerCertificate=true" "-Dfile.encoding=UTF-8" "-Dscalardb.jdbc.username=no_superuser" "-Dscalardb.jdbc.password=no_superuser_password" ${{ matrix.mode.group_commit_enabled && env.INT_TEST_GRADLE_OPTIONS_FOR_GROUP_COMMIT || '' }}

- name: Upload Gradle test reports
if: always()
Expand Down Expand Up @@ -1242,7 +1256,7 @@ jobs:
docker cp -L "$container_id:/usr/java/default" /usr/lib/jvm/oracle-jdk && docker rm "$container_id"

- name: Create no superuser
run: ./ci/no-superuser/create-no-superuser-sqlserver.sh sqlserver19 SqlServer19 10 3
run: ./ci/no-superuser/create-no-superuser-sqlserver.sh SqlServer19 10 3 sqlserver19
timeout-minutes: 1

- name: Setup Gradle
Expand Down Expand Up @@ -1312,7 +1326,7 @@ jobs:
docker cp -L "$container_id:/usr/java/default" /usr/lib/jvm/oracle-jdk && docker rm "$container_id"

- name: Create no superuser
run: ./ci/no-superuser/create-no-superuser-sqlserver.sh sqlserver22 SqlServer22 10 3
run: ./ci/no-superuser/create-no-superuser-sqlserver.sh SqlServer22 10 3 sqlserver22
timeout-minutes: 1

- name: Setup Gradle
Expand Down
43 changes: 24 additions & 19 deletions ci/no-superuser/create-no-superuser-sqlserver.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,24 @@
set -u

# Get container name and password from arguments
SQL_SERVER_CONTAINER_NAME=$1
SQL_SERVER_PASSWORD=$2
MAX_RETRY_COUNT=$3
RETRY_INTERVAL=$4
SQL_SERVER_PASSWORD=$1
MAX_RETRY_COUNT=$2
RETRY_INTERVAL=$3
# If set, use `sqlcmd` of the SQL Server docker container. If unset, use `sqlcmd` installed on the host.
SQL_SERVER_CONTAINER_NAME=${4:-''}
Comment on lines +5 to +9
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed the order of the script input parameters to set the $SQL_SERVER_CONTAINER_NAME last, since it is now optional.

COUNT=0

# Check if the `/opt/mssql-tools18/bin/sqlcmd` command exists or not.
docker exec -t ${SQL_SERVER_CONTAINER_NAME} ls /opt/mssql-tools18/bin/sqlcmd
if [[ $? -eq 0 ]]; then
SQLCMD=/opt/mssql-tools18/bin/sqlcmd
if [[ -n $SQL_SERVER_CONTAINER_NAME ]]; then
# Check if the `/opt/mssql-tools18/bin/sqlcmd` command exists or not.
docker exec -t ${SQL_SERVER_CONTAINER_NAME} ls /opt/mssql-tools18/bin/sqlcmd
if [[ $? -eq 0 ]]; then
SQLCMD="docker exec -t $SQL_SERVER_CONTAINER_NAME /opt/mssql-tools18/bin/sqlcmd"
else
# If there is no `/opt/mssql-tools18/bin/sqlcmd` command, we use old command.
SQLCMD="docker exec -t $SQL_SERVER_CONTAINER_NAME /opt/mssql-tools/bin/sqlcmd"
fi
else
# If there is no `/opt/mssql-tools18/bin/sqlcmd` command, we use old command.
SQLCMD=/opt/mssql-tools/bin/sqlcmd
SQLCMD=sqlcmd
fi

echo "INFO: Creating no superuser start."
Expand All @@ -30,7 +35,7 @@ do

echo "INFO: Retry count: ${COUNT}"

docker exec -t ${SQL_SERVER_CONTAINER_NAME} ${SQLCMD} -S localhost -U sa -P ${SQL_SERVER_PASSWORD} -d master -C -Q "SELECT 1"
${SQLCMD} -S localhost -U sa -P ${SQL_SERVER_PASSWORD} -d master -C -Q "SELECT 1"

if [[ $? -eq 0 ]]; then
break
Expand All @@ -50,40 +55,40 @@ echo "INFO: sqlcmd command succeeded. Continue creating no superuser."

# Create login
echo "INFO: Create login start"
docker exec -t ${SQL_SERVER_CONTAINER_NAME} ${SQLCMD} -S localhost -U sa -P ${SQL_SERVER_PASSWORD} -d master -C -Q "CREATE LOGIN no_superuser WITH PASSWORD = 'no_superuser_password', DEFAULT_DATABASE = master , CHECK_POLICY = OFF, CHECK_EXPIRATION = OFF"
${SQLCMD} -S localhost -U sa -P ${SQL_SERVER_PASSWORD} -d master -C -Q "CREATE LOGIN no_superuser WITH PASSWORD = 'no_superuser_password', DEFAULT_DATABASE = master , CHECK_POLICY = OFF, CHECK_EXPIRATION = OFF"
echo "INFO: Create login end"

# Create database
echo "INFO: Create database start"
docker exec -t ${SQL_SERVER_CONTAINER_NAME} ${SQLCMD} -S localhost -U sa -P ${SQL_SERVER_PASSWORD} -d master -C -Q "CREATE DATABASE test_db COLLATE Japanese_BIN2"
${SQLCMD} -S localhost -U sa -P ${SQL_SERVER_PASSWORD} -d master -C -Q "CREATE DATABASE test_db COLLATE Japanese_BIN2"
echo "INFO: Create database end"

# Create no_superuser
echo "INFO: Create no_superuser start"
docker exec -t ${SQL_SERVER_CONTAINER_NAME} ${SQLCMD} -S localhost -U sa -P ${SQL_SERVER_PASSWORD} -d test_db -C -Q "CREATE USER no_superuser FOR LOGIN no_superuser"
${SQLCMD} -S localhost -U sa -P ${SQL_SERVER_PASSWORD} -d test_db -C -Q "CREATE USER no_superuser FOR LOGIN no_superuser"
echo "INFO: Create no_superuser end"

# Add roles
echo "INFO: Add role db_ddladmin start"
docker exec -t ${SQL_SERVER_CONTAINER_NAME} ${SQLCMD} -S localhost -U sa -P ${SQL_SERVER_PASSWORD} -d test_db -C -Q "EXEC sp_addrolemember @rolename = 'db_ddladmin', @membername = 'no_superuser'"
${SQLCMD} -S localhost -U sa -P ${SQL_SERVER_PASSWORD} -d test_db -C -Q "EXEC sp_addrolemember @rolename = 'db_ddladmin', @membername = 'no_superuser'"
echo "INFO: Add role db_ddladmin end"

echo "INFO: Add role db_datawriter start"
docker exec -t ${SQL_SERVER_CONTAINER_NAME} ${SQLCMD} -S localhost -U sa -P ${SQL_SERVER_PASSWORD} -d test_db -C -Q "EXEC sp_addrolemember @rolename = 'db_datawriter', @membername = 'no_superuser'"
${SQLCMD} -S localhost -U sa -P ${SQL_SERVER_PASSWORD} -d test_db -C -Q "EXEC sp_addrolemember @rolename = 'db_datawriter', @membername = 'no_superuser'"
echo "INFO: Add role db_datawriter end"

echo "INFO: Add role db_datareader start"
docker exec -t ${SQL_SERVER_CONTAINER_NAME} ${SQLCMD} -S localhost -U sa -P ${SQL_SERVER_PASSWORD} -d test_db -C -Q "EXEC sp_addrolemember @rolename = 'db_datareader', @membername = 'no_superuser'"
${SQLCMD} -S localhost -U sa -P ${SQL_SERVER_PASSWORD} -d test_db -C -Q "EXEC sp_addrolemember @rolename = 'db_datareader', @membername = 'no_superuser'"
echo "INFO: Add role db_datareader end"

# Check the collation of test_db (for debugging purposes)
echo "INFO: Check collation start"
docker exec -t ${SQL_SERVER_CONTAINER_NAME} ${SQLCMD} -S localhost -U no_superuser -P no_superuser_password -d test_db -C -Q "SELECT name, collation_name FROM sys.databases"
${SQLCMD} -S localhost -U no_superuser -P no_superuser_password -d test_db -C -Q "SELECT name, collation_name FROM sys.databases"
echo "INFO: Check collation end"

# Check if no_superuser can access SQL Server (for debugging purposes)
echo "INFO: SELECT @@version start"
docker exec -t ${SQL_SERVER_CONTAINER_NAME} ${SQLCMD} -S localhost -U no_superuser -P no_superuser_password -d test_db -C -Q "SELECT @@version"
${SQLCMD} -S localhost -U no_superuser -P no_superuser_password -d test_db -C -Q "SELECT @@version"
echo "INFO: SELECT @@version end"

echo "INFO: Creating no superuser succeeded."
4 changes: 2 additions & 2 deletions ...scalar/db/transaction/consensuscommit/ConsensusCommitNullMetadataIntegrationTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ public void scan_ScanGivenForPreparedWhenCoordinatorStateAborted_ShouldRollback(
selection_SelectionGivenForPreparedWhenCoordinatorStateNotExistAndExpired_ShouldAbortTransaction(
Selection s) throws ExecutionException, CoordinatorException, TransactionException {
// Arrange
long prepared_at = System.currentTimeMillis() - RecoveryHandler.TRANSACTION_LIFETIME_MILLIS;
long prepared_at = System.currentTimeMillis() - RecoveryHandler.TRANSACTION_LIFETIME_MILLIS - 1;
Copy link
Contributor Author

@Torch3333 Torch3333 Apr 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

On the Windows runner, failures occurred for integration tests that verify the behavior of reading a prepared record without a corresponding transaction status in the coordinator table, which should roll back the transaction if it is expired.

Without the fix above, the transaction was considered not to have expired in some cases by the RecoveryHandler triggered after the prepared record was read.

scalardb/core/src/main/java/com/scalar/db/transaction/consensuscommit/RecoveryHandler.java

Lines 95 to 107 in a98b694

private void abortIfExpired(Selection selection, TransactionResult result) {
long current = System.currentTimeMillis();
if (current <= result.getPreparedAt() + TRANSACTION_LIFETIME_MILLIS) {
return;
}
try {
coordinator.putStateForLazyRecoveryRollback(result.getId());
rollbackRecord(selection, result);
} catch (CoordinatorException e) {
logger.warn("Coordinator tries to abort {}, but failed", result.getId(), e);
}
}

This happened because System.currentTimeInMillis() sometimes returns the same timestamp as the previous call on this line, which set the prepared_at value for the record.

scalardb/integration-test/src/main/java/com/scalar/db/transaction/consensuscommit/ConsensusCommitNullMetadataIntegrationTestBase.java

Line 641 in cb170fd

long prepared_at = System.currentTimeMillis() - RecoveryHandler.TRANSACTION_LIFETIME_MILLIS - 1;

So, in RecoveryHandler.abortIfExpired(), the if (current <= result.getPreparedAt() + TRANSACTION_LIFETIME_MILLIS) { condition was true and the transaction was considered as not expired, so not a target for abort.

The same timestamp value is returned because of a relative lack of system clock precision that we don't observe on Ubuntu. Increasing the system clock precision may be possible( see this documentation), but it is not trivial, so we updated the integration test code instead.

populatePreparedRecordWithNullMetadataAndCoordinatorStateRecord(
storage, namespace1, TABLE_1, TransactionState.PREPARED, prepared_at, null);
DistributedTransaction transaction = manager.begin();
Expand Down Expand Up @@ -1002,7 +1002,7 @@ public void scan_ScanGivenForDeletedWhenCoordinatorStateAborted_ShouldRollback()
selection_SelectionGivenForDeletedWhenCoordinatorStateNotExistAndExpired_ShouldAbortTransaction(
Selection s) throws ExecutionException, CoordinatorException, TransactionException {
// Arrange
long prepared_at = System.currentTimeMillis() - RecoveryHandler.TRANSACTION_LIFETIME_MILLIS;
long prepared_at = System.currentTimeMillis() - RecoveryHandler.TRANSACTION_LIFETIME_MILLIS - 1;
populatePreparedRecordWithNullMetadataAndCoordinatorStateRecord(
storage, namespace1, TABLE_1, TransactionState.DELETED, prepared_at, null);
DistributedTransaction transaction = manager.begin();
Expand Down
4 changes: 2 additions & 2 deletions ...com/scalar/db/transaction/consensuscommit/ConsensusCommitSpecificIntegrationTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ public void scan_ScanGivenForPreparedWhenCoordinatorStateAborted_ShouldRollback(
Selection s, CommitType commitType)
throws ExecutionException, CoordinatorException, TransactionException {
// Arrange
long prepared_at = System.currentTimeMillis() - RecoveryHandler.TRANSACTION_LIFETIME_MILLIS;
long prepared_at = System.currentTimeMillis() - RecoveryHandler.TRANSACTION_LIFETIME_MILLIS - 1;
String ongoingTxId =
populatePreparedRecordAndCoordinatorStateRecord(
storage, namespace1, TABLE_1, TransactionState.PREPARED, prepared_at, null, commitType);
Expand Down Expand Up @@ -919,7 +919,7 @@ public void scan_ScanGivenForDeletedWhenCoordinatorStateAborted_ShouldRollback(
Selection s, CommitType commitType)
throws ExecutionException, CoordinatorException, TransactionException {
// Arrange
long prepared_at = System.currentTimeMillis() - RecoveryHandler.TRANSACTION_LIFETIME_MILLIS;
long prepared_at = System.currentTimeMillis() - RecoveryHandler.TRANSACTION_LIFETIME_MILLIS - 1;
String ongoingTxId =
populatePreparedRecordAndCoordinatorStateRecord(
storage, namespace1, TABLE_1, TransactionState.DELETED, prepared_at, null, commitType);
Expand Down
4 changes: 2 additions & 2 deletions ...ar/db/transaction/consensuscommit/TwoPhaseConsensusCommitSpecificIntegrationTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ public void scan_ScanGivenForPreparedWhenCoordinatorStateAborted_ShouldRollback(
SelectionType selectionType)
throws ExecutionException, TransactionException, CoordinatorException {
// Arrange
long prepared_at = System.currentTimeMillis() - RecoveryHandler.TRANSACTION_LIFETIME_MILLIS;
long prepared_at = System.currentTimeMillis() - RecoveryHandler.TRANSACTION_LIFETIME_MILLIS - 1;
populatePreparedRecordAndCoordinatorStateRecordForStorage1(
TransactionState.PREPARED, prepared_at, null);

Expand Down Expand Up @@ -794,7 +794,7 @@ public void scan_ScanGivenForDeletedWhenCoordinatorStateAborted_ShouldRollback()
SelectionType selectionType)
throws ExecutionException, TransactionException, CoordinatorException {
// Arrange
long prepared_at = System.currentTimeMillis() - RecoveryHandler.TRANSACTION_LIFETIME_MILLIS;
long prepared_at = System.currentTimeMillis() - RecoveryHandler.TRANSACTION_LIFETIME_MILLIS - 1;
populatePreparedRecordAndCoordinatorStateRecordForStorage1(
TransactionState.DELETED, prepared_at, null);

Expand Down
Loading