Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Cache Iceberg column types in Glue for faster access #18315

Merged
merged 12 commits into from
Aug 1, 2023
Merged

Cache Iceberg column types in Glue for faster access #18315

merged 12 commits into from
Aug 1, 2023

Conversation

findepi
Copy link
Member

@findepi findepi commented Jul 17, 2023 •
edited
Loading

Make information_schema.columns queries faster by storing the
necessary directly in Glue so that loading Iceberg metadata from storage
is not need.

  • Table comment is stored as comment Glue table parameter.

  • Column Trino type is stored as Glue column type (for some cases) or as
    trino_type column table parameter. This is because, following
    Iceberg own's org.apache.iceberg.aws.glue.IcebergToGlueConverter.toTypeString
    the column type written to Glue is not accurate, so this piece of
    information may be lossy. In such cases the column parameter is used
    to store the Trino type.

    • For more compatibility we could store Iceberg type string, but
      Iceberg lacks API to reconstruct Type from string (except for
      primitive types). This is not surprising, as it needs IDs for all
      fields. Something that is not needed to answer metadata queries.

  • Column NOT NULL constraint is stored as trino_not_null=true column table
    parameter (omitted for nullable columns).

Before the above cached information is used, the following conditions
are checked

  • the trino_table_metadata_info_valid_for table property must be set
    to current metadata location. This ensures that the cached information
    is invalided whenever metadata location changes.

  • at least one column must have trino_type property set. This ensures
    the cached information is not used when column parameters were lost in
    transit or otherwise erased.

iceberg.glue.cache-table-metadata serves as a kill-switch for the new
functionality (both write & read parts).

Alternative to #18299

@cla-bot cla-bot bot added the cla-signed label Jul 17, 2023
@findepi findepi force-pushed the findepi/iceberg-is-columns-2 branch from da9a649 to dcc9e29 Compare July 17, 2023 15:51
@github-actions github-actions bot added tests:hive iceberg Iceberg connector hive Hive connector labels Jul 17, 2023
@findepi findepi force-pushed the findepi/iceberg-is-columns-2 branch from dcc9e29 to bb6dbc6 Compare July 17, 2023 19:27
@findepi findepi mentioned this pull request Jul 17, 2023
Merged
@findepi findepi force-pushed the findepi/iceberg-is-columns-2 branch from bb6dbc6 to 9a259ca Compare July 17, 2023 19:37
@findepi findepi mentioned this pull request Jul 18, 2023
Closed
@findepi findepi force-pushed the findepi/iceberg-is-columns-2 branch 3 times, most recently from 1fb31bf to e2c3fa7 Compare July 20, 2023 09:26
@findepi
Copy link
Member Author

findepi commented Jul 20, 2023

(just rebased)

@findepi findepi force-pushed the findepi/iceberg-is-columns-2 branch 3 times, most recently from b187e54 to 0d1de42 Compare July 20, 2023 14:15
@findepi findepi force-pushed the findepi/iceberg-is-columns-2 branch from 0d1de42 to 0f4a14c Compare July 20, 2023 19:42
@findepi findepi changed the title Write Iceberg types to Glue metastore (v2) Cache Iceberg table types in Glue for faster access Jul 20, 2023
@findepi
Copy link
Member Author

findepi commented Jul 20, 2023

cc @pettyjamesm @phd3

@findepi findepi force-pushed the findepi/iceberg-is-columns-2 branch 2 times, most recently from 14c180e to 9973336 Compare July 20, 2023 20:27
@ebyhr
Copy link
Member

ebyhr commented Aug 1, 2023

CI hit #18446

@findepi
Copy link
Member Author

findepi commented Aug 1, 2023

IcebergMetadata#streamTableColumns calls TrinoCatalog#listTables. The method internally callsGetTablesResult that returns com.amazonaws.services.glue.model.Table.

That's a very good point.

@findepi
Copy link
Member Author

findepi commented Aug 1, 2023

will rebase and add a test.
last build https://github.com/trinodb/trino/actions/runs/5716561690/job/15493176390?pr=18315 is "green"

@findepi findepi force-pushed the findepi/iceberg-is-columns-2 branch from 7c32725 to aad0ca1 Compare August 1, 2023 07:31
@findepi
Copy link
Member Author

findepi commented Aug 1, 2023

(just rebased)

@findepi
Reuse assertMultisetsEqual
1b6bf4a
@findepi
Suppress IDE warning about Integer unboxing
a9de28a
@findepi
Build delta Multiset directly
e8fd7b6 
No need to build intermediate `Map<.., Integer>`.
@findepi
Test Iceberg information_schema.columns Glue accesses
d4ecbc0
@findepi
Test Iceberg information_schema.columns file accesses with Glue
5875206
@findepi
Rename variable
96e1419 
Outdated after project rename.
@findepi
Separate file metastore column serialization from internals
3e24aef 
Introduce separate column class representing column serialized in file
metastore.
@findepi
Pass metadata and its location to getTableInput in Glue Catalog
3335b56
@findepi
Report metastore recording path when load fails
823f88f
@findepi
Add metastore column properties
4be2e54
@findepi
Cache Iceberg column types in Glue for faster access
63c10e5 
Make `information_schema.columns` queries faster by storing the
necessary information directly in Glue so that loading Iceberg metadata
from storage is not need.

- Table comment is stored as `comment` Glue table parameter.

- Column Trino type is stored as Glue column type (for some cases) or as
  `trino_type` column table parameter. This is because, following
  Iceberg own's `org.apache.iceberg.aws.glue.IcebergToGlueConverter.toTypeString`
  the column type written to Glue is not accurate, so this piece of
  information may be lossy. In such cases the column parameter is used
  to store the Trino type.
  - For more compatibility we could store Iceberg type string, but
    Iceberg lacks API to reconstruct Type from string (except for
    primitive types). This is not surprising, as it needs IDs for all
    fields. Something that is not needed to answer metadata queries.

- Column NOT NULL constraint is stored as `trino_not_null=true` column table
  parameter (omitted for nullable columns).

Before the above cached information is used, the following conditions
are checked

- the `trino_table_metadata_info_valid_for` table property must be set
  to current metadata location. This ensures that the cached information
  is invalided whenever metadata location changes.

- at least one column must have `trino_type` property set. This ensures
  the cached information is not used when column parameters were lost in
  transit or otherwise erased.

`iceberg.glue.cache-table-metadata` serves as a kill-switch for the new
functionality (both write & read parts).
@findepi findepi force-pushed the findepi/iceberg-is-columns-2 branch from aad0ca1 to 63c10e5 Compare August 1, 2023 08:40
@findepi
Copy link
Member Author

findepi commented Aug 1, 2023

test added (io.trino.plugin.iceberg.catalog.glue.TestIcebergGlueCatalogAccessOperations#testInformationSchemaColumns)

@findepi
Copy link
Member Author

findepi commented Aug 1, 2023

CI hit #18446

again.

@findepi findepi merged commit 7b147f4 into master Aug 1, 2023
107 of 108 checks passed
@findepi findepi deleted the findepi/iceberg-is-columns-2 branch August 1, 2023 12:01
@findepi findepi mentioned this pull request Aug 1, 2023
Closed
@github-actions github-actions bot added this to the 423 milestone Aug 1, 2023
@colebow colebow mentioned this pull request Aug 2, 2023
Merged
This was referenced Aug 3, 2023
Merged
Draft
@findepi findepi mentioned this pull request Sep 19, 2023
Closed
This was referenced Oct 31, 2024
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ebyhr ebyhr ebyhr approved these changes

@electrum electrum Awaiting requested review from electrum

@losipiuk losipiuk Awaiting requested review from losipiuk

@findinpath findinpath Awaiting requested review from findinpath

@alexjo2144 alexjo2144 Awaiting requested review from alexjo2144

@homar homar Awaiting requested review from homar

Assignees
No one assigned
Labels
cla-signed hive Hive connector iceberg Iceberg connector
Milestone
423
Development

Successfully merging this pull request may close these issues.
5 participants
@findepi @alexjo2144 @ebyhr @homar @findinpath