shaoyu-li opened a new issue, #12431:
URL: https://github.com/apache/gravitino/issues/12431
### Describe the feature
Introduce an SPI that lets a deployment delegate the **Lance data-plane
operations** performed by
`LanceTableOperations` — creating, opening, altering, indexing and dropping
the underlying Lance
dataset — to an implementation of its own choosing, so that the Gravitino
server can run as a
metadata-only service and the actual dataset manipulation happens outside
its process.
The metadata contract would not change: Gravitino would still own the entity
store, still order
metadata and data operations the way it does today, and still decide *what*
must happen. The
proposal is only about making *who executes it* pluggable.
### Motivation
Today the Gravitino server links the Lance Java SDK directly and performs
dataset operations
in-process. Concretely, in
`catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/lance/LanceTableOperations.java`:
- `createTable` calls `Dataset.write(...).mode(WriteMode.CREATE)`, writing
the dataset manifest to
object storage;
- `dropTable` / `purgeTable` call `Dataset.drop(location, storageOptions)`;
- `alterTable` calls `Dataset.open(...)` and then `dropColumns` /
`alterColumns` / `createIndex`;
- `loadTable` opens the dataset to read its Arrow schema and version for the
schema-refresh path.
`org.lance:lance-core` is a JNI wrapper over a native library, and it is on
the server's runtime
classpath by construction: `core/build.gradle.kts` declares
`implementation(libs.lance)`, and
`catalogs/catalog-lakehouse-generic/build.gradle.kts` declares
`compileOnly(libs.lance) // This will be provided by core module at runtime`.
`core/src/main/java/org/apache/gravitino/stats/storage/LancePartitionStatisticStorage.java`
opens,
scans and appends to datasets in the server process as well.
This works, and for many deployments it is the right default. In ours it
creates three problems:
**1. The metadata server becomes a storage-data-plane principal.** Because
it writes and deletes
datasets itself, the server must hold credentials for the storage location
of every table it
serves — supplied through `lance.storage.*` catalog and table properties.
That gives a single
service broad read/write/delete authority over data it otherwise never needs
to touch, and it makes
per-tenant credential isolation hard to express: the blast radius of a
compromise or a bug is every
bucket the catalog knows about.
**2. Native-library coupling and blast radius.** A JNI crash, a native
abort, or exhaustion of the
off-heap Arrow allocator does not fail one request — it can take down the
metadata service for
every catalog and every format it hosts. The Lance version is also pinned
process-wide, while the
engines reading and writing these tables (Spark, Ray, Python) ship their
own. Upgrading Lance
becomes a coordinated upgrade of the metadata service rather than of the
compute that actually
uses it.
**3. There is no supported way to opt out.** This is the part we would most
like to fix, and it is
worth being precise about, because the module *does* have a ServiceLoader
seam that looks like it
should work and does not:
`LakehouseTableDelegatorFactory` builds its registry with
```java
Collectors.toMap(provider -> provider.get().tableFormat(),
ServiceLoader.Provider::get)
```
Since `LanceTableDelegator` is registered in the module's own
`META-INF/services` file, a
deployment that adds its own delegator for the `lance` format produces a
duplicate key, and
`Collectors.toMap` throws `IllegalStateException` during static
initialization — the server fails
to start rather than picking up the override. The existing SPI can therefore
add a *new* format,
but it cannot replace the behaviour of a built-in one.
We are aware of the metadata-only escape hatches that already exist —
`external=true`,
`lance.table.register=true`, and `lance.declared=true`. They are useful, but
they are per-operation
flags set by the caller, they do not cover `alterTable` or index creation,
and they push the
responsibility onto every client rather than letting an operator configure
the behaviour once for a
catalog. They are not a substitute for a deployment-level execution mode.
That leaves forking the catalog module, which we would rather not do.
### Describe the solution
A `ServiceLoader`-discovered SPI, selected per catalog by a property,
covering exactly the dataset
operations `LanceTableOperations` performs today — no more:
```java
public interface LanceDatasetExecutor extends Closeable {
/** Name used to select this executor via the catalog property. */
String name();
void initialize(Map<String, String> catalogProperties);
/** Creates the dataset with the given Arrow schema; returns its initial
version. */
DatasetInfo createDataset(CreateDatasetRequest request);
/** Reads the dataset's current Arrow schema and version, for the
schema-refresh path. */
DatasetInfo describeDataset(DescribeDatasetRequest request);
/** Applies column and index changes; returns the resulting version. */
DatasetInfo applyChanges(ApplyChangesRequest request);
/** Deletes the dataset. */
void dropDataset(DropDatasetRequest request);
}
```
The request objects carry only what the current code already passes to the
SDK — the table's
`NameIdentifier`, the resolved `location`, the resolved storage options,
and, depending on the
call, the Arrow schema or the list of `TableChange`s. `DatasetInfo` carries
the Arrow schema and
the dataset version, which is what `LanceTableOperations` reads back today.
The default implementation is the current in-process one: the existing
`Dataset.write` /
`Dataset.open` / `Dataset.drop` calls, moved behind the interface unchanged.
It stays the default
when no executor is configured, so nothing changes for anyone who does not
opt in.
### Wiring
- Discovered with `ServiceLoader`, following the `LakehouseTableDelegator` /
`LakehouseTableDelegatorFactory` and `CredentialProvider` /
`CredentialProviderFactory` patterns
already in the codebase.
- Selected per catalog via a property, e.g. `lance-dataset-executor =
<name>`.
- `LanceTableOperations` keeps all of its current logic — ordering relative
to the entity store,
the `TableAlreadyExistsException` cleanup, the `external` / `register` /
`declared` checks, the
schema-refresh CAS retries — and only the SDK calls move behind the
interface.
### Ordering and failure semantics
We propose keeping today's semantics exactly, since they are already
load-bearing:
- create: dataset first, then metadata; if metadata creation fails with
`TableAlreadyExistsException`, the dataset is dropped (this is the
existing cleanup path);
- drop/purge: metadata first, then dataset, and only if metadata removal
succeeded;
- alter: dataset first, then metadata, so a failed change is not recorded.
A remote executor makes the failure windows more visible than they are
in-process, but it does not
introduce them — `handleLanceTableChange` already documents that it is not
atomic across multiple
changes. We are not proposing to change that here.
### Scope for a first iteration
- **In scope:** the four dataset operations above, as used by `createTable`,
`loadTable`,
`alterTable`, `dropTable` and `purgeTable` in `LanceTableOperations`.
- **Out of scope:** `LancePartitionStatisticStorage` in `core`. It has the
same in-process
characteristic and would benefit from the same treatment, but it already
has its own
`LancePartitionStatisticStorageFactory` seam, so it is separable and
better handled on its own.
- **Out of scope:** generalizing this to other formats. Delta has the same
shape of problem and the
interface could be made format-neutral later, but we would rather validate
the pattern on one
format first than design an abstraction for two at once.
### Backward compatibility
Fully preserved. With no `lance-dataset-executor` configured, the built-in
in-process executor runs
and every code path behaves exactly as it does today. This is a refactor
plus an extension point,
not a behaviour change.
### Additional context
- **Relevant code:**
`catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/lance/LanceTableOperations.java`
(`createTable`, `loadTable`, `alterTable`, `dropTable`, `purgeTable`,
`handleLanceTableChange`,
`openDataset`);
`catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/generic/LakehouseTableDelegatorFactory.java`
(the duplicate-key limitation described above);
`core/build.gradle.kts` and
`catalogs/catalog-lakehouse-generic/build.gradle.kts` (how the Lance
SDK reaches the server's runtime classpath).
- We recognize this touches a core responsibility of the catalog and is
larger than a typical
feature request. We are happy to write a design document or bring it to
the dev mailing list
first if maintainers prefer, and we are willing to contribute the
implementation.
- Separately, the duplicate-key behaviour in
`LakehouseTableDelegatorFactory` means a stray
delegator on the classpath fails server startup with an
`IllegalStateException` that does not
name the conflicting format. That is arguably a small bug in its own
right; we can file it
separately if that is useful.
- We searched existing issues and did not find one covering out-of-process
execution of Lance
dataset operations; apologies if we missed a prior discussion.
--
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]
