-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Add support for exchange spooling on Alluxio in FTE #26669
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
Add support for exchange spooling on Alluxio in FTE #26669
Conversation
d8008ca
to
9084874
Compare
return this; | ||
} | ||
|
||
public Optional<@FileExists String> getAlluxioSiteConfPath() |
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.
Why do we need this one? We do not have it for io.trino.filesystem.alluxio
.
If we need to be able to change some specific Alluxio config parameters it is better to expose those as specific getters/setters in ExchangeAlluxioConfig
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.
Thank you for your feedback!
In Alluxio filesystem doc, the Alluxio cluster connection is configured in the alluxio-site.properties
properties file. The same config file must be located in /opt/alluxio/conf
on all Trino cluster nodes.
Since the Alluxio client automatically loads configurations from /opt/alluxio/conf/alluxio-site.properties
, a custom configuration is likely necessary for exchange spooling, as its requirements may differ from those of the Alluxio filesystem used for the catalog. So if Alluixo is used for exchange spooling, should we avoid loading the configuration from /opt/alluxio/conf/alluxio-site.properties
which is only used for catalog?
This configuration is similar to hdfs.config.resources
which would be used if HDFS serves as the exchange storage.
Due to the vast number of configuration settings available in Alluxio, It seems that using configuration files would be more convenient.
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.
Yeah. It makes sense. Thanks
9084874
to
d5375e2
Compare
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.
This looks good.
I am a bit concerned about using syncronous APIs for implementing asynchronous SPI but let's see how it works in practice.
Would you be fine adding some minimal documentation to fault-tolerant-execution.md
?
.../main/java/io/trino/plugin/exchange/filesystem/alluxio/AlluxioFileSystemExchangeStorage.java
Show resolved
Hide resolved
Currently, I use Alluxio as exchange storage by HDFS client, yet the P99 query latency decreased by 1/3 compared to using HDFS. |
Thanks |
Now that this is supported natively, do we still need |
The configuration |
Description
Support exchange spooling on Alluxio
Additional context and related issues
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: