Repository navigation
[CXH-2723] - Add Unity Catalog table and volume sync - Databricks Connector #73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: mateoHernandez123/unity-catalog-schema-sync
Are you sure you want to change the base?
Changes from all commits
acfcba8
01bab61
101ce0f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,6 +45,8 @@ func (d *Databricks) ResourceSyncers(_ context.Context) []connectorbuilder.Resou | |
| newMetastoreBuilder(d.client, d.clientID, workspaces, principals), | ||
| newCatalogBuilder(d.client, d.clientID, workspaces, principals), | ||
| newSchemaBuilder(d.client, d.clientID, workspaces, principals), | ||
| newTableBuilder(d.client, d.clientID, workspaces, principals), | ||
| newVolumeBuilder(d.client, d.clientID, workspaces, principals), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. O(schemas^2) API calls per catalog (schema_child.go:127-229). pushSchemaChildRoute runs on page 1 of every schema, once for tables and once for volumes. Each run lists every schema of the catalog through schemaThrough (50 per page) and repeats the uncached privilege probes. A catalog with 1,000 schemas makes about 2,000 x 20 schema-list pages, around 40k calls before any table is listed, which will hit rate limits. Fix: a single GET schemas/{catalog}.{schema}, or reuse the owner data the schema syncer already wrote onto the schema resource. Per-table cost in Grants (securable_helpers.go, schema_child.go:356). Each table does its own permissions read, and a non-owner connector adds an uncached effective-privileges read per table. Entitlements run at roughly 10 per table (table.go:285). With 100k tables that is 200k+ extra calls per sync. Consider trusting the route-level proof carried in the profile rather than re-reading per resource. I am a bit worried about the performance. Could you please confirm with it?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are right on the first one. Sergio flagged the same shape earlier in this PR and I answered that the uhttp GET cache kept it cheap. That was too comfortable: the cache is 5 MB shared by the whole sync, so it only holds while a catalog's entire schema listing fits in it. Measured against a simulated tenant, one catalog with 1,000 schemas: 43,024 requests with the cache off (your ~40k is right), 5,027 warm, and with 30 KB schema rows the listing count goes from 4 to 108 at only 200 schemas, with the default cache untouched. Fixed in 01bab61. Per-table cost: correct for a connector that does not own the metastore — 600 extra uncached effective-permissions reads over 200 schemas. This account owns the metastore, so it costs nothing today, and removing it needs the route memoized per catalog, which I would rather do separately. Entitlements does not apply: |
||
| } | ||
| } | ||
|
|
||
|
|
@@ -76,7 +78,7 @@ func (d *Databricks) Metadata(_ context.Context) (*v2.ConnectorMetadata, error) | |
| return &v2.ConnectorMetadata{ | ||
| DisplayName: "Databricks", | ||
| Description: "Connector syncing Databricks workspaces, users, groups, service principals and roles to Baton, " + | ||
| "along with Unity Catalog data assets — metastores, catalogs and schemas — and the privileges granted on them", | ||
| "along with Unity Catalog data assets — metastores, catalogs, schemas, tables and volumes — and the privileges granted on them", | ||
| AccountCreationSchema: &v2.ConnectorAccountCreationSchema{ | ||
| FieldMap: map[string]*v2.ConnectorAccountCreationSchema_Field{ | ||
| "email": { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: still says
TRAIT_APPfor table (and volume at :502), but resource_types.go dropped the trait. The capabilities workflow only regenerates on push to main, so CI won't catch it here. Worth regenerating so the file/docs match.