[#11133] feat(iceberg): Enable table metadata cache by default and increase default capacity by lasdf1234 · Pull Request #11156 · apache/gravitino

lasdf1234 · 2026-05-19T17:25:31Z

What changes were proposed in this pull request?

Default table-metadata-cache-impl to org.apache.gravitino.iceberg.common.cache.LocalTableMetadataCache in IcebergConfig.
Raise default table-metadata-cache-capacity from 200 to 1000 in IcebergConfig.
Update IRC and lakehouse-iceberg catalog docs to match the new defaults.
Align IcebergCatalogPropertiesMetadata defaults with IcebergConfig.
Add TestIcebergConfig.testTableMetadataCacheDefaults to assert the new defaults.

IcebergCatalogWrapper already loads the configured cache implementation when table-metadata-cache-impl is set; with the new config default, deployments no longer silently fall back to TableMetadataCache.DUMMY unless the property is explicitly cleared.

Why are the changes needed?

Fixes #11133

TABLE_METADATA_CACHE_IMPL has no default today, so every deployment that does not set the property uses TableMetadataCache.DUMMY in IcebergCatalogWrapper.loadTableMetadataCache and reads metadata.json from object storage on every loadTable. The default capacity of 200 is also too small for typical enterprise working sets.

Does this PR introduce any user-facing change?

Yes.

table-metadata-cache-impl defaults to LocalTableMetadataCache when not configured.
table-metadata-cache-capacity defaults to 1000 when not configured.

How was this patch tested?

./gradlew :iceberg:iceberg-common:spotlessApply :catalogs:catalog-lakehouse-iceberg:spotlessApply
./gradlew :iceberg:iceberg-common:test -PskipITs -PskipDockerTests=true --tests "org.apache.gravitino.iceberg.common.TestIcebergConfig"

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR makes Iceberg table metadata caching enabled-by-default with a larger default capacity, updates docs/catalog metadata to reflect the new defaults, and makes cache initialization tolerant of catalogs that don’t support metadata locations.

Changes:

Set defaults for table-metadata-cache-impl (LocalTableMetadataCache) and increase default capacity to 1000.
Change cache initialization to warn + skip (use DUMMY) when the catalog doesn’t support SupportsMetadataLocation instead of throwing.
Update docs and catalog property metadata; add a unit test validating the new defaults.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
iceberg/iceberg-common/src/test/java/org/apache/gravitino/iceberg/common/TestIcebergConfig.java	Adds assertions for new cache default impl/capacity.
iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java	Avoids failing fast; skips metadata cache for unsupported catalogs with a warning.
iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/IcebergConfig.java	Defines default cache implementation + increases default capacity.
docs/lakehouse-iceberg-catalog.md	Documents new defaults for cache implementation and capacity.
docs/iceberg-rest-service.md	Documents new defaults for REST cache implementation and capacity.
catalogs/catalog-lakehouse-iceberg/src/main/java/org/apache/gravitino/catalog/lakehouse/iceberg/IcebergCatalogPropertiesMetadata.java	Updates catalog property defaults to match new cache defaults.

Default table-metadata-cache-impl to LocalTableMetadataCache and raise table-metadata-cache-capacity to 1000 in IcebergConfig and docs. Co-authored-by: Cursor <cursoragent@cursor.com>

Keep only the table-metadata-cache-capacity default change (1000). Co-authored-by: Cursor <cursoragent@cursor.com>

Default table-metadata-cache-impl to LocalTableMetadataCache and raise table-metadata-cache-capacity to 1000 per issue apache#11133. Co-authored-by: Cursor <cursoragent@cursor.com>

…adata Assert table metadata cache defaults in TestIcebergConfig and update IcebergCatalogPropertiesMetadata to match IcebergConfig. Co-authored-by: Cursor <cursoragent@cursor.com>

…aults Assert catalog property metadata defaults align with IcebergConfig. Co-authored-by: Cursor <cursoragent@cursor.com>

lasdf1234 · 2026-05-20T07:51:47Z

@yuqi1129 If you have time, please take a moment to review it for me. Thank you.

yuqi1129 · 2026-05-20T08:02:09Z

                "Table metadata cache implementation",
                false /* immutable */,
-                null /* defaultValue */,
+                LocalTableMetadataCache.class.getName() /* defaultValue */,


I did not see this class in the PR, so has it already existed in the code? Why did we set it as default value before.

This cache already exists in the code.

Why didn't we set it as the default value before? Do you have some informations on this part?

TABLE_METADATA_CACHE_IMPL has no default value, causing every default deployment to fall back to TableMetadataCache.DUMMY. Every loadTable call reads metadata.json from object storage on every request, with no caching. Latency degrades silently as commit history grows.

yuqi1129 · 2026-05-20T08:02:35Z

 |--------------------------------------------------------------|---------------------------------------------|---------------|----------|---------------|
-| `gravitino.iceberg-rest.table-metadata-cache-impl`           | The implement of the cache.                 | (none)        | No       | 1.1.0         |
-| `gravitino.iceberg-rest.table-metadata-cache-capacity`       | The capacity of table metadata cache.       | 200           | No       | 1.1.0         |
+| `gravitino.iceberg-rest.table-metadata-cache-impl`           | The implement of the cache.                 | `org.apache.gravitino.iceberg.common.cache.LocalTableMetadataCache` | No       | 1.1.0         |


format the table

The formatting has been revised.

yuqi1129 · 2026-05-20T08:02:46Z

 |---------------------------------------|---------------------------------------------|---------------|----------|---------------|
-| `table-metadata-cache-impl`           | The implement of the cache.                 | (none)        | No       | 1.1.0         |
-| `table-metadata-cache-capacity`       | The capacity of table metadata cache.       | 200           | No       | 1.1.0         |
+| `table-metadata-cache-impl`           | The implement of the cache.                 | `org.apache.gravitino.iceberg.common.cache.LocalTableMetadataCache` | No       | 1.1.0         |


The formatting has been revised.

yuqi1129 · 2026-05-20T08:03:30Z

          .version(ConfigConstants.VERSION_1_1_0)
          .stringConf()
-          .create();
+          .createWithDefault("org.apache.gravitino.iceberg.common.cache.LocalTableMetadataCache");


Can we use LocalTableMetadataCache.class.getName() here to avoid possible class rename in the future?

Got resolved

…-defaults' into issue-11133-table-metadata-cache-defaults

github-actions · 2026-05-20T09:17:50Z

Code Coverage Report

Overall Project	66.17% `+0.01%`	🟢
Files changed	68.09%	🟢

Module	Coverage
aliyun	1.72%	🔴
api	46.82%	🟢
authorization-common	85.96%	🟢
aws	1.08%	🔴
azure	2.47%	🔴
catalog-common	10.2%	🔴
catalog-fileset	80.02%	🟢
catalog-glue	64.03%	🟢
catalog-hive	79.59%	🟢
catalog-jdbc-clickhouse	80.02%	🟢
catalog-jdbc-common	44.46%	🟢
catalog-jdbc-doris	80.28%	🟢
catalog-jdbc-hologres	54.03%	🟢
catalog-jdbc-mysql	79.23%	🟢
catalog-jdbc-oceanbase	78.38%	🟢
catalog-jdbc-postgresql	82.17%	🟢
catalog-jdbc-starrocks	78.27%	🟢
catalog-kafka	77.01%	🟢
catalog-lakehouse-generic	44.89%	🟢
catalog-lakehouse-hudi	79.1%	🟢
catalog-lakehouse-iceberg	85.71% `+0.17%`	🟢
catalog-lakehouse-paimon	79.29%	🟢
catalog-model	77.72%	🟢
cli	44.51%	🟢
client-java	77.94%	🟢
common	49.99%	🟢
core	82.24%	🟢
filesystem-hadoop3	76.97%	🟢
flink	0.0%	🔴
flink-common	43.17%	🟢
flink-runtime	0.0%	🔴
gcp	14.12%	🔴
hadoop-common	10.39%	🔴
hive-metastore-common	53.26%	🟢
iceberg-common	55.28% `+6.56%`	🟢
iceberg-rest-server	69.81%	🟢
idp-basic	94.17%	🟢
integration-test-common	0.0%	🔴
jobs	66.17%	🟢
lance-common	20.9%	🔴
lance-rest-server	62.78%	🟢
lineage	53.02%	🟢
optimizer	82.95%	🟢
optimizer-api	21.95%	🔴
server	85.09%	🟢
server-common	72.8%	🟢
spark	32.79%	🔴
spark-common	39.09%	🔴
trino-connector	34.82%	🔴

Files

Module	File	Coverage
catalog-lakehouse-iceberg	IcebergCatalogPropertiesMetadata.java	91.67%	🟢
iceberg-common	IcebergConfig.java	100.0%	🟢
	IcebergCatalogWrapper.java	31.25%	🔴

RESTCatalog does not support SupportsMetadataLocation; set an empty table-metadata-cache-impl in integration tests that use REST backend. Co-authored-by: Cursor <cursoragent@cursor.com>

Co-authored-by: Cursor <cursoragent@cursor.com>

maintenance/jobs has no dependency on catalog-common; use the property key string instead of IcebergConstants in env IT setup. Co-authored-by: Cursor <cursoragent@cursor.com>

Use IcebergConstants in TestIcebergUpdateStatsJobWithSpark instead of a hard-coded property key so the jobs module compiles like optimizer. Co-authored-by: Cursor <cursoragent@cursor.com>

roryqi · 2026-05-21T03:37:28Z

How to handle the consistency issue? For example,
We have two IRC servers,
Two IRC servers cached Table A.
We modify the table A in the server 1.
Server 2 stills used the legacy data, doesn't it?

lasdf1234 · 2026-05-21T06:21:20Z

How to handle the consistency issue? For example,
We have two IRC servers,
Two IRC servers cached Table A.
We modify the table A in the server 1.
Server 2 stills used the legacy data, doesn't it?

The BaseTableMetadataCache.getTableMetadata() method first checks the latest metadata location of the catalog before returning the cache. If the result is inconsistent with the metadataFileLocation() in the cache, it will invalidate the cache and return an empty value (triggering a reload):

roryqi · 2026-05-21T06:45:24Z

Could u check the logic?

 private TableMetadataCache loadTableMetadataCache(IcebergConfig config, Catalog catalog) {
    String impl = config.get(IcebergConfig.TABLE_METADATA_CACHE_IMPL);
    if (StringUtils.isBlank(impl)) {
      return TableMetadataCache.DUMMY;
    }

    Preconditions.checkArgument(
        catalog instanceof SupportsMetadataLocation,
        "You shouldn't enable Iceberg metadata cache for the catalog %s,"
            + " because the catalog impl does not support get metadata location.",
        catalog.name());

Some catalog will throws exception if you changed the default value.
Only part of catalogs support metadata location.

lasdf1234 · 2026-05-21T06:54:14Z

Could u check the logic?

 private TableMetadataCache loadTableMetadataCache(IcebergConfig config, Catalog catalog) {
    String impl = config.get(IcebergConfig.TABLE_METADATA_CACHE_IMPL);
    if (StringUtils.isBlank(impl)) {
      return TableMetadataCache.DUMMY;
    }

    Preconditions.checkArgument(
        catalog instanceof SupportsMetadataLocation,
        "You shouldn't enable Iceberg metadata cache for the catalog %s,"
            + " because the catalog impl does not support get metadata location.",
        catalog.name());

Some catalog will throws exception if you changed the default value. Only part of catalogs support metadata location.

Yes,

Could u check the logic?

 private TableMetadataCache loadTableMetadataCache(IcebergConfig config, Catalog catalog) {
    String impl = config.get(IcebergConfig.TABLE_METADATA_CACHE_IMPL);
    if (StringUtils.isBlank(impl)) {
      return TableMetadataCache.DUMMY;
    }

    Preconditions.checkArgument(
        catalog instanceof SupportsMetadataLocation,
        "You shouldn't enable Iceberg metadata cache for the catalog %s,"
            + " because the catalog impl does not support get metadata location.",
        catalog.name());

Some catalog will throws exception if you changed the default value. Only part of catalogs support metadata location.

When implementing this requirement, I have already checked this part of the logic, and only rest(hive),rest(jdbc),rest(memory) support cache.Should I explain this issue in the irc doc?

roryqi · 2026-05-21T08:22:03Z

Could u elaborate more about influence? Does it only influence the custom catalog? Custom catalog can't be used. This would be a breaking change. Could u avoid a breaking change?

lasdf1234 · 2026-05-21T11:28:15Z

Could u elaborate more about influence? Does it only influence the custom catalog? Custom catalog can't be used. This would be a breaking change. Could u avoid a breaking change?

IRC has five backends: memory/jdbc/hive/rest/custom. Memory, jdbc and hive support table cache, while rest does not explicitly support it. Custom depends on the specific implementation.

I have come up two solutions as follows:

1.Enable table cache by default.
The document should specify which backends support table cache, and also indicate that if an unsupported backend is used, this configuration should be manually adjusted.

2.Disabled table cache by default.
The configuration of the table cache remains the same as before, but it is necessary to add information in the documentation which backend supports table cache and which one doesn't.

Which is better?
@roryqi

roryqi · 2026-05-21T11:36:06Z

Could u elaborate more about influence? Does it only influence the custom catalog? Custom catalog can't be used. This would be a breaking change. Could u avoid a breaking change?

IRC has five backends: memory/jdbc/hive/rest/custom. Memory, jdbc and hive support table cache, while rest does not explicitly support it. Custom depends on the specific implementation.

I have come up two solutions as follows:

1.Enable table cache by default. The document should specify which backends support table cache, and also indicate that if an unsupported backend is used, this configuration should be manually adjusted.

2.Disabled table cache by default. The configuration of the table cache remains the same as before, but it is necessary to add information in the documentation which backend supports table cache and which one doesn't.

Which is better? @roryqi

REST catalog is disable cached. Maybe we can disable the cache for the custom catalog, too. cc @jerryshao WDYT?

jerryshao · 2026-05-22T06:04:05Z

Can you update the PR as we discussed in the morning? @lasdf1234

Clarify that table-metadata-cache-impl should be set to "" for REST or custom catalogs without SupportsMetadataLocation, and improve the runtime error message to point users at the config key and restart. Co-authored-by: Cursor <cursoragent@cursor.com>

lasdf1234 · 2026-05-22T07:34:36Z

Can you update the PR as we discussed in the morning? @lasdf1234

Got.The instructions in the configuration have been fully updated, and the log that prompts users to modify the configuration has also been added.

lasdf1234 · 2026-05-22T07:35:19Z

@roryqi The logs and doc have been added. If you have time, please review code for me.THX.

roryqi · 2026-05-22T07:36:51Z

@roryqi The logs and doc have been added. If you have time, please review code for me.THX.

Could u address the conflicts first?

Resolve FlinkIcebergRestCatalogIT import conflict by keeping both IcebergConstants for REST metadata cache config and main's Flink SQL test imports. Co-authored-by: Cursor <cursoragent@cursor.com>

lasdf1234 · 2026-05-22T07:53:23Z

@roryqi The logs and doc have been added. If you have time, please review code for me.THX.

Could u address the conflicts first?

Got resolved

Add IcebergCacheIT to verify REST and custom catalogs disable or enable the default metadata cache based on SupportsMetadataLocation, and fall back to DUMMY with a warning when the catalog does not support it. Co-authored-by: Cursor <cursoragent@cursor.com>

Explain that the cache is disabled because the catalog implementation does not support get metadata location. Co-authored-by: Cursor <cursoragent@cursor.com>

Co-authored-by: Cursor <cursoragent@cursor.com>

lasdf1234 · 2026-05-22T09:28:26Z

@roryqi The code has been fixed. If you have time, please take a look at it for review.

Move custom catalog test doubles into IcebergCacheIT as inner classes. Drop maintenance/jobs test dependency on catalog-common by using the table-metadata-cache-impl property key directly in tests. Co-authored-by: Cursor <cursoragent@cursor.com>

IcebergCatalogWrapper already falls back to DUMMY when the catalog does not support metadata location. Cache behavior is covered by IcebergCacheIT. Co-authored-by: Cursor <cursoragent@cursor.com>

roryqi

LGTM.

Copilot AI review requested due to automatic review settings May 19, 2026 17:25

Copilot AI reviewed May 19, 2026

View reviewed changes

[apache#11133] Enable table metadata cache by default

89d3223

Default table-metadata-cache-impl to LocalTableMetadataCache and raise table-metadata-cache-capacity to 1000 in IcebergConfig and docs. Co-authored-by: Cursor <cursoragent@cursor.com>

lasdf1234 force-pushed the issue-11133-table-metadata-cache-defaults branch from 172134f to 89d3223 Compare May 19, 2026 17:31

[apache#11133] Drop default table-metadata-cache-impl

5e6b0b0

Keep only the table-metadata-cache-capacity default change (1000). Co-authored-by: Cursor <cursoragent@cursor.com>

lasdf1234 changed the title ~~[#11133] feat(iceberg): Enable table metadata cache by default~~ [#11133] feat(iceberg): Raise table metadata cache default capacity to 1000 May 19, 2026

[apache#11133] Enable table metadata cache by default

a8b9c47

Default table-metadata-cache-impl to LocalTableMetadataCache and raise table-metadata-cache-capacity to 1000 per issue apache#11133. Co-authored-by: Cursor <cursoragent@cursor.com>

lasdf1234 changed the title ~~[#11133] feat(iceberg): Raise table metadata cache default capacity to 1000~~ [#11133] feat(iceberg): Enable table metadata cache by default May 20, 2026

lasdf1234 and others added 2 commits May 20, 2026 15:36

[apache#11133] Add cache default tests and align catalog property met…

5046b39

…adata Assert table metadata cache defaults in TestIcebergConfig and update IcebergCatalogPropertiesMetadata to match IcebergConfig. Co-authored-by: Cursor <cursoragent@cursor.com>

[apache#11133] Add TestIcebergCatalogPropertiesMetadata for cache def…

fdbaa07

…aults Assert catalog property metadata defaults align with IcebergConfig. Co-authored-by: Cursor <cursoragent@cursor.com>

lasdf1234 changed the title ~~[#11133] feat(iceberg): Enable table metadata cache by default~~ [#11133] feat(iceberg): Enable table metadata cache by default and increase default capacity May 20, 2026

yuqi1129 reviewed May 20, 2026

View reviewed changes

lasdf1234 added 2 commits May 20, 2026 17:16

optimized code

305fe7b

Merge remote-tracking branch 'origin/issue-11133-table-metadata-cache…

73b14a1

…-defaults' into issue-11133-table-metadata-cache-defaults

lasdf1234 and others added 5 commits May 21, 2026 07:22

[apache#11133] Disable table metadata cache in REST catalog ITs

03d0850

RESTCatalog does not support SupportsMetadataLocation; set an empty table-metadata-cache-impl in integration tests that use REST backend. Co-authored-by: Cursor <cursoragent@cursor.com>

[apache#11133] Fix import order in FlinkIcebergRestCatalogIT

4a0ffd7

Co-authored-by: Cursor <cursoragent@cursor.com>

[apache#11133] Fix maintenance jobs test compile for REST cache property

0fb1b5b

maintenance/jobs has no dependency on catalog-common; use the property key string instead of IcebergConstants in env IT setup. Co-authored-by: Cursor <cursoragent@cursor.com>

[apache#11133] Add catalog-common test dependency for maintenance jobs

8ea3741

Use IcebergConstants in TestIcebergUpdateStatsJobWithSpark instead of a hard-coded property key so the jobs module compiles like optimizer. Co-authored-by: Cursor <cursoragent@cursor.com>

Merge branch 'main' into issue-11133-table-metadata-cache-defaults

2f34013

Merge branch 'main' into issue-11133-table-metadata-cache-defaults

73b2249

Resolve FlinkIcebergRestCatalogIT import conflict by keeping both IcebergConstants for REST metadata cache config and main's Flink SQL test imports. Co-authored-by: Cursor <cursoragent@cursor.com>

lasdf1234 and others added 3 commits May 22, 2026 17:19

[apache#11133] Clarify metadata cache disable warn message

9b826ae

Explain that the cache is disabled because the catalog implementation does not support get metadata location. Co-authored-by: Cursor <cursoragent@cursor.com>

[apache#11133] Fix punctuation in metadata cache disable warn message

d180810

Co-authored-by: Cursor <cursoragent@cursor.com>

roryqi reviewed May 22, 2026

View reviewed changes

Comment thread maintenance/jobs/build.gradle.kts Outdated

roryqi reviewed May 22, 2026

View reviewed changes

Comment thread ...rc/test/java/org/apache/gravitino/iceberg/integration/test/CustomSupportLocationCatalog.java Outdated

roryqi reviewed May 22, 2026

View reviewed changes

Comment thread ...t/java/org/apache/gravitino/maintenance/jobs/iceberg/TestIcebergUpdateStatsJobWithSpark.java Outdated

lasdf1234 and others added 2 commits May 22, 2026 18:13

[apache#11133] Drop explicit REST cache disable from unrelated ITs

bae9c2f

IcebergCatalogWrapper already falls back to DUMMY when the catalog does not support metadata location. Cache behavior is covered by IcebergCacheIT. Co-authored-by: Cursor <cursoragent@cursor.com>

roryqi approved these changes May 22, 2026

View reviewed changes

Merge branch 'main' into issue-11133-table-metadata-cache-defaults

f389db3

Conversation

lasdf1234 commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lasdf1234 commented May 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lasdf1234 May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Report

Uh oh!

roryqi commented May 21, 2026

Uh oh!

lasdf1234 commented May 21, 2026

Uh oh!

roryqi commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lasdf1234 commented May 21, 2026

Uh oh!

roryqi commented May 21, 2026

Uh oh!

lasdf1234 commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roryqi commented May 21, 2026

Uh oh!

jerryshao commented May 22, 2026

Uh oh!

lasdf1234 commented May 22, 2026

Uh oh!

lasdf1234 commented May 22, 2026

Uh oh!

roryqi commented May 22, 2026

Uh oh!

lasdf1234 commented May 22, 2026

Uh oh!

lasdf1234 commented May 22, 2026

Uh oh!

Uh oh!

lasdf1234 commented May 19, 2026 •

edited

Loading

lasdf1234 May 20, 2026 •

edited

Loading

github-actions Bot commented May 20, 2026 •

edited

Loading

roryqi commented May 21, 2026 •

edited

Loading

lasdf1234 commented May 21, 2026 •

edited

Loading