edrevo commented on a change in pull request #232:
URL: https://github.com/apache/arrow-datafusion/pull/232#discussion_r624557196
##########
File path: ballista/rust/executor/src/main.rs
##########
@@ -158,16 +166,22 @@ async fn main() -> Result<()> {
let scheduler = SchedulerGrpcClient::connect(scheduler_url)
.await
.context("Could not connect to scheduler")?;
- let executor = Arc::new(BallistaExecutor::new(config));
- let service = BallistaFlightService::new(executor);
+ let service = BallistaFlightService::new(work_dir);
let server = FlightServiceServer::new(service);
info!(
"Ballista v{} Rust Executor listening on {:?}",
BALLISTA_VERSION, addr
);
let server_future =
tokio::spawn(Server::builder().add_service(server).serve(addr));
- let client = BallistaClient::try_new(&external_host, port).await?;
+ let client_host = external_host.as_deref().unwrap_or_else(|| {
+ if bind_host == "0.0.0.0" {
Review comment:
I have added a comment to clarify. Right now the executor does a really
weird thing which has a big TODO here:
https://github.com/apache/arrow-datafusion/blob/70afe4c459af33b8cb190383c923fcee09cde252/ballista/rust/executor/src/execution_loop.rs#L100-L101
Basically, the executor needs to connect to itself through a BallistaClient
in order to work. If there is an external host defined, then it is clear how to
connect to oneself. If not, we need to check the bind address, but since
`0.0.0.0` is a meta-address, for that case we can just use localhost to connect
to ourselves. Does that make sense?
I started working on the TODO to get rid of this ugliness, but then the PR
would have been too big, so I was planning on tackling that separately.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
