Update to Java 11 #11512
Update to Java 11 #11512
Conversation
aherrmann-da
force-pushed
the
java11
branch
4 times, most recently
from
7b884ad to
6a637c5
Compare
| transaction.effectiveAt.map(t => t.seconds * 1000000L + t.nanos.toLong / 1000L).get, | ||
| transaction.effectiveAt | ||
| .map(t => t.seconds * 1000000L + t.nanos.toLong / 1000000L * 1000L) | ||
| .get, |
There was a problem hiding this comment.
The migration tests compare transactions represented as
data Transaction = Transaction
{ transactionId :: T.Text
, events :: [Event]
, letMicros :: !I.Int64 -- :: Ledger effective time, in microseconds since unix epoch
} deriving (Generic, Eq, Show)
The transactionId and events seem reasonable to compare for equality. But, the ledger effective time is surprising. Should we really expect timestamps to be exactly the same?
It turns out that they no longer are the same when we update to Java 11. The reason seems to be that Java increased the time resolution in some contexts on the update from version 8 to 9, see commit message of af77e14 and JDK-8068730. The migration tests fail due to a difference in precision in these timestamps: In one case we only have mill second precision but in the other microsecond precision:
- letMicros = 1636122007333866
+ letMicros = 1636122007333000
This change is a simple hack to round down to millisecond precision again. But, the question is, what guarantees about timestamp stability do we really want to give and subsequently test in these migration tests?
cc @cocreature
There was a problem hiding this comment.
I think I don’t fully understand what is failing. So for this setup, we run everything with JDK11 afaict so this seems to be a build-time change. and if I understand it correctly the precision increased rather than decreased in newer JDK versions.
So shouldn’t we be able to preserve the precision on upgrades?
There was a problem hiding this comment.
It is indeed a runtime property, to be sure I tested with a JAR built with Java 8/11 and run with Java 8/11 that calls Instant.now().
Interestingly, the migration test works up to SDK version 1.0.1 and starts failing at version 1.1.1.
<-- Tested: SDK version: 1.0.1
--> Testing: SDK version: 1.1.1
Looking at the commits between these releases I see two that are related to timestamps: 49c6a20 fe29cfb
Perhaps one of them is relevant.
There was a problem hiding this comment.
@rautenrieth-da Is it expected that migration on JDK 11 changes the precision of the timestamps returned by the ledger API?
There was a problem hiding this comment.
for now, I don’t think this is blocking.
There was a problem hiding this comment.
I believe those are the actual numbers but they are the wrong way around. The numbers seem to have increased in precision. You can find the full transactions that cause the failure at https://gist.github.com/cocreature/054fb8fa171b51f59a14b0dd6644b20d.
So it looks like we stored with higher precision before but it wasn’t exposed whereas after the upgrade it did get exposed.
There was a problem hiding this comment.
Any sub-microsecond precision should be erased with the migration to SDK 1.17. It's impossible to store nanosecond-precision timestamps in the 1.17 index database.
If the transactions returned by SDK 1.1.0 contain nanosecond-precision LETs, then I'm surprised the migration to SDK 1.17 did not fail.
There was a problem hiding this comment.
So I pushed a commit to debug this 624a607 and I still get the mismatch even after 1.17. So it doesn’t seem like the migrations on 1.17 blow up nor does it seem like we only emit microsecond precision at least for old transactions (haven’t checked for new ones).
There was a problem hiding this comment.
I don’t have time to debug this further and I don’t understand the indexer schema but happy to help you run those tests to reproduce, just ping me.
There was a problem hiding this comment.
Just to avoid confusion: I had an off by 3 orders of magnitude above:
On SDK 1.0.0 we get _mili_second precision, afterwards we get _micro_second precision. We never get _nano_second precision.
We expect to get microsecond precision and it looks like that is always stored in the database. There seems to be a bug in SDK 1.0.0 where things get truncated to milisecond precision on the ledger API.
Given that this doesn’t affect later versions, we agreed with @rautenrieth-da to continue with what I suggested above and bump the starting version in the compat tests to 1.6.
| @@ -58,7 +58,7 @@ conformance_test( | |||
| "@curl_nix//:bin/curl", | |||
| "@grpcurl_nix//:bin/grpcurl", | |||
| "@jq_dev_env//:jq", | |||
| "@jdk11_nix//:bin/java", | |||
| "@bazel_tools//tools/jdk", | |||
There was a problem hiding this comment.
I think we could also undo bc4e00b#diff-cd3817887ce3122fe5224097f3ef967cbc72d04c2b0dd483a024dec6f0f18eb4 but let’s not do this in this PR if at all. This seems very likely to require changes with the integration of Canton in the Daml repo anyway.
There was a problem hiding this comment.
Yes, it could perhaps even be simplified to importing canton as a java_binary since that would use the correct JDK version as well. Could avoid some of the dance around $JAVA .... There are other instances of this pattern in the code base where that may also be possible.
| transaction.effectiveAt.map(t => t.seconds * 1000000L + t.nanos.toLong / 1000L).get, | ||
| transaction.effectiveAt | ||
| .map(t => t.seconds * 1000000L + t.nanos.toLong / 1000000L * 1000L) | ||
| .get, |
There was a problem hiding this comment.
for now, I don’t think this is blocking.
changelog_begin changelog_end
Using Guava as a replacement, since it is already a project dependency.
See https://bugs.openjdk.java.net/browse/JDK-8068730 The precision of `Instant.now()` increased between Java 8 and Java 9. On Linux and MacOS this doesn't seem to be a problem, as the precision still seems to be at micro seconds. However, on Windows this now causes errors of the following form: ``` java.lang.IllegalArgumentException: Conversion of Instant 2021-11-05T13:58:56.726875100Z to microsecond granularity would result in loss of precision. ``` Suggesting that it now offers sub-microsecond precision. `TimestampConversion.instantToMicros` had a check to fail if the conversion lead to a loss of precision. In the specific failing test case this is not a concern, so this adds a `roundInstantToMicros` variant that avoids this kind of error.
This reverts commit af8e261.
changelog_begin changelog_end
aherrmann-da
requested review from
garyverhaegen-da,
remyhaemmerle-da,
S11001001,
stefanobaghino-da and
a team
as code owners
|
I've added logic to skip versions before 1.6.0 in the migration tests and removed the lte rounding patch. |
This PR has been created by a script, which is not very smart and does not have all the context. Please do double-check that the version prefix is correct before merging. @nickchapman-da is in charge of this release. Commit log: ``` bd2a685 Make ACS reader parameters configurable (#11732) 329e609 [Self-service error codes] Group explanations (#11715) 32e9c4b Bump go rules (#11829) 6df7af7 update compat versions for 1.18.0-snapshot.20211117.8399.0.a05a40ae (#11743) 9461702 [DPP-673][Self-service error codes] Split one narrow migration table into multiple narrower ones to fit in the generated pdf's pages. (#11803) 5519184 [DPP-673][Self-service error-codes] Generate error code directory compatible with both pdf and html docs (#11794) 5a2c070 Skip append-only migration tests on each PR (#11816) e6c8229 port #11798 documentation changes to json-api HTTP codes documentation (#11823) 1610d97 LF: parsing interface primitives (#11825) f2aa09c Allow encoded server key files to be in base64 [DPP-761] (#11796) cc3f551 update NOTICES file (#11824) 1bfcbc9 Refactoring for Canton compatibility on PackageServiceErrors (#11812) c4b6a17 [Self-service error codes] Update existing error code references [DPP-593] (#11798) addb2ed Drop Scala 2.13 version guards from Bazel definitions (#11819) 5785bbf Drop LF < 1.14 from supported damlc output versions (#11701) ae8391e LF Parser: handle interface (#11797) faddba4 Increase range for excluded snapshots in compatilbility tests (#11802) f8933d1 Make lookupTemplateChoice return only choices in the template. (#11808) 3e0ac71 update NOTICES file (#11801) 8e31e8e [Self-service error codes] Enabled by default [DPP-691] (#11685) ab520fb Fix es (#11784) 350ad98 Bump Windows postgres (#11804) 2d60ae8 update NOTICES file (#11799) 2d9ee20 [ledger-api-client] - Do not account for deduplication_time as timeout [KVL-1172] (#11791) f334861 LF: Simplify LF Syntax (#11795) cbe9c92 LF: Clean up Ast (#11786) 25e5bbb kvutils - Removed unused setting from conformance tests [KVL-1174] (#11785) cd52f13 Remove `@aherrmann-da` from `CODEOWNERS` (#11792) 9bb12d0 update NOTICES file (#11787) d23d113 Restructure interface desugaring (#11555) 1bb2fc2 LF: Simplify transaction versionning for interface (#11744) 4b59c57 Move select DB logic to JVM [DPP-760] (#11777) 5fb5784 Limit the number of parallel Bazel actions (#11779) a9754fe Write exception reports into the logfile in case of ssl misconfiguration (#11776) 4a8e110 Add missing export of deleteBy from DA.NonEmpty (#11716) 51f646c Add engine tests for interfaces. (#11773) ac57355 Drop Scala 2.12 compatibility layer from //daml-lf/data (#11758) 3b5f8a7 Remove precondition field from TemplateImplements (#11763) 2e789dd [DPP-417][Self-service error codes][Docs] Add more complete information for forking gRPC status codes. (#11739) 4ebdaf5 kvutils - Deprecate deduplicate_until [kvl-1174] (#11765) f5a6302 [participant-integration-api] - Use internal CommandCompletionService [KVL-1172] (#11741) 39a38d3 Update to Java 11 (#11512) 91b4fb9 Backport: Skip protobuf compatibility check for PRs to non-main (#11772) a3556a5 Retry release signature checks (#11771) 8f458d8 [ledger-api] Include the completion offset in the command_service.proto responses [KVL-1171] (#11658) e891180 participant-state: Remove the v1 API. (#11553) 869b805 Bump sandbox acquisition timeout of trigger service tests to 1m (#11764) 4106222 Fix docs regarding deduplication periods [KVL-1194] (#11738) 186ba10 Fix missing encoding of nested maps in typescript bindings (#11746) dd88ba2 kvutils - Backwards looking command deduplication [KVL-1174] (#11634) 43438c6 sandbox: Remove PostgreSQL conformance tests. (#11434) b0a1bf7 More missing error codes (#11749) 3366413 Delete empty scala test library (#11747) 85979b3 Upgrade to Postgres 10 (#11751) 39c5966 Drop aherrman-da from release rotation (#11756) c8ace8b Factor kind projector target into a variable (#11759) e78bfc7 update NOTICES file (#11757) fa76631 Drop 2.12 versioned_scala_deps (#11748) c77e150 [JSON-API] ContractDao JMH benchmarks against PG (#11718) d60cf70 rotate release duty after 1.18.0-snapshot.20211116.8395.0.ccbf7140 (#11734) bb19c0d Drop Scala 2.12 support (#11619) c2d4ea4 Add separate commands for create/fetch/exercise by template/interface. (#11724) 7e4acf9 link to doc detailing gRPC-to-HTTP code mapping (#11742) 7974427 release 1.18.0-snapshot.20211117.8399.0.a05a40ae (#11740) ``` Changelog: ``` - [Daml Compiler] Damlc can only produce Daml-LF 1.14 or newer. Passing aynthing older to `--target` is an error. If you need to produce older versions, use an older SDK. [Ledger API Specification] The Ledger API returns enriched error codes (see https://docs.daml.com/error-codes/self-service/index.html) For backwards-compatibility, a new API flag `--use-pre-1.18-error-codes` is introduced for preserving the legacy behavior for clients that want to migrate incrementally to the changed gRPC status code responses and error details format. ledger-api-client - The default command tracking timeout is no longer influenced by the deprecated deduplication_time as a deduplication period. Previously if no timeout was being set in the command and deduplication_time was set as the deduplication period then the command tracking timeout was the minimum between the deduplication_time and max tracking timeout. [ledger-api] - Include the completion offset in the responses from the command_service.proto - [Integration Kit] v1 of the participant state API is no longer supported. You will need to upgrade to v2. - [Typescript Bindings] Fix an issue where nested maps did not get encoded properly before sent to the JSON API which caused requests to fail with a decoding error on the JSON API. kvutils - The deduplication duration passed in the command is now used for command deduplication, and is no longer always overwritten with the max deduplication duration. The command deduplication duration can still be extended by the committer to account for time skews. ``` CHANGELOG_BEGIN CHANGELOG_END
Update the Nix imported Java runtime to Java 11.
Update the dadew provided JDK to Java 11.
Update the Java toolchain to Java 11.
Fix deprecation warnings and build errors due to the update to Java 11.
Closes #11065
Pull Request Checklist
CHANGELOG_BEGINandCHANGELOG_ENDtagsNOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with
/AzurePipelines runtotrigger the build.