Repository navigation
branch-4.1: [feature](lance) Add Lance index admission, job inspection SQL and catalog DDL guard - #67630
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
6b1bbd6 to
602d1df
Compare
|
run buildall |
|
/review |
9cfaa4c to
35bdb48
Compare
|
/review |
|
run buildall |
|
/review |
|
run buildall |
There was a problem hiding this comment.
Requesting changes after a full three-round review of the authoritative 38-file diff at exact head dc54320b7501fe99e1f8a6fede3aeb16290b3f74 against verified base 14d3d957c02f79e5505879f9de60b3a4a2a05f1c.
The six P1 issues are: incomplete storage-routing identity can redirect an unresolved mutation; SHOW authorization can disclose provider failures and can authorize historical jobs against a different current dataset; the production mutation gate is enableable before the required isolation/possible-live/release boundary exists; the pinned Lance SDK cannot supply the primitive fixed-list contract as modeled; and case-distinct provider fields can resolve to and journal the wrong physical field.
The seven P2 issues cover system/user UUID collision handling, quoted top-level IF matching, database-name normalization in SHOW filtering, follower-local ANN admission limits, the implicit fixed num_bits=8 comparison, malformed numeric schema facts entering durable contracts, and irreversible regression-suite consumption of global unresolved-job quota. Inline comments contain the reproductions and narrow remediation requirements.
Critical checkpoints reviewed: parser and command routing, catalog target capture/ALTER/DROP guards, admission and IF semantics, job durability/quota/fence/replay behavior, SHOW authorization and confidentiality, pinned Java/JNI/Rust schema compatibility, configuration ownership, and unit/regression coverage. No additional user focus was supplied.
This is a static review only: the runner instructions prohibit local builds/tests. At submission time the GitHub-hosted style/license/dependency/formatter checks and reported BE/Cloud/coverage/performance checks pass (with expected skipped extension/third-party jobs); TeamCity COMPILE and FE UT remain pending.
| + "可降低导入内存压力并提升随机分桶表的吞吐量,覆盖所有导入类型。"}) | ||
| public static boolean enable_adaptive_random_bucket_load = true; | ||
|
|
||
| @ConfField(mutable = true, masterOnly = true, varType = VariableAnnotation.EXPERIMENTAL, description = { |
There was a problem hiding this comment.
[P1] Keep this slice impossible to enable in production. The accepted v5.1 contract (sections 9.1 and 9.7) says this is disabled FE mutation control and mutation cannot be enabled until hard worker isolation, finite possible-live limits, and the remaining evidence gates exist. mutable=true (and ordinary fe.conf loading) instead permits admission now; the warning confirms every resulting job/fence is durable but unresolvable because dispatch and FORCE_RELEASE are absent. Please use a test-only/internal seam for admission coverage and expose an enableable runtime setting only with the complete safety boundary.
There was a problem hiding this comment.
Keeping mutable=true for now: this is the same seam the dispatch and FORCE_RELEASE slices use as the production gate (roadmap: #66497 (comment)), and the regression suite enables it via ADMIN SET. A hard lock can land together with FORCE_RELEASE instead of freezing the seam twice — see #67630 (comment).
| "Max unresolved Lance index jobs across all catalogs (global)."}) | ||
| public static long lance_index_job_max_unresolved_global = 256; | ||
|
|
||
| @ConfField(mutable = true, masterOnly = false, |
There was a problem hiding this comment.
[P2] These admission limits should route to the admission owner. Lance mutation validation runs only after AlterTableCommand is forwarded to the master, but masterOnly=false lets ADMIN SET FRONTEND CONFIG on a follower succeed locally without changing what the master enforces. Mark both ANN bounds master-only (while retaining the explicit all-FE form when desired), or otherwise make the update cluster-wide, and test follower routing instead of locking in the false annotation.
There was a problem hiding this comment.
Fixed in 4fba663 — both ANN bounds are now masterOnly=true, matching where the validation actually runs.
| * Keys match case-insensitively because the catalog property chain performs no key | ||
| * normalization. | ||
| */ | ||
| private static final Set<String> LANCE_TARGET_IDENTITY_KEYS = ImmutableSet.of( |
There was a problem hiding this comment.
[P1] Include storage-routing properties in the target identity guard. s3.endpoint (and the OSS equivalent) is not a credential: changing it can make the same s3://bucket/path resolve on a different MinIO/S3 service. Jobs persist the URI but not the endpoint, and later reads/workers rebuild options from the current catalog, while this five-key fingerprint also misses the change during in-flight admission. Please fingerprint/guard the normalized effective non-secret routing identity (including every accepted endpoint alias, provider selector, and region-derived endpoint), while leaving access keys/tokens rotatable.
There was a problem hiding this comment.
Fixed in 4fba663 — the guard now fingerprints the 18 storage-routing keys the Lance property chain actually consumes (endpoint/region families across s3/oss/cos/obs/minio/gs/ozone); credentials stay rotatable.
| // 2. One pinned snapshot for every authoritative decision below. | ||
| CatalogMgr catalogMgr = Env.getCurrentEnv().getCatalogMgr(); | ||
| CatalogMgr.LanceIndexTarget target = catalogMgr.captureLanceIndexTarget(catalog); | ||
| LanceIndexAdmissionSnapshot snapshot = loader.load(catalog, db.getRemoteName(), table.getRemoteName()); |
There was a problem hiding this comment.
[P1] Reject ambiguous top-level field identities in this pinned snapshot before the IF preflight or contract selection. Lance accepts distinct A and a, but ExternalTable.getColumn() returns the first equalsIgnoreCase match; static validation and storedColumnName() can therefore select and journal the wrong field ID/type, while CREATE IF may falsely match before the builder runs. Detect collisions under the actual Doris lookup relation as well as the persisted normalization (ROOT-lowercasing alone misses some Unicode equalsIgnoreCase pairs), fail closed, and cover ASCII and quoted-Unicode cases.
There was a problem hiding this comment.
Fixed in de4daa4 — top-level field names colliding under the table's equalsIgnoreCase lookup relation now fail closed before the IF preflight and contract selection.
| if (logical.getColumns().size() != 1) { | ||
| return false; | ||
| } | ||
| String requestColumn = LanceIndexNameNormalizer.normalize(def.getCols().get(0)); |
There was a problem hiding this comment.
[P2] Compare the same field-name representation on both sides. Logical metadata escapes even a top-level name as a path segment (for example, a field named a.b becomes a backtick-quoted path), while the parser strips identifier quoting before putting raw a.b in def.getCols(). Thus a valid CREATE INDEX IF NOT EXISTS is rejected as a different definition although table lookup and schema-contract construction resolve the same field. Compare field identity/raw stored names, or canonicalize the request with the same path formatter, and test dots, spaces, and embedded backticks.
There was a problem hiding this comment.
Fixed in de4daa4 — the request column is converted to the same loader path-segment representation before comparison, so a name the parser unquotes compares equal to its escaped logical form.
| vectorElementType = canonicalType(element.getType()); | ||
| vectorElementNullable = element.isNullable(); | ||
| } | ||
| return new LanceIndexSchemaContract.IndexedField( |
There was a problem hiding this comment.
[P2] Validate provider-supplied numeric schema facts before journaling this contract. The pinned manifest-read path reconstructs protobuf fields without calling Schema.validate(), so a corrupt or historical manifest can reach Java with field.getId() < 0 or a fixed-list size <= 0; neither this builder nor IndexedField.validateForAdmission() rejects them. Reject negative IDs (zero is valid) and non-positive dimensions here, map the failure to the bounded Lance admission error, and add manifest-shaped cases.
There was a problem hiding this comment.
Fixed in d192e4d — negative field ids and non-positive fixed-list dimensions are rejected before the contract is journaled.
| if (ctlName != null && (catalog == null || !ctlName.equals(catalog.getName()))) { | ||
| return false; | ||
| } | ||
| if (dbName != null && !dbName.equals(job.getDbName())) { |
There was a problem hiding this comment.
[P2] Apply the catalog's database-name semantics to this filter. Admission persists the resolved local database name, but FROM keeps the user's identifier spelling and this exact equality runs before resolution. With lower_case_database_names=1 or 2, SHOW ... FROM DB1 can return no rows even though the catalog resolves DB1 to stored db1. Resolve/canonicalize the requested database through the selected catalog without weakening authorization, and cover both case modes.
There was a problem hiding this comment.
Fixed in a53722f — the FROM database name is now resolved through the catalog's own name semantics before the equality filter.
| job.getDbName(), job.getTableName(), PrivPredicate.SHOW); | ||
| } | ||
|
|
||
| static boolean targetResolves(CatalogIf<? extends DatabaseIf<? extends TableIf>> catalog, LanceIndexJob job) { |
There was a problem hiding this comment.
[P1] Authorize against the job's durable target, not just today's catalog/db/table names. After a terminal job releases the unresolved guard, a legal warehouse, namespace, or routing ALTER can repoint the same catalog id and names from locator A to dataset B. This helper then succeeds, so a user with SHOW on B can read A's historical job message, executor, and FORCE audit fields. Freshly resolve and revalidate the provider plus normalized effective target (including non-secret routing identity) against the job; treat mismatches or missing identity as ADMIN-only orphans while keeping credential rotation allowed.
There was a problem hiding this comment.
Fixed in a53722f — authorization additionally revalidates the job's persisted locator against the catalog's currently resolved dataset target; a mismatch degrades to the orphan/ADMIN-only path.
| if (catalog == null) { | ||
| return false; | ||
| } | ||
| DatabaseIf<? extends TableIf> db = catalog.getDbNullable(job.getDbName()); |
There was a problem hiding this comment.
[P1] Keep provider lookup failures inside the non-disclosing authorization boundary. getTableNullable() can initialize, list, or load external metadata and throw on expired credentials or an outage. For detail, that makes a valid job ID return a provider-specific error while a missing or unauthorized ID returns fixed 5103; for list, one bad target aborts every row. Treat lookup failure as an unresolved/orphan target through an exception-safe lookup boundary, so non-ADMIN callers learn nothing and ADMIN can still inspect the orphan.
There was a problem hiding this comment.
Fixed in a53722f — target resolution is exception-safe: any provider lookup failure counts as an unresolved target (fixed 5103 for detail, row omitted for list), so provider errors never leak through the authorization boundary.
| String externalEnvIp = context.config.otherConfigs.get("externalEnvIp") | ||
| String minioPort = context.config.otherConfigs.get("iceberg_minio_port") | ||
| String lanceRestPort = context.config.otherConfigs.get("lance_rest_port") | ||
| // Admitted jobs are durable and stay PENDING forever in this delivery slice: dispatch, |
There was a problem hiding this comment.
[P2] This test is not repeatable on the shared FE state it explicitly anticipates. A successful intended run leaves this catalog plus three permanently PENDING jobs (normal CREATE, quota-case CREATE, and DROP); the timestamp avoids name/fence collisions but every run still consumes the global unresolved quota. With the default 256 limit, 85 runs leave 255 entries and the next run cannot finish, with partial failures leaking sooner. Run this scenario only in disposable/resettable FE state or defer it until an authorized cleanup/release path exists.
There was a problem hiding this comment.
Not taking for this slice — the regression pipeline FE is per-run disposable, so CI runs don't accumulate against the global quota; the suite adopts RESOLVE-based cleanup when PR3E (#67754) lands — see #67630 (comment).
FE UT Coverage ReportIncrement line coverage |
|
Response plan per point: P1 – gate mutability (Config): keeping P1 – storage-routing identity (CatalogMgr): agreed — the endpoint-family keys the Lance catalog property chain actually consumes will join the target identity guard; access keys and tokens stay rotatable. P1 – SHOW authorization (both comments): agreed — target resolution becomes exception-safe (any provider lookup failure counts as an unresolved target, so non-ADMIN callers learn nothing and ADMIN still sees the orphan row), and authorization will additionally revalidate the job's persisted locator against the catalog's currently resolved dataset target. P1 – fixed-list contract vs pinned SDK: verifying empirically against a real local dataset on the pinned 9.1.0-beta.3 before choosing between narrowing the contract facts and an SDK-side path; findings will follow here. P2s: column case-collision fail-closed under the table-lookup relation, P2 – regression quota consumption: the regression pipeline FE is per-run disposable, so CI runs don't accumulate; the suite will adopt the RESOLVE-based cleanup as soon as PR3E (#67754) lands. |
|
P1 – fixed-list contract vs pinned SDK: findings and fix. Verified empirically on the pinned 9.1.0-beta.3 with a real on-disk dataset through the JNI path: The contract now sources element facts from the reconstructed Arrow view ( Landed in 4f887ac, together with the fixture migration and a real-dataset contract test. |
|
/review |
|
run buildall |
FE UT Coverage ReportIncrement line coverage |
FE Regression Coverage ReportIncrement line coverage |
|
run feut |
|
run external |
… guard Add the PR3C admission prerequisites: the enable_lance_index_mutation gate (mutable, masterOnly, EXPERIMENTAL, default off), three positive unresolved-job quotas (per-table/catalog/global, 8/64/256) and two positive static bounds (num_partitions 4096, num_sub_vectors 256) with assigning validators that reject non-positive values; block ALTER CATALOG identity-key value changes and DROP CATALOG on a Lance catalog with unresolved index jobs while leaving credentials, same-value rewrites, renames and replay unguarded; extend LanceIndexJobManager with getAllJobsSnapshot and hasUnresolvedJobsForCatalog.
Rewire the Lance branch of AlterTableCommand behind the enable_lance_index_mutation gate (default off): validate() keeps the static validation, rejects with the new 5102 error while the gate stays off (no metadata read, no id allocation), and otherwise collects the top-level CREATE/DROP INDEX op; run() admits it through the new LanceIndexAdmission and answers a single-column JobId result set — one row when admitted, zero rows on an IF no-op — bypassing the generic op loop and alterTable entirely (for ANN this also stops an internal index id from being burned on the rejected path). Admission works off the pinned admission snapshot only: reserved __lance_ names and ambiguous case-only collisions fail closed, the IF preflight compares the requested algorithm, the corroborating physical family, the single normalized column, and each whitelist property the snapshot actually exposes (metric case-folded, numeric properties by parsed value, num_partitions never), the schema contract is built from the stored column name, REPLACE/DROP persist the stored display name of a unique case-insensitive match, and exactly one id is allocated after every preflight passes, immediately before the durable createJob transfer whose fence/quota rejections pass through verbatim. A non-positive unresolved-job quota that slipped in through fe.conf (callbacks only fire on ADMIN SET) is asserted with 5102 before any allocation. The static layer gains the matching pieces: rejectMutationDisabled (5102), the reserved __lance_ prefix rejection shared with admission so CREATE/CREATE OR REPLACE/DROP all reject it in either gate mode, and the configured static upper bounds for num_partitions and num_sub_vectors (design section 2.4). New error codes: ERR_LANCE_INDEX_MUTATION_DISABLED (5102) and ERR_LANCE_INDEX_JOB_NOT_FOUND (5103).
Add the job inspection surface of the Lance index lifecycle: SHOW LANCE INDEX JOBS [FROM [catalog.]db] [WHERE TableName = "tbl" [AND State = "PENDING"]] lists the durable job records held by the master, and SHOW LANCE INDEX JOB <jobId> shows the single-job detail with the dispatch and FORCE audit fields. Job rows are authorized per persisted target: rows whose catalog or db/table no longer resolves (orphan and half-orphan) are visible to global ADMIN only and are omitted entirely for everyone else, leaking neither existence nor count; the detail command answers a missing job and an unauthorized job with the same fixed ERR_LANCE_INDEX_JOB_NOT_FOUND (5103) response naming only the job id. Locator, provider, normalized names, propertiesJson and the schema contract are never shown. Result and dispatch fields, which are all null before a worker exists, render as empty strings. The WHERE clause is deliberately narrowed to EqualTo predicates combined with AND over TableName/State (no Like), State values are validated against LanceIndexJobMutationState, and both commands run as FORWARD_NO_SYNC like the other master-replayed-state SHOW commands.
…-op case The external regression suite has failed on every run of this PR: the CREATE INDEX IF NOT EXISTS no-op case against the preloaded embedding_ivf_pq_f32 index omitted num_bits, and the authoritative preflight compares an omitted num_bits as the job-persisted 8, while the preloaded fixture genuinely persists num_bits=4 (lance_build_preinstalled_catalog.py PQ_BUILD_PARAMS) and describeIndices exposes it. The mismatch rejection is the designed fail-closed behavior; the test expectation was wrong. Carry num_bits=4 explicitly in the no-op request, turn the omitted num_bits into its own negative case (the effective-8 comparison now gets live coverage against real provider data), and extend the metric negative case to match the fixture definition as well.
|
run external |
|
run p0 |
…_PQ index num_bits=4 (the preloaded fixture's real value) is rejected by static validation with "num_bits must be 8" before admission ever runs, and an omitted num_bits compares as the job-persisted 8 and mismatches the on-disk 4 — so no SQL-valid request can produce a matching no-op against this fixture. Drop the unconstructible positive case and assert the fail-closed behavior at each layer instead: the static rejection, the effective-8 preflight mismatch, and the metric mismatch. The matching-definition no-op path stays covered by LanceIndexAdmissionTest against mocked snapshots.
|
run external |
FE Regression Coverage ReportIncrement line coverage |
|
run buildall |
|
@Gabriel39 @zhangstar333 @yiguolei This is ready for another pass when you have time. It builds on #67201 and #67235 (both merged); the feedback from the review rounds is addressed, and CI is green across the board (FE UT, P0, cloud_p0, external regression — including the Lance index admission suite). Given the branch-4.1 window, another review round or an approval would help land this. |
FE Regression Coverage ReportIncrement line coverage |
2 similar comments
FE Regression Coverage ReportIncrement line coverage |
FE Regression Coverage ReportIncrement line coverage |
|
/review |
FE Regression Coverage ReportIncrement line coverage |
|
PR approved by anyone and no changes requested. |
|
PR approved by at least one committer and no changes requested. |
…n SQL and catalog DDL guard (#67630) ## What problem does this PR solve? Issue: #66497 (design v5.1, slice 3). Target: `branch-4.1`. This is PR3C, building on #67201 (PR3A, DDL surface) and #67235 (PR3B, durable jobs), both already merged into branch-4.1. It adds the FE admission path for top-level Lance CREATE, CREATE OR REPLACE, and DROP INDEX, returns durable job IDs, and exposes job inspection SQL. **`enable_lance_index_mutation` remains false by default.** This slice does not execute index mutations: admitted jobs remain PENDING until dispatch/worker support and FORCE_RELEASE arrive in later slices. Enabling it now leaves unresolved jobs that prevent target-changing ALTER CATALOG and DROP CATALOG. ## Behavior - Admission reads dataset version, schema fields, logical indexes, and physical index families from one opened latest Dataset. It does not call `countRows()` or `getIndexStatistics()`. Schema contract v1 preserves Arrow field identity, nullability, vector dimensions/element types, and relevant type parameters for later worker revalidation. For fixed-size lists the pinned SDK reconstructs no child field — the element type lives only in the manifest logical type and element nullability is synthesized as nullable (a manifest-format fact that also binds existing datasets) — so the contract sources element facts from the reconstructed Arrow view and is pinned by a real on-disk dataset fixture. - Reserved names and ambiguous case-only collisions are rejected (reserved names fail before any metadata read). REPLACE and DROP persist the stored display name of a unique case-insensitive match. Column identities that are ambiguous under the table's case-insensitive lookup relation are rejected before anything is journaled. CREATE IF NOT EXISTS is a no-op only when algorithm, physical family, column (compared in the same path-segment representation on both sides), and exposed whitelist properties match — an omitted `num_bits` compares as the persisted 8; DROP IF EXISTS is a no-op when the authoritative name is absent. - Metric comparison ignores case. Numeric properties use strict parsed-long equality for numeric primitives or integer strings; fractions, exponent notation, overflow, and malformed values fail closed. Unexposed properties are skipped, and `num_partitions` is not compared. - An admitted statement returns one `JobId` row after the job and fence are durable; an IF no-op returns the same column with zero rows. The response works for direct and forwarded connections. - `SHOW LANCE INDEX JOBS [FROM [catalog.]db] [WHERE TableName = "t" [AND State = "PENDING"]]` and `SHOW LANCE INDEX JOB <id>` authorize each persisted target. Orphans — including targets whose provider lookup fails (credential expiry, outage) and targets whose currently resolved dataset locator no longer matches the job's persisted locator after a legal identity change — require global ADMIN; unauthorized rows are omitted, and unauthorized/missing job IDs share error 5103, so provider failures never leak through the authorization boundary. Locators, credentials, properties JSON, and schema-contract contents are not exposed. WHERE values accept what the parser actually produces for string literals, including `VarcharLiteral`; the `FROM` database name is canonicalized through the selected catalog's resolution semantics. ## Concurrency and configuration Catalog identity is captured under the CatalogMgr lock before the remote snapshot read. After the read, admission rechecks catalog existence, identity properties, and a local identity-change generation under the same lock used by catalog DDL, then allocates the ID and creates the job. This closes both the guard-check/create race and A → B → A identity changes. IF no-ops also revalidate the target. Identity changes invalidate in-flight reads even when a tentative ALTER is rolled back; same-value rewrites with unchanged key spelling and credential-only changes remain allowed. Metadata I/O runs outside the DDL lock. Final admission uses the lock order CatalogMgr → job manager → journal. DROP and changes to target-identity properties — `lance.catalog.type`, `warehouse`, namespace parent/delimiter/root_database, and the storage-routing endpoint/region keys the Lance property chain actually consumes — reject unresolved jobs; access keys and tokens stay rotatable. Renames and replay are not blocked by the unresolved-job guard. The gate and unresolved-job quotas (table/catalog/global defaults 8/64/256) are mutable and masterOnly. ADMIN SET changes runtime values on the master; it does not persist them to the custom configuration file or automatically synchronize other FEs. Restart loads configured file values/defaults, while promotion uses the promoted FE's own configuration. The two static bounds (`num_partitions` 4096, `num_sub_vectors` 256) are masterOnly — validation runs on the master — so a single-FE ADMIN SET takes effect only there; use ADMIN SET ALL FRONTENDS CONFIG to update the cluster. ADMIN SET callbacks validate positive quotas/bounds. File-loaded quotas are checked again before ID allocation; the job manager independently rejects non-positive limits. Gate-off admission uses error 5102. Job SHOW commands use FORWARD_NO_SYNC: follower requests execute on the master, without waiting for follower journal replay. This does not make SHOW a follower-local stale read. ## Validation - FE reactor compilation passed (JDK 17, Maven 3.9.9). - 262 tests passed across 16 focused FE test classes, with zero failures/errors/skips: configuration, catalog guard, admission, snapshots, index families, static validation, schema contracts, job queries, loader defenses, parser, command responses, and job authorization. Beyond the admission/concurrency cases (concurrent DROP and identity ALTER versus admission, A → B → A, failed ALTER rollback, replay with null old properties, target changes during IF no-ops, strict numeric parsing, real command-to-CatalogMgr integration), coverage includes WHERE predicate shapes through the real parser (LIKE, reversed literals, `VarcharLiteral` values, which the parser produces for regular string literals), reserved-name rejection without a snapshot read, half-orphan row hiding for non-ADMIN, gate-off persistence assertions, the exactly-one-operation precondition, multi-column stored indexes in the IF preflight, unparsable snapshot properties, the loader's physical-entry defenses (bounds, duplicate UUIDs recorded before the system-entry filter, system-entry filtering, ordering), malformed provider schema facts (negative field ids, non-positive dimensions), the catalog's identity epoch, failure sanitization and REST early rejection, storage-routing key guarding, SHOW locator revalidation and provider-failure non-disclosure, and a real on-disk Lance dataset fixture pinning the fixed-size-list contract semantics. - JaCoCo new-code line coverage: 93% on the new classes (each ≥ 86%), 81% on the new lines of modified files, 90% overall. The remainder is the JNI dataset-open path exercised by the external regression suite. - The full FE test suite (fe-common + fe-core + reactor dependencies, ~9.6k tests) ran to completion on both the pre-review and the review-fix heads: the only failures are this machine's documented timezone-, network- and native-library-related environmental tests — 38 failing methods across 15 classes, verified identical (same classes, same counts) by running the same classes on the merge base. - Maven validate/Checkstyle passed with zero violations; `git diff --check` passed. - Groovy compilation and a harness exercising the actual regression suite's original-value restoration and cleanup-failure propagation passed. - The external MinIO/REST regression was not run locally; the corresponding Docker environment is not running. BE compilation was not run. The external regression suite runs as `nonConcurrent`, captures the original master gate/quota values, and restores them even on failure. Cleanup failures are reported. Admitted PENDING jobs/catalogs remain durable in this slice; per-run names avoid same-name collisions, but repeated runs still consume the global unresolved-job quota. ## Deferred Dispatch, BE selection, worker execution, possible-live slot configuration, and the local/file operator assertion belong to subsequent dispatch/worker slices. FORCE_RELEASE/RESOLVE and retention GC belong to PR3E (#67754). The gate default remains unchanged. REST catalog mutations, ALTER TABLE ADD/DROP INDEX, and BUILD INDEX remain unsupported. ## Release note Experimental, default-disabled FE admission and job inspection for Lance index mutations; actual index execution remains deferred. --------- Co-authored-by: u70b3 <u70b3@users.noreply.github.com>
…e#66497) Per the updated direction in apache#66497 (apache#66497 (comment)), Lance index mutations move to synchronous SQL with bounded worker execution modeled on Iceberg rewrite_data_files, and a durable asynchronous job framework is no longer part of the initial implementation. This removes it outright rather than leaving it disabled behind a flag or maintaining two execution paths. Removed: - the durable Lance index job state machine, manager, persistent same-name fences, unresolved-job quotas, and job replay infrastructure introduced by apache#67235 - the job-specific parts of apache#67630: durable job creation and JobId result sets, SHOW LANCE INDEX JOB(S) with its grammar and plan wiring, and the catalog DDL guards tied to unresolved jobs - the three unresolved-job quota configuration keys, the job-specific tests, and the now-unreferenced job-locator resolver pair on the Lance catalog and client - stale comments in the retained validation files that still described the removed framework Kept and adapted for the synchronous path: - the privilege, schema, parameter, snapshot, and IF-condition validation (the LanceIndexAdmission preflight, the static validator, the schema-contract builder, and the target-aware catalog guard); a mutation that passes the whole preflight now ends in the shared not-supported rejection, while an IF preflight no-op completes as a plain success - read-only SHOW INDEX, the lance_index_entries TVF, and the statically validated DDL surface; enable_lance_index_mutation remains the gate - the schema-contract, name-normalization, and dataset-locator helpers, relocated out of the removed job package; the replay-path target guard keeps slimmed direct coverage Compatibility: enable_lance_index_mutation has defaulted to false since apache#67630 merged and no released build exposes it enabled, so no deployment can hold persisted job records. The OP_LANCE_INDEX_JOB_UPSERT (500) edit-log reader is removed and the code is not reused; the image chunk the old manager wrote is skipped through the existing deprecated-module list, so images from previous builds still load.
…e#66497) Per the updated direction in apache#66497 (apache#66497 (comment)), Lance index mutations move to synchronous SQL with bounded worker execution modeled on Iceberg rewrite_data_files, and a durable asynchronous job framework is no longer part of the initial implementation. This removes it outright rather than leaving it disabled behind a flag or maintaining two execution paths. Removed: - the durable Lance index job state machine, manager, persistent same-name fences, unresolved-job quotas, and job replay infrastructure introduced by apache#67235 - the job-specific parts of apache#67630: durable job creation and JobId result sets, SHOW LANCE INDEX JOB(S) with its grammar and plan wiring, and the catalog DDL guards tied to unresolved jobs - the three unresolved-job quota configuration keys, the job-specific tests, and the now-unreferenced job-locator resolver pair on the Lance catalog and client - stale comments in the retained validation files that still described the removed framework Kept and adapted for the synchronous path: - the privilege, schema, parameter, snapshot, and IF-condition validation (the LanceIndexAdmission preflight, the static validator, the schema-contract builder, and the target-aware catalog guard); a mutation that passes the whole preflight now ends in the shared not-supported rejection, while an IF preflight no-op completes as a plain success - read-only SHOW INDEX, the lance_index_entries TVF, and the statically validated DDL surface; enable_lance_index_mutation remains the gate - the schema-contract, name-normalization, and dataset-locator helpers, relocated out of the removed job package; the replay-path target guard keeps slimmed direct coverage Compatibility: enable_lance_index_mutation has defaulted to false since apache#67630 merged and no released build exposes it enabled, so no deployment can hold persisted job records. The OP_LANCE_INDEX_JOB_UPSERT (500) edit-log reader is removed and the code is not reused; the image chunk the old manager wrote is skipped through the existing deprecated-module list, so images from previous builds still load.
…e#66497) Per the updated direction in apache#66497 (apache#66497 (comment)), Lance index mutations move to synchronous SQL with bounded worker execution modeled on Iceberg rewrite_data_files, and a durable asynchronous job framework is no longer part of the initial implementation. This removes it outright rather than leaving it disabled behind a flag or maintaining two execution paths. Removed: - the durable Lance index job state machine, manager, persistent same-name fences, unresolved-job quotas, and job replay infrastructure introduced by apache#67235 - the job-specific parts of apache#67630: durable job creation and JobId result sets, SHOW LANCE INDEX JOB(S) with its grammar and plan wiring, the catalog DDL guards tied to unresolved jobs, and the Flight SQL JobId result-schema special case for Lance index DDL - the three unresolved-job quota configuration keys, the job-specific tests, and the now-unreferenced job-locator resolver pair and dataset-locator helper on the Lance catalog and client - stale comments in the retained validation files that still described the removed framework Kept and adapted for the synchronous path: - the privilege, schema, parameter, snapshot, and IF-condition validation (the LanceIndexAdmission preflight, the static validator, the schema-contract builder, and the target-aware catalog guard); a mutation that passes the whole preflight now ends in the shared not-supported rejection, while an IF preflight no-op completes as a plain success - read-only SHOW INDEX, the lance_index_entries TVF, and the statically validated DDL surface; enable_lance_index_mutation remains the gate - the schema-contract and name-normalization helpers, relocated out of the removed job package; the replay-path target guard keeps slimmed direct coverage Compatibility: enable_lance_index_mutation has defaulted to false since apache#67630 merged and no released build exposes it enabled, so no deployment can hold persisted job records. The OP_LANCE_INDEX_JOB_UPSERT (500) code is kept as a tombstone that is never reused: journals from builds that manually enabled the gate stay readable through a minimal consume-and-discard branch (the payload was Text-delimited JSON), and the image chunk the old manager wrote is skipped through the existing deprecated-module list, so metadata from previous builds still loads.
…e#66497) Per the updated direction in apache#66497 (apache#66497 (comment)), Lance index mutations move to synchronous SQL with bounded worker execution modeled on Iceberg rewrite_data_files, and a durable asynchronous job framework is no longer part of the initial implementation. This removes it outright rather than leaving it disabled behind a flag or maintaining two execution paths. Removed: - the durable Lance index job state machine, manager, persistent same-name fences, unresolved-job quotas, and job replay infrastructure introduced by apache#67235 - the job-specific parts of apache#67630: durable job creation and JobId result sets, SHOW LANCE INDEX JOB(S) with its grammar and plan wiring, the catalog DDL guards tied to unresolved jobs, and the Flight SQL JobId result-schema special case for Lance index DDL - the three unresolved-job quota configuration keys, the job-specific tests, and the now-unreferenced job-locator resolver pair and dataset-locator helper on the Lance catalog and client - stale comments in the retained validation files that still described the removed framework Kept and adapted for the synchronous path: - the privilege, schema, parameter, snapshot, and IF-condition validation (the LanceIndexAdmission preflight, the static validator, the schema-contract builder, and the target-aware catalog guard); a mutation that passes the whole preflight now ends in the shared not-supported rejection, while an IF preflight no-op completes as a plain success - read-only SHOW INDEX, the lance_index_entries TVF, and the statically validated DDL surface; enable_lance_index_mutation remains the gate - the schema-contract and name-normalization helpers, relocated out of the removed job package; the replay-path target guard keeps slimmed direct coverage Compatibility: enable_lance_index_mutation has defaulted to false since apache#67630 merged and no released build exposes it enabled, so no deployment can hold persisted job records. The OP_LANCE_INDEX_JOB_UPSERT (500) code is kept as a tombstone that is never reused: journals from builds that manually enabled the gate stay readable through a minimal consume-and-discard branch (the payload was Text-delimited JSON), and the image chunk the old manager wrote is skipped through the existing deprecated-module list, so metadata from previous builds still loads.
…e#66497) Per the updated direction in apache#66497 (apache#66497 (comment)), Lance index mutations move to synchronous SQL with bounded worker execution modeled on Iceberg rewrite_data_files, and a durable asynchronous job framework is no longer part of the initial implementation. This removes it outright rather than leaving it disabled behind a flag or maintaining two execution paths. Removed: - the durable Lance index job state machine, manager, persistent same-name fences, unresolved-job quotas, and job replay infrastructure introduced by apache#67235 - the job-specific parts of apache#67630: durable job creation and JobId result sets, SHOW LANCE INDEX JOB(S) with its grammar and plan wiring, the catalog DDL guards tied to unresolved jobs, and the Flight SQL JobId result-schema special case for Lance index DDL - the three unresolved-job quota configuration keys, the job-specific tests, and the now-unreferenced job-locator resolver pair and dataset-locator helper on the Lance catalog and client - stale comments in the retained validation files that still described the removed framework Kept and adapted for the synchronous path: - the privilege, schema, parameter, snapshot, and IF-condition validation (the LanceIndexAdmission preflight, the static validator, the schema-contract builder, and the target-aware catalog guard); a mutation that passes the whole preflight now ends in the shared not-supported rejection, while an IF preflight no-op completes as a plain success - read-only SHOW INDEX, the lance_index_entries TVF, and the statically validated DDL surface; enable_lance_index_mutation remains the gate - the schema-contract and name-normalization helpers, relocated out of the removed job package; the replay-path target guard keeps slimmed direct coverage Also fixes an Arrow Flight SQL error path exposed by that rejection: a statement forwarded by a follower FE that the master rejected kept the follower-local query state OK, so DorisFlightSqlProducer synthesized a success StatusResult for it. The forwarded proxy status is now checked before that result is produced, for direct and prepared execution alike. Compatibility: enable_lance_index_mutation has defaulted to false since apache#67630 merged and no released build exposes it enabled, so no deployment can hold persisted job records. The OP_LANCE_INDEX_JOB_UPSERT (500) code is kept as a tombstone that is never reused: journals from builds that manually enabled the gate stay readable through a minimal consume-and-discard branch (the payload was Text-delimited JSON), and the image chunk the old manager wrote is skipped through the existing deprecated-module list, so metadata from previous builds still loads.
…ework (#66497) (#68781) Per the updated direction in [#66497 (comment)](#66497 (comment)), Lance index mutations move to synchronous SQL with bounded worker execution. A durable asynchronous job framework is no longer part of the initial implementation, so this change reverts it explicitly rather than leaving it disabled or maintaining two execution paths. ### Removed - the durable Lance index job state machine, manager, same-name fences, unresolved-job quotas, and replay infrastructure introduced by #67235 - the job-specific parts of #67630: durable job creation and JobId results, `SHOW LANCE INDEX JOB(S)` with its grammar and plan wiring, and the catalog DDL guards tied to unresolved jobs - the Flight SQL JobId result-schema special case for Lance index DDL, so Flight advertises the same generic DDL status schema the statement now returns - the three `lance_index_job_max_unresolved_*` quota configuration keys, the job-specific tests, the now-unreferenced job-locator resolver pair, and the orphaned dataset-locator helper ### Preserved - read-only `SHOW INDEX` (#66637) and the `lance_index_entries` inspection TVF (#66671) - the statically validated DDL surface (#67201); `enable_lance_index_mutation` remains the mutation gate - the privilege, schema, parameter, snapshot, and IF-condition validation from #67630, rewired as pure admission preflight; a mutation that passes the whole preflight now ends in the shared not-supported rejection until the synchronous implementation lands - the schema-contract and name-normalization helpers, relocated out of the removed `job` package next to their remaining callers; the replay-path target guard keeps slimmed direct coverage ### Journal and image compatibility `enable_lance_index_mutation` has defaulted to `false` since #67630 merged, and no released build exposes it enabled, so no deployment can hold persisted job records unless someone manually enabled an undocumented, gated feature on an unreleased build. The edit-log operation `OP_LANCE_INDEX_JOB_UPSERT` (= 500) keeps its number as a tombstone and is never reused; its old Text-delimited JSON payload is consumed and ignored by a minimal discard branch in the journal reader, so edit logs from such a build stay readable without restoring any framework code. The image chunk the old manager wrote is skipped through the existing deprecated-module list, so images from previous builds still load. ### Evidence Focused FE suites (`mvn test -pl fe-common,fe-core -am -Dtest='*Lance*,DorisFlightSqlSchemaTest'`): 453 tests run, 0 failures, 0 errors, 6 skipped (known environment skips), checkstyle included, BUILD SUCCESS — the Flight schema suite is now in the filter (33 tests green, with the Lance DDL cases rewritten against the generic `StatusResult` schema) and a slimmed direct test covers the retained replay-path target guard (`hasLanceIdentityKeyChange` and its admission-epoch advance). Net change: 62 files, +400/−8922. Co-authored-by: u70b3 <u70b3@users.noreply.github.com>
…ework (#66497) (#68781) Per the updated direction in [#66497 (comment)](#66497 (comment)), Lance index mutations move to synchronous SQL with bounded worker execution. A durable asynchronous job framework is no longer part of the initial implementation, so this change reverts it explicitly rather than leaving it disabled or maintaining two execution paths. ### Removed - the durable Lance index job state machine, manager, same-name fences, unresolved-job quotas, and replay infrastructure introduced by #67235 - the job-specific parts of #67630: durable job creation and JobId results, `SHOW LANCE INDEX JOB(S)` with its grammar and plan wiring, and the catalog DDL guards tied to unresolved jobs - the Flight SQL JobId result-schema special case for Lance index DDL, so Flight advertises the same generic DDL status schema the statement now returns - the three `lance_index_job_max_unresolved_*` quota configuration keys, the job-specific tests, the now-unreferenced job-locator resolver pair, and the orphaned dataset-locator helper ### Preserved - read-only `SHOW INDEX` (#66637) and the `lance_index_entries` inspection TVF (#66671) - the statically validated DDL surface (#67201); `enable_lance_index_mutation` remains the mutation gate - the privilege, schema, parameter, snapshot, and IF-condition validation from #67630, rewired as pure admission preflight; a mutation that passes the whole preflight now ends in the shared not-supported rejection until the synchronous implementation lands - the schema-contract and name-normalization helpers, relocated out of the removed `job` package next to their remaining callers; the replay-path target guard keeps slimmed direct coverage ### Journal and image compatibility `enable_lance_index_mutation` has defaulted to `false` since #67630 merged, and no released build exposes it enabled, so no deployment can hold persisted job records unless someone manually enabled an undocumented, gated feature on an unreleased build. The edit-log operation `OP_LANCE_INDEX_JOB_UPSERT` (= 500) keeps its number as a tombstone and is never reused; its old Text-delimited JSON payload is consumed and ignored by a minimal discard branch in the journal reader, so edit logs from such a build stay readable without restoring any framework code. The image chunk the old manager wrote is skipped through the existing deprecated-module list, so images from previous builds still load. ### Evidence Focused FE suites (`mvn test -pl fe-common,fe-core -am -Dtest='*Lance*,DorisFlightSqlSchemaTest'`): 453 tests run, 0 failures, 0 errors, 6 skipped (known environment skips), checkstyle included, BUILD SUCCESS — the Flight schema suite is now in the filter (33 tests green, with the Lance DDL cases rewritten against the generic `StatusResult` schema) and a slimmed direct test covers the retained replay-path target guard (`hasLanceIdentityKeyChange` and its admission-epoch advance). Net change: 62 files, +400/−8922. Co-authored-by: u70b3 <u70b3@users.noreply.github.com>
What problem does this PR solve?
Issue: #66497 (design v5.1, slice 3). Target:
branch-4.1.This is PR3C, building on #67201 (PR3A, DDL surface) and #67235 (PR3B, durable jobs), both already merged into branch-4.1. It adds the FE admission path for top-level Lance CREATE, CREATE OR REPLACE, and DROP INDEX, returns durable job IDs, and exposes job inspection SQL.
enable_lance_index_mutationremains false by default. This slice does not execute index mutations: admitted jobs remain PENDING until dispatch/worker support and FORCE_RELEASE arrive in later slices. Enabling it now leaves unresolved jobs that prevent target-changing ALTER CATALOG and DROP CATALOG.Behavior
countRows()orgetIndexStatistics(). Schema contract v1 preserves Arrow field identity, nullability, vector dimensions/element types, and relevant type parameters for later worker revalidation. For fixed-size lists the pinned SDK reconstructs no child field — the element type lives only in the manifest logical type and element nullability is synthesized as nullable (a manifest-format fact that also binds existing datasets) — so the contract sources element facts from the reconstructed Arrow view and is pinned by a real on-disk dataset fixture.num_bitscompares as the persisted 8; DROP IF EXISTS is a no-op when the authoritative name is absent.num_partitionsis not compared.JobIdrow after the job and fence are durable; an IF no-op returns the same column with zero rows. The response works for direct and forwarded connections.SHOW LANCE INDEX JOBS [FROM [catalog.]db] [WHERE TableName = "t" [AND State = "PENDING"]]andSHOW LANCE INDEX JOB <id>authorize each persisted target. Orphans — including targets whose provider lookup fails (credential expiry, outage) and targets whose currently resolved dataset locator no longer matches the job's persisted locator after a legal identity change — require global ADMIN; unauthorized rows are omitted, and unauthorized/missing job IDs share error 5103, so provider failures never leak through the authorization boundary. Locators, credentials, properties JSON, and schema-contract contents are not exposed. WHERE values accept what the parser actually produces for string literals, includingVarcharLiteral; theFROMdatabase name is canonicalized through the selected catalog's resolution semantics.Concurrency and configuration
Catalog identity is captured under the CatalogMgr lock before the remote snapshot read. After the read, admission rechecks catalog existence, identity properties, and a local identity-change generation under the same lock used by catalog DDL, then allocates the ID and creates the job. This closes both the guard-check/create race and A → B → A identity changes. IF no-ops also revalidate the target. Identity changes invalidate in-flight reads even when a tentative ALTER is rolled back; same-value rewrites with unchanged key spelling and credential-only changes remain allowed.
Metadata I/O runs outside the DDL lock. Final admission uses the lock order CatalogMgr → job manager → journal. DROP and changes to target-identity properties —
lance.catalog.type,warehouse, namespace parent/delimiter/root_database, and the storage-routing endpoint/region keys the Lance property chain actually consumes — reject unresolved jobs; access keys and tokens stay rotatable. Renames and replay are not blocked by the unresolved-job guard.The gate and unresolved-job quotas (table/catalog/global defaults 8/64/256) are mutable and masterOnly. ADMIN SET changes runtime values on the master; it does not persist them to the custom configuration file or automatically synchronize other FEs. Restart loads configured file values/defaults, while promotion uses the promoted FE's own configuration. The two static bounds (
num_partitions4096,num_sub_vectors256) are masterOnly — validation runs on the master — so a single-FE ADMIN SET takes effect only there; use ADMIN SET ALL FRONTENDS CONFIG to update the cluster.ADMIN SET callbacks validate positive quotas/bounds. File-loaded quotas are checked again before ID allocation; the job manager independently rejects non-positive limits. Gate-off admission uses error 5102.
Job SHOW commands use FORWARD_NO_SYNC: follower requests execute on the master, without waiting for follower journal replay. This does not make SHOW a follower-local stale read.
Validation
VarcharLiteralvalues, which the parser produces for regular string literals), reserved-name rejection without a snapshot read, half-orphan row hiding for non-ADMIN, gate-off persistence assertions, the exactly-one-operation precondition, multi-column stored indexes in the IF preflight, unparsable snapshot properties, the loader's physical-entry defenses (bounds, duplicate UUIDs recorded before the system-entry filter, system-entry filtering, ordering), malformed provider schema facts (negative field ids, non-positive dimensions), the catalog's identity epoch, failure sanitization and REST early rejection, storage-routing key guarding, SHOW locator revalidation and provider-failure non-disclosure, and a real on-disk Lance dataset fixture pinning the fixed-size-list contract semantics.git diff --checkpassed.The external regression suite runs as
nonConcurrent, captures the original master gate/quota values, and restores them even on failure. Cleanup failures are reported. Admitted PENDING jobs/catalogs remain durable in this slice; per-run names avoid same-name collisions, but repeated runs still consume the global unresolved-job quota.Deferred
Dispatch, BE selection, worker execution, possible-live slot configuration, and the local/file operator assertion belong to subsequent dispatch/worker slices. FORCE_RELEASE/RESOLVE and retention GC belong to PR3E (#67754). The gate default remains unchanged. REST catalog mutations, ALTER TABLE ADD/DROP INDEX, and BUILD INDEX remain unsupported.
Release note
Experimental, default-disabled FE admission and job inspection for Lance index mutations; actual index execution remains deferred.