-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Modify alluxio version to stable 2.9.5 #22350
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Claim that 2.9.4 is "more stable" lacks evidence. Is there any issue that 2.9.4 solves?
Hi thanks for raising the concern, 2.9.x is the stable branch that we have been tested in customer env for a long time. While 3xx is still a beta version that have breaking compatibility changes so that's why we want to avoid bring it into Trino in early stage. Regarding the local cache manager Trino is using right now, 2.9.4 & 312 should have the exactly same code. |
a4b6f7b
to
43bfb18
Compare
There are related failures. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this version is truly more stable and works I am fine with the downgrade but it needs to pass CI and we need evidence from people using it in practice that this works. For example @jkylling or @raunaqmorarka might be able to test it a bit better.
Also why do we need all these additional exclusions .. it the upstream dependency declaration a lot more messed up than in 312?
fwiw we at Dune used to run 2.9.3 here, with extra fixes to work around issues that got fixed in alluxio master later. We're (literally just now) moving to Trino 448's official caching code relying on alluxio 312, so we won't have experience running 2.9.4 in prod. |
That said I agree with the comments above; if the 2.x.x is the stable branch then I don't see any issue using it. Hopefully we should have enough coverage in CI to ensure that once everything is green, the caching code is fine 👍 |
d8b94ef
to
507a339
Compare
9fdf16c
to
edf3a91
Compare
edf3a91
to
12d9ad7
Compare
Thanks @jja725 |
@wendigo thanks for the quick merge! |
Description
2.9.4 is a more stable version and we would keep backporting changes from 3xx to 2.9.x line. With 2.9.4 we would have the same code of local cache manager as 3xx and compatibility with the stable Alluxio Filesystem interface. That's why we chose to have 2.9.4 here
Additional context and related issues
https://github.com/trinodb/trino/pull/21603/files#r1628261795
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
(x) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: