Skip to content
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.

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

Revert behavior : the Gradle build does not require to use JDK 8 (temurin) #1837

Merged
merged 2 commits into from
Jun 6, 2024

Conversation

Torch3333
Copy link
Contributor

@Torch3333 Torch3333 commented Jun 5, 2024

Description

Changes introduced by #1749 made the Gradle build require having a JDK 8 (temurin) installed on the system. This forced users cloning ScalarDB repository and using the master branch to have installed JDK 8 (temurin), which is inconvenient.
This PR reverts to the previous behavior, which is not to force Gradle to use a specific JDK. Users can use any JDK they desire to build ScalarDB.

Additionally, #1749 added Gradle project properties to configure Gradle to use a specific JDK version and vendor. This is left unchanged.

Related issues and/or PRs

Changes made

Updated JdkConfigurationPlugin to not require Java-related tasks to use JDK 8 (temurin). Java-related tasks will used by default the JDK used by Gradle.

Checklist

The following is a best-effort checklist. If any items in this checklist are not applicable to this PR or are dependent on other, unmerged PRs, please still mark the checkboxes after you have read and understood each item.

  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation to reflect the changes.
  • 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

@Torch3333 Torch3333 self-assigned this Jun 5, 2024
@Torch3333 Torch3333 force-pushed the gradle_do_not_force_jdk_8_temurin branch from ca93b65 to 1a8fa57 Compare June 5, 2024 01:35
@Torch3333 Torch3333 changed the title Gradle do not require anymore to use JDK 8 temurin by default. Revert bahavior : Gradle build doest not require to use JDK 8 temurin Jun 5, 2024
@Torch3333 Torch3333 changed the title Revert bahavior : Gradle build doest not require to use JDK 8 temurin Revert bahavior : Gradle build doest not require to use JDK 8 (temurin) Jun 5, 2024
@Torch3333 Torch3333 changed the title Revert bahavior : Gradle build doest not require to use JDK 8 (temurin) Revert bahavior : the Gradle build does not require to use JDK 8 (temurin) Jun 5, 2024
@Torch3333 Torch3333 marked this pull request as ready for review June 5, 2024 04:36
@Torch3333 Torch3333 changed the title Revert bahavior : the Gradle build does not require to use JDK 8 (temurin) Revert behavior : the Gradle build does not require to use JDK 8 (temurin) Jun 5, 2024
Copy link
Contributor

@feeblefakie feeblefakie left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

Copy link
Contributor

@jnmt jnmt left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for quickly fixing it. I was able to confirm the behavior reverted in my environment.

Copy link
Collaborator

@brfrn169 brfrn169 left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

Copy link
Contributor

@komamitsu komamitsu left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@feeblefakie
Copy link
Contributor

@Torch3333 The Cosmos DB integration test is failing. Can you check?

@jnmt
Copy link
Contributor

jnmt commented Jun 6, 2024

@Torch3333 The Cosmos DB integration test is failing. Can you check?

@feeblefakie CI passed by retrying it since it was an out-of-memory error. So, I think we can merge this. (If the same issue occurs a lot, we might need to consider something, though.)

java.lang.AssertionError: 
Expecting actual throwable to be an instance of:
  com.scalar.db.exception.transaction.PreparationException
but was:
  java.lang.OutOfMemoryError: GC overhead limit exceeded

	at com.scalar.db.transaction.consensuscommit.TwoPhaseConsensusCommitSpecificIntegrationTestBase.commit_WriteSkewOnNonExistingRecordsWithSerializableWithExtraWrite_OneShouldCommitTheOtherShouldThrowPreparationException(TwoPhaseConsensusCommitSpecificIntegrationTestBase.java:2180)

@feeblefakie feeblefakie merged commit f10270c into master Jun 6, 2024
26 checks passed
@feeblefakie
Copy link
Contributor

@jnmt Thank you!

@Torch3333
Copy link
Contributor Author

@jnmt
Thanks for checking.

I noticed CosmosDB integration tests have been particularly flaky recently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants