druid icon indicating copy to clipboard operation
druid copied to clipboard

Bump avatica-core from 1.17.0 to 1.22.0

Open dependabot[bot] opened this issue 3 years ago • 7 comments

Bumps avatica-core from 1.17.0 to 1.22.0.

Commits
  • 71fc0ab [CALCITE-5220] Release Avatica 1.22
  • 0c097b6 [CALCITE-5218] Verify HTTP client class before instantiating it
  • aad227f Checkout release svn repository when promoting a release using the docker script
  • bad14c7 Update website for Avatica 1.21.0 release
  • 0883262 [CALCITE-5097] Release Avatica 1.21.0
  • 360c0e7 [CALCITE-5095] Support Java 18 and Guava 31.1-jre
  • 05658fe [CALCITE-5116] Upgrade vlsi-release-plugins to 1.78
  • 1f0f0c1 [CALCITE-4147] Rename "master" branch to "main"
  • 7f9844c [CALCITE-5108] Make website GDPR-compliant
  • 7e168f2 [CALCITE-5106] Upgrade to Jekyll 4 and remove unnecessary dependencies from g...
  • Additional commits viewable in compare view

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the Security Alerts page.

dependabot[bot] avatar Aug 06 '22 09:08 dependabot[bot]

The upgrade is disabled, see #13160

FrankChen021 avatar Oct 06 '22 04:10 FrankChen021

Oh, I mistook avatica as calcite, ignored the comment above.

FrankChen021 avatar Oct 06 '22 05:10 FrankChen021

Looks like it is failing code coverage, weird, no code was added / removed. Tried restarting those jobs, lets see.

zachjsh avatar Oct 06 '22 06:10 zachjsh

@dependabot rebase

zachjsh avatar Oct 06 '22 06:10 zachjsh

Looks like this PR has been edited by someone other than Dependabot. That means Dependabot can't rebase it - sorry!

If you're happy for Dependabot to recreate it from scratch, overwriting any edits, you can request @dependabot recreate.

dependabot[bot] avatar Oct 06 '22 06:10 dependabot[bot]

Looks like it is failing code coverage, weird, no code was added / removed. Tried restarting those jobs, lets see.

Ah, the coverage always runs, even when the tests fail, it looks like some of the tests are failing: https://app.travis-ci.com/github/apache/druid/jobs/584853001#L1978

clintropolis avatar Oct 06 '22 08:10 clintropolis

Looked into the issues. There are several. Upshot: we cannot upgrade to this version: Avatica itself needs some fixes. We may have compatibility issues when older Avatica clients talk with newer Avatica servers.

Version 1.22 appears to be a bit better at enforcing types. This has introduce multiple subtle failures in the tests, in part because Druid seems to be working around prior issues and those workarounds now seem to be causing problems. All of the issues are related to type propagation.

Let's start with Protobuf. This protocol preserves the Java types we define in the server. Some tests fail because, before 1.22, the data sent across the wire would change type: Double would get deserialized as Float, say. With 1.22, tests have to be updated to expect Double values if we say that the column is of type DOUBLE. However, this uncovers test flaws. We expect fractional floating point values to compare exactly. They were equal when the type was converted to Float, but are no longer equal when the full Double type is preserved.

Druid supports both the Prototbuf and JSON protocols. While Protobuf appears to preserve types, JSON does not: we're at the mercy of whatever Jackson chooses as the deserialized type. This is related to the fact that JSON has only a single number type: there is some fudging required to map to a Java type. The result is that we tell Avatica that the type is BIGINT (Long), but Jackson deserializes the value as Integer if it is small. This seems to work OK for scalar values, but in 1.22, it causes problems with arrays.

Avatica expects that the elements in an array match the defined type. But, with JSON, we create an array of Long on the server, but Jackson deserializes them to an array of Integer on the client. Avatica attempts to cast the item to a Long, which fails. It would seem that Avatica should expect mixed Integer and Long values in this case, casting them to Long

Druid appearently tried to work around this by declaring the Java type of some numbers to be Number. While this worked in 1.17, it does not work in 1.22. We have to declare the specific type.

Thus, when doing this upgrade, we have four issues:

  • Avatica is broken. We probably have to submit a patch to get arrays to work with JSON. We can then consider using 1.23 (or later) with the patch.
  • Druid has to change to match the stricter type rules for 1.22.
  • Tests have to change to match the new behavior, and cannot expect that Double values compare exactly.
  • Compatibility

With the last item, a worry is compatibility. Some of the changes do suggest that newer servers may not work with older clients. We cannot expect users to upgrade their clients concurrently with Druid: some cross-version interoperation is necessary. Thus, we need testing, which is a bit hard to do as a unit test or integration test.

Net review is -1 on this change until we resolve the above issues.

paul-rogers avatar Oct 09 '22 05:10 paul-rogers

A newer version of these dependencies exists, but since this PR has been edited by someone other than Dependabot I haven't updated it. You'll get a PR for the updated version as normal once this PR is merged.

dependabot[bot] avatar Jan 20 '23 23:01 dependabot[bot]

looks like this has been closed as part of #14510, can you confirm @somu-imply ?

xvrl avatar Aug 15 '23 02:08 xvrl

When we upgraded to 1.23 we did not see any of the issues we saw in the Travis failure here. To reconfirm we are triggering a test with both the old and new versions of Avatica. I'll close this PR when we have the results. But so far everything looks good and the CI on #14510 did not see any issues

somu-imply avatar Aug 17 '23 20:08 somu-imply

We finished our runs, and things look good. Here is what we know about Druid upgrading to Avatica 1.23.0 : The customer can continue to use older versions of Calcite client jar file. We tested 1.19.0 and that works fine. They do not need to do anything if they are using a version that is 1.20.0 or lower. If the customer uses a version that is 1.21.0 or higher, they will have to add the connection option transparent_reconnection=true. See https://calcite.apache.org/avatica/docs/client_reference.html#transparent_reconnection. This issue can be closed

somu-imply avatar Aug 22 '23 22:08 somu-imply

Thanks @somu-imply I will close the issue here. Do you mind adding a comment to the PR in https://github.com/apache/druid/pull/14510 to ensure we mention the compatibility requirements as part of the release notes? Thanks!

xvrl avatar Aug 22 '23 22:08 xvrl

OK, I won't notify you again about this release, but will get in touch when a new version is available. If you'd rather skip all updates until the next major or minor version, let me know by commenting @dependabot ignore this major version or @dependabot ignore this minor version. You can also ignore all major, minor, or patch releases for a dependency by adding an ignore condition with the desired update_types to your config file.

If you change your mind, just re-open this PR and I'll resolve any conflicts on it.

dependabot[bot] avatar Aug 22 '23 22:08 dependabot[bot]