NoahKusaba commented on code in PR #2638:
URL: 
https://github.com/apache/datafusion-ballista/pull/2638#discussion_r4507339540


##########
integrations/iceberg/src/wire.rs:
##########
@@ -0,0 +1,181 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! What the codecs send in place of a live [`Table`], and how they encode it.
+
+use std::collections::VecDeque;
+use std::fmt;
+use std::sync::{Arc, LazyLock, Mutex};
+
+use datafusion::common::{DataFusionError, exec_datafusion_err};
+use iceberg::io::FileIO;
+use iceberg::spec::{TableMetadata, TableMetadataRef};
+use iceberg::table::Table;
+use iceberg::{Runtime, TableIdent};
+use iceberg_datafusion::to_datafusion_error;
+use serde::de::DeserializeOwned;
+use serde::{Deserialize, Serialize};
+use tokio::runtime::{Handle, RuntimeFlavor};
+
+/// Runs the codecs' I/O and owns the tables they rebuild: Iceberg plans scans 
on
+/// a table's runtime, which must outlive the table, and a codec's caller may 
not.
+static RUNTIME: LazyLock<tokio::runtime::Runtime> = LazyLock::new(|| {
+    tokio::runtime::Builder::new_multi_thread()
+        .thread_name("ballista-iceberg")
+        .enable_all()
+        .build()
+        .expect("failed to build the ballista-iceberg runtime")
+});
+
+/// Runs `fut` on [`RUNTIME`] and waits for it, from the synchronous codec API.
+pub(crate) fn block_on<T: Send + 'static>(
+    fut: impl Future<Output = T> + Send + 'static,
+) -> Result<T, DataFusionError> {
+    let wait = || futures::executor::block_on(RUNTIME.spawn(fut));
+    let joined = match Handle::try_current().map(|handle| 
handle.runtime_flavor()) {
+        Ok(RuntimeFlavor::MultiThread) => tokio::task::block_in_place(wait),
+        _ => wait(),
+    };
+    joined.map_err(|e| DataFusionError::External(Box::new(e)))
+}
+
+/// Bounds memory rather than tuning hits: a query touches a few table 
versions,
+/// but each can hold megabytes of metadata.
+const TABLE_CACHE_CAPACITY: usize = 16;
+
+/// Tables rebuilt by [`TableWire::load`], least recently used first, so each
+/// process rebuilds a version once. Metadata files never change, so entries
+/// can't go stale.
+static TABLES: LazyLock<Mutex<VecDeque<(TableWire, Table)>>> =
+    LazyLock::new(|| Mutex::new(VecDeque::new()));
+
+/// A [`Table`] as its metadata file and `FileIO`, from which any node can
+/// rebuild exactly that version without a catalog.
+#[derive(Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub(crate) struct TableWire {
+    identifier: TableIdent,
+    metadata_location: String,
+    file_io: serde_json::Value,
+    readonly: bool,
+    /// The content of `metadata_location`, sent from the client so the

Review Comment:
   Good catch. I went further than a threshold and stopped sending the metadata 
altogether in 56cd7bd3: the table cache already keeps the scheduler's read to 
once per version, so it saved little, and a plan's size no longer grows with 
the table's history.



##########
integrations/iceberg/src/wire.rs:
##########
@@ -0,0 +1,181 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! What the codecs send in place of a live [`Table`], and how they encode it.
+
+use std::collections::VecDeque;
+use std::fmt;
+use std::sync::{Arc, LazyLock, Mutex};
+
+use datafusion::common::{DataFusionError, exec_datafusion_err};
+use iceberg::io::FileIO;
+use iceberg::spec::{TableMetadata, TableMetadataRef};
+use iceberg::table::Table;
+use iceberg::{Runtime, TableIdent};
+use iceberg_datafusion::to_datafusion_error;
+use serde::de::DeserializeOwned;
+use serde::{Deserialize, Serialize};
+use tokio::runtime::{Handle, RuntimeFlavor};
+
+/// Runs the codecs' I/O and owns the tables they rebuild: Iceberg plans scans 
on
+/// a table's runtime, which must outlive the table, and a codec's caller may 
not.
+static RUNTIME: LazyLock<tokio::runtime::Runtime> = LazyLock::new(|| {
+    tokio::runtime::Builder::new_multi_thread()
+        .thread_name("ballista-iceberg")
+        .enable_all()
+        .build()
+        .expect("failed to build the ballista-iceberg runtime")
+});
+
+/// Runs `fut` on [`RUNTIME`] and waits for it, from the synchronous codec API.
+pub(crate) fn block_on<T: Send + 'static>(
+    fut: impl Future<Output = T> + Send + 'static,
+) -> Result<T, DataFusionError> {
+    let wait = || futures::executor::block_on(RUNTIME.spawn(fut));
+    let joined = match Handle::try_current().map(|handle| 
handle.runtime_flavor()) {
+        Ok(RuntimeFlavor::MultiThread) => tokio::task::block_in_place(wait),
+        _ => wait(),
+    };
+    joined.map_err(|e| DataFusionError::External(Box::new(e)))
+}
+
+/// Bounds memory rather than tuning hits: a query touches a few table 
versions,
+/// but each can hold megabytes of metadata.
+const TABLE_CACHE_CAPACITY: usize = 16;
+
+/// Tables rebuilt by [`TableWire::load`], least recently used first, so each
+/// process rebuilds a version once. Metadata files never change, so entries

Review Comment:
   Agreed, I've reworded the comment in aa0473fb: a version is rebuilt once 
only while its credentials stay the same. Caching the metadata by its location 
and rebuilding the `Table` with each request's `FileIO` would avoid the extra 
copies, so I'll leave that as a follow-up.



##########
integrations/iceberg/src/wire.rs:
##########
@@ -0,0 +1,181 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! What the codecs send in place of a live [`Table`], and how they encode it.
+
+use std::collections::VecDeque;
+use std::fmt;
+use std::sync::{Arc, LazyLock, Mutex};
+
+use datafusion::common::{DataFusionError, exec_datafusion_err};
+use iceberg::io::FileIO;
+use iceberg::spec::{TableMetadata, TableMetadataRef};
+use iceberg::table::Table;
+use iceberg::{Runtime, TableIdent};
+use iceberg_datafusion::to_datafusion_error;
+use serde::de::DeserializeOwned;
+use serde::{Deserialize, Serialize};
+use tokio::runtime::{Handle, RuntimeFlavor};
+
+/// Runs the codecs' I/O and owns the tables they rebuild: Iceberg plans scans 
on
+/// a table's runtime, which must outlive the table, and a codec's caller may 
not.
+static RUNTIME: LazyLock<tokio::runtime::Runtime> = LazyLock::new(|| {
+    tokio::runtime::Builder::new_multi_thread()
+        .thread_name("ballista-iceberg")
+        .enable_all()
+        .build()
+        .expect("failed to build the ballista-iceberg runtime")
+});
+
+/// Runs `fut` on [`RUNTIME`] and waits for it, from the synchronous codec API.
+pub(crate) fn block_on<T: Send + 'static>(
+    fut: impl Future<Output = T> + Send + 'static,
+) -> Result<T, DataFusionError> {
+    let wait = || futures::executor::block_on(RUNTIME.spawn(fut));
+    let joined = match Handle::try_current().map(|handle| 
handle.runtime_flavor()) {
+        Ok(RuntimeFlavor::MultiThread) => tokio::task::block_in_place(wait),
+        _ => wait(),
+    };
+    joined.map_err(|e| DataFusionError::External(Box::new(e)))
+}
+
+/// Bounds memory rather than tuning hits: a query touches a few table 
versions,
+/// but each can hold megabytes of metadata.
+const TABLE_CACHE_CAPACITY: usize = 16;
+
+/// Tables rebuilt by [`TableWire::load`], least recently used first, so each
+/// process rebuilds a version once. Metadata files never change, so entries
+/// can't go stale.
+static TABLES: LazyLock<Mutex<VecDeque<(TableWire, Table)>>> =
+    LazyLock::new(|| Mutex::new(VecDeque::new()));
+
+/// A [`Table`] as its metadata file and `FileIO`, from which any node can
+/// rebuild exactly that version without a catalog.
+#[derive(Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub(crate) struct TableWire {
+    identifier: TableIdent,
+    metadata_location: String,
+    file_io: serde_json::Value,
+    readonly: bool,
+    /// The content of `metadata_location`, sent from the client so the
+    /// scheduler needn't read it; scans sent to executors leave it out.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    metadata: Option<TableMetadataRef>,
+}
+
+impl fmt::Debug for TableWire {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_struct("TableWire")
+            .field("identifier", &self.identifier)
+            .field("metadata_location", &self.metadata_location)
+            .field("file_io", &"<redacted>")
+            .field("readonly", &self.readonly)
+            .finish_non_exhaustive()
+    }
+}
+
+impl TableWire {
+    pub(crate) fn new(table: &Table) -> Result<Self, DataFusionError> {
+        let file_io = table
+            .file_io()
+            .serialize_all()
+            .map_err(to_datafusion_error)?;
+        Ok(Self {
+            identifier: table.identifier().clone(),
+            metadata_location: table
+                .metadata_location_result()
+                .map_err(to_datafusion_error)?
+                .to_string(),
+            file_io: serde_json::from_slice(&file_io).map_err(json_err)?,
+            readonly: table.readonly(),
+            metadata: None,
+        })
+    }
+
+    /// Like [`TableWire::new`], also carrying the table's metadata.
+    pub(crate) fn with_metadata(table: &Table) -> Result<Self, 
DataFusionError> {
+        Ok(Self {
+            metadata: Some(table.metadata_ref()),
+            ..Self::new(table)?
+        })
+    }
+
+    /// Rebuilds the table, or returns it from [`TABLES`].
+    pub(crate) fn load(mut self) -> Result<Table, DataFusionError> {
+        // Keyed without the metadata, which its location determines.
+        let metadata = self.metadata.take();
+        if let Some(table) = self.touch(&mut TABLES.lock().unwrap()) {
+            return Ok(table);
+        }
+        let table =
+            
block_on(self.clone().read(metadata))?.map_err(to_datafusion_error)?;
+        let mut tables = TABLES.lock().unwrap();
+        // Another thread may have rebuilt it meanwhile.
+        if let Some(table) = self.touch(&mut tables) {
+            return Ok(table);
+        }
+        if tables.len() == TABLE_CACHE_CAPACITY {
+            tables.pop_front();
+        }
+        tables.push_back((self.clone(), table.clone()));
+        Ok(table)
+    }
+
+    /// Moves this table's entry, if any, to the back of `tables` and returns 
it.
+    fn touch(&self, tables: &mut VecDeque<(TableWire, Table)>) -> 
Option<Table> {
+        let entry = tables.remove(tables.iter().position(|(wire, _)| wire == 
self)?)?;
+        let table = entry.1.clone();
+        tables.push_back(entry);
+        Some(table)
+    }
+
+    async fn read(self, metadata: Option<TableMetadataRef>) -> 
iceberg::Result<Table> {
+        let file_io = 
FileIO::deserialize_all(&serde_json::to_vec(&self.file_io)?)?;
+        let metadata = match metadata {
+            Some(metadata) => metadata,
+            None => Arc::new(
+                TableMetadata::read_from(&file_io, 
&self.metadata_location).await?,
+            ),
+        };
+        Table::builder()

Review Comment:
   Added to the limitations in abcf538e.



##########
integrations/iceberg/src/scheduler.rs:
##########
@@ -0,0 +1,67 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Readies a separately deployed scheduler for Iceberg.
+
+use std::sync::Arc;
+
+use ballista_core::utils::default_session_builder;
+use ballista_scheduler::config::SchedulerConfig;
+use datafusion::catalog::CatalogProvider;
+
+use crate::{IcebergLogicalCodec, IcebergPhysicalCodec, register_iceberg};
+
+/// Readies a [`SchedulerConfig`] for Iceberg.
+pub trait SchedulerConfigExt {
+    /// Adds the Iceberg codecs after any codecs already set, and mounts
+    /// `catalogs` in each session, readied with [`register_iceberg`]. The
+    /// scheduler plans queries from Flight SQL clients itself, so they can
+    /// then query those catalogs.
+    fn with_iceberg<N: Into<String>>(
+        self,
+        catalogs: impl IntoIterator<Item = (N, Arc<dyn CatalogProvider>)>,
+    ) -> Self;
+}
+
+impl SchedulerConfigExt for SchedulerConfig {
+    fn with_iceberg<N: Into<String>>(
+        mut self,
+        catalogs: impl IntoIterator<Item = (N, Arc<dyn CatalogProvider>)>,
+    ) -> Self {
+        let logical = 
IcebergLogicalCodec::after(self.override_logical_codec.take());

Review Comment:
   Added in 7755c0f1, both on `with_iceberg` and in the docs: call it once, 
passing every catalog.



##########
docs/source/user-guide/iceberg.md:
##########
@@ -0,0 +1,162 @@
+<!---
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
+-->
+
+# Apache Iceberg
+
+The `ballista-iceberg` crate (`integrations/iceberg`) lets Ballista scan
+[Apache Iceberg](https://iceberg.apache.org/) tables. On top of
+[iceberg-datafusion](https://github.com/apache/datafusion-iceberg), it adds the
+codecs that send Iceberg tables and scans between the client, the scheduler and
+the executors.
+
+The crate is not published yet. It depends on unreleased revisions of
+iceberg-datafusion and iceberg-rust, and needs Rust 1.95. It is its own
+workspace, with its own `Cargo.lock` and CI, outside Ballista's, and depends
+on a pinned commit of Ballista rather than the crates beside it, so a
+DataFusion upgrade in Ballista that iceberg-rust doesn't support yet doesn't
+block Ballista's build or release. Build and test it from
+`integrations/iceberg`.
+
+## Usage
+
+Ready the session with `register_iceberg`, then mount an Iceberg catalog:
+
+```rust
+use std::sync::Arc;
+
+use ballista::prelude::{SessionConfigExt, SessionContextExt};
+use datafusion::execution::SessionStateBuilder;
+use datafusion::prelude::{SessionConfig, SessionContext};
+use ballista_iceberg::register_iceberg;
+use iceberg_datafusion::IcebergCatalogProvider;
+
+let state = SessionStateBuilder::new()
+    .with_config(SessionConfig::new_with_ballista())
+    .with_default_features()
+    .build();
+let ctx = 
SessionContext::standalone_with_state(register_iceberg(state)).await?;
+
+ctx.register_catalog("rest", 
Arc::new(IcebergCatalogProvider::try_new(catalog).await?));
+ctx.sql("SELECT * FROM rest.ns.t").await?.show().await?;
+```
+
+A single table registered as an `IcebergCatalogTableProvider` works the same
+way. Either kind reads the table's latest version in each query.
+
+For time travel, register an `IcebergStaticTableProvider`, which keeps one
+version: `IcebergStaticTableProvider::try_new_from_table_snapshot(table, 
snapshot_id)`.
+
+The example registers both on a table in a temporary local warehouse, and shows
+a commit reaching the live table but not the static one:
+
+```shell
+cd integrations/iceberg
+cargo run --example standalone-iceberg-scan
+```
+
+## SQL clients
+
+Flight SQL clients (JDBC, ADBC, BI tools) send SQL to the scheduler, which 
plans
+it, so they hold no catalog. To let them query Iceberg tables, mount catalogs
+on the scheduler with `SchedulerConfigExt::with_iceberg` (the `scheduler`
+feature), which also adds the Iceberg codecs:
+
+```rust
+use ballista_iceberg::SchedulerConfigExt;
+
+let rest = IcebergCatalogProvider::try_new(catalog).await?;
+let config = SchedulerConfig::default()
+    .with_flight_sql(true)
+    .with_iceberg([("rest", Arc::new(rest) as _)]);
+```
+
+Executors add the codecs with `ExecutorProcessConfigExt::with_iceberg` (the
+`executor` feature):
+
+```rust
+use ballista_iceberg::ExecutorProcessConfigExt;
+
+let config = ExecutorProcessConfig::default().with_iceberg();
+```
+
+Both keep any codecs the config already has, and the scheduler keeps its
+session builder, mounting the catalogs on each session it builds. A client can
+then run `SELECT * FROM rest.ns.t`. Tables are pinned the same way as on the
+client, while the scheduler plans the query.
+
+## How it works
+
+Ballista sends the client's plan to the scheduler, and each stage's plan to
+the executors, as bytes. The codecs replace each live Iceberg table with one
+version of it: the location of its metadata file and its serialized `FileIO`.
+Metadata files never change, so every node rebuilds exactly that version.
+
+- **A catalog-backed table** is pinned to the version its catalog serves when
+  the client plans the query. DataFusion looks up each table a query names
+  once, so every scan of it, such as each side of a self-join, reads that
+  version, and each new query reads the latest one.
+- **A static table** is sent as the version it holds.
+- **A scan** is sent as the version the scheduler planned it against.
+
+Tables sent from the client also carry the metadata itself, so the scheduler
+needn't read the file. Scans sent to executors carry only the location, which
+keeps each task's plan small.
+
+Only the node that plans a query talks to a catalog: the client, or the
+scheduler for SQL clients. Executors never need catalog configuration.
+
+## Combining with other table formats
+
+`register_iceberg` and `with_iceberg` add the Iceberg codecs after the codecs
+already set, with `ComposedLogicalExtensionCodec` and DataFusion's
+`ComposedPhysicalExtensionCodec` (see
+[Combining Codecs](extending-components.md#combining-codecs)). To read tables
+of another format too, such as Delta, set its codecs first: on the session's
+`SessionConfig` before calling `register_iceberg`, and on `SchedulerConfig` and
+`ExecutorProcessConfig` before calling `with_iceberg`. Clients, the scheduler
+and executors must all set the same codecs in the same order.
+
+## Limitations
+
+- **Reads only.** `INSERT INTO` an Iceberg table fails with "isn't supported
+  on Ballista yet".
+- **No metadata tables.** Tables such as `t$snapshots` can't be sent to the
+  scheduler yet.
+- **One task per scan.** Iceberg scans do not split into partitions yet, so one
+  executor reads the whole table. Operators above the scan still run in

Review Comment:
   Taken as suggested in cd54fd58.



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to