FANNG1 opened a new issue, #13340:
URL: https://github.com/apache/gravitino/issues/13340
### What would you like to be improved?
Part of #12485, companion to #13339.
Schema freshness versus cost is currently a deployment-wide switch.
`LanceTableOperations.loadTableInternal` is driven by one catalog property,
`lance.schema-refresh-mode` (`LanceConstants.java:43`,
`GenericCatalogPropertiesMetadata.java:57-64`), with two modes:
- `DECLARED_AND_EMPTY` (default) — opens the dataset only for declared-only
tables and tables with no stored columns; ordinary loads return the cached
schema, which may be stale.
- `VERSION_CHECK` — opens the dataset on every `loadTable` to compare
versions: accurate, but one object-store round trip per request.
A single setting has to serve two callers that want opposite things: the
engine-facing Lance REST hot path, where most requests need only location and
storage options and cannot afford a dataset open each time, and the
governance-facing Gravitino REST path, where a caller that asks for a schema
should never be handed a stale one.
Concretely:
1. **The caller's intent is already on the wire, and then discarded.**
`GravitinoLanceTableOperations.describeTable` receives `loadDetailedMetadata`
and uses it only to decide whether to *put* the schema into the response
(`:137-141`). The `loadTable` call above it (`:132`) is identical either way,
so under `VERSION_CHECK` a describe that does not want the schema still pays
for the dataset open.
2. **Pure metadata callers pay the same price.** `deregisterTable` (`:280`)
and `dropTable` (`:343`) call `loadTable` only to read `location` and
`properties`. `LanceTableOperations.purgeTable` already works around this by
calling `super.loadTable` directly to skip the refresh — an ad-hoc light load
that exists because there is no real one.
3. **The mode has to be woven through everything else.** `loadTableInternal`
(`:452-517`) mixes the mode with `declaredOnly`, `emptySchema`, the "confirmed
empty" version sentinel and a `forAlter` flag into a chain of five early
returns. Every new caller adds another condition.
### How should we improve?
Replace the switch with two explicit load paths and let the caller pick:
```
light load entity store only, never opens the dataset
-> location, storage options, properties, cached columns as-is
full load light load + open dataset + version check, always
-> schema and indexes guaranteed to match the dataset
```
- The version check stops being configurable: light never checks, full
always checks. `lance.schema-refresh-mode`, the `SchemaRefreshMode` enum and
`schemaRefreshMode()` are removed, along with their entries in
`docs/lakehouse-generic-catalog.md:51` and the schema-refresh-mode section of
`docs/lakehouse-generic-lance-table.md`.
- Route the existing callers: `describeTable` selects by the
`loadDetailedMetadata` flag it already receives; `deregisterTable` /
`dropTable` / `purgeTable` use light load, which removes the `super.loadTable`
workaround; `alterTable` uses full load, which removes the `forAlter` flag.
- Whatever freshness/repair work remains belongs to the schema updater in
#13339 — this issue only decides which path invokes it.
A sketch worth evaluating for where the split lives: keep
`TableCatalog.loadTable` as the full load, so every existing caller's semantics
are unchanged, and add the light load as an internal Lance-specific interface
reached through the dispatchers. No public API change, and callers choose per
request instead of per deployment.
Related: #12485, #13339, #11295.
--
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]
