Optimize Iceberg MV freshness checks based on grace period #27608

piotrrzysko · 2025-12-10T15:41:21Z

Description

This PR introduces a mechanism to guide connectors on whether they can skip expensive freshness checks when a
materialized view's grace period allows it.

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text:

## Section
* Optimize Iceberg materialized view freshness checks based on grace period ({issue}`27608`)

This commit introduces MaterializedViewFreshnessCheckPolicy to guide connectors on whether they can skip expensive freshness checks when a materialized view's grace period allows it. The policy has two modes: - Exact: Used for operations like REFRESH MATERIALIZED VIEW that require precise freshness determination. Connectors must perform all freshness checks. - ConsiderGracePeriod: Used for SELECT queries where staleness within the grace period is acceptable. Connectors may skip expensive operations (e.g., metastore calls to check base table snapshots) if the MV is fresh enough within the grace period bounds. A new freshness status FRESH_WITHIN_GRACE_PERIOD distinguishes when this optimization was applied versus when base tables were actually checked.

findepi

Clarify Iceberg MV refresh time resolution

findepi · 2025-12-11T13:32:26Z

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java

-        Optional<Instant> refreshTime = currentSnapshot.map(snapshot -> snapshot.summary().get(TRINO_QUERY_START_TIME))
-                .map(Instant::parse)
-                .or(() -> currentSnapshot.map(snapshot -> Instant.ofEpochMilli(snapshot.timestampMillis())));
+        // refreshStartTime is captured at the beginning of refresh


isn't it obvious from the property name - TRINO_QUERY_START_TIME?

It wasn’t obvious to me at first what query we were talking about. What do you think about renaming TRINO_QUERY_START_TIME to LAST_REFRESH_START_TIME?

Renaming the constant doesn't help and is actually more confusing. The constant name corresponds to it's value (the property name). It's what it is.

findepi · 2025-12-11T13:33:35Z

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java

+        // For MVs refreshed before TRINO_QUERY_START_TIME was introduced, fall back to snapshot timestamp (captured at end of refresh)
+        Optional<Instant> refreshTime = refreshStartTime.or(() -> currentSnapshot.map(snapshot -> Instant.ofEpochMilli(snapshot.timestampMillis())));


Adding a comment doesn't require adding a variable

Optional<Instant> refreshTime = currentSnapshot.map(snapshot -> snapshot.summary().get(TRINO_QUERY_START_TIME)) .map(Instant::parse) // Fallback to snapshot commit time (end of refresh) for MVs defined before TRINO_QUERY_START_TIME was introduced .or(() -> currentSnapshot.map(snapshot -> Instant.ofEpochMilli(snapshot.timestampMillis())));

findepi · 2025-12-11T16:35:43Z

core/trino-spi/src/main/java/io/trino/spi/connector/MaterializedViewFreshnessCheckPolicy.java

+/**
+ * Policy for checking materialized view freshness, guiding a connector on whether it can
+ * optimize freshness checks.
+ */
+public sealed interface MaterializedViewFreshnessCheckPolicy
+        permits MaterializedViewFreshnessCheckPolicy.Exact, MaterializedViewFreshnessCheckPolicy.ConsiderGracePeriod
+{
+    /**
+     * Policy requiring exact freshness check.
+     */
+    record Exact()
+            implements MaterializedViewFreshnessCheckPolicy {}
+
+    /**
+     * Policy that allows optimizing freshness checks by considering the materialized view's
+     * grace period. A connector may skip expensive operations (e.g., metastore calls to check
+     * base table snapshots) if it can determine the MV is fresh enough based on the
+     * referenceTime and the grace period configured in the materialized view definition.
+     *
+     * @param referenceTime The point in time against which freshness is being evaluated
+     *                      (typically the query start time)
+     */
+    record ConsiderGracePeriod(Instant referenceTime)
+            implements MaterializedViewFreshnessCheckPolicy {}
+}


That's beautiful, but mouthful and makes following the logic harder.
I wanted this to be single boolean param like boolean considerGracePeriod...

It seems this SPI design was forced by the fact we don't want the connector to use session.getStart(), and we don't want the connector to use that value because... it's mocked in tests.
So, SPI shape (and complexity) is a by product of existing tests. Not great.
Let's find a way to avoid that.

For example, we could pass SessionTimeProvider to io.trino.Session#toConnectorSession so that FullConnectorSession can have its reported start time overriden.
Or, perhaps we could fix this to work (in tests only)

trino/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java

Line 1520 in 3d347c5

// This gets ignored: .setStart(...)

There were two reasons:

More important: I wanted to avoid making the connector aware of what the reference time is. Even in the analyzer we have a TODO (which we probably should remove) that suggests we aren’t sure whether it should be the current time or the session start time.

The second reason you almost figured out: I initially wanted to use the current time in the connector as the safest, most pessimistic option. But the already-mocked session start time stopped me…

findepi · 2025-12-11T16:39:46Z

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java

+            Optional<java.time.Duration> gracePeriod = materializedViewDefinition.get().getGracePeriod();
+            if (gracePeriod.isEmpty()) {
+                // infinite grace period
+                return new MaterializedViewFreshness(FRESH_WITHIN_GRACE_PERIOD, Optional.empty());


This case doesn't have to be guarded with refreshStartTime.isPresent() check. It's easy to address, by melding the ifs into one:

if (policy instanceof ConsiderGracePeriod(Instant referenceTime) && withinGracePeriod(gracePeriod, refreshTime, referenceTime)) { // bla bla bla return new MaterializedViewFreshness(FRESH_WITHIN_GRACE_PERIOD, Optional.empty()); } boolean withinGracePeriod(gracePeriod, ...) { if gracePeriod.isEmpty() { // infinite grace period return true; } ... }

findepi · 2025-12-11T16:40:45Z

...rino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergMetastoreAccessOperations.java

        assertMetastoreInvocations("SELECT * FROM test_select_fresh_mview_view",
                ImmutableMultiset.<MetastoreMethod>builder()
-                        .addCopies(GET_TABLE, 2)
+                        .addCopies(GET_TABLE, 1)


❤️

maybe it would be wise to have an MV defined on couple source tables, so that impact is more visible in the test

(i should have thought about this in 37a7dca ...)

cla-bot bot added the cla-signed label Dec 10, 2025

github-actions bot added iceberg Iceberg connector lakehouse labels Dec 10, 2025

piotrrzysko force-pushed the iceberg_consider_grace_period branch from cd631a4 to 262a026 Compare December 11, 2025 10:09

piotrrzysko added 2 commits December 11, 2025 11:22

Clarify Iceberg MV refresh time resolution

d168a42

piotrrzysko force-pushed the iceberg_consider_grace_period branch from 262a026 to 891309e Compare December 11, 2025 10:23

piotrrzysko marked this pull request as ready for review December 11, 2025 11:09

piotrrzysko requested a review from findepi December 11, 2025 11:09

findepi reviewed Dec 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Optimize Iceberg MV freshness checks based on grace period #27608

Optimize Iceberg MV freshness checks based on grace period #27608

piotrrzysko commented Dec 10, 2025 •

edited

Loading

Uh oh!

findepi left a comment

Uh oh!

findepi Dec 11, 2025

Uh oh!

piotrrzysko Dec 12, 2025

Uh oh!

findepi Dec 12, 2025

Uh oh!

findepi Dec 11, 2025

Uh oh!

findepi Dec 11, 2025 •

edited

Loading

Uh oh!

piotrrzysko Dec 11, 2025

Uh oh!

findepi Dec 11, 2025

Uh oh!

findepi Dec 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

		// For MVs refreshed before TRINO_QUERY_START_TIME was introduced, fall back to snapshot timestamp (captured at end of refresh)
		Optional<Instant> refreshTime = refreshStartTime.or(() -> currentSnapshot.map(snapshot -> Instant.ofEpochMilli(snapshot.timestampMillis())));

Optimize Iceberg MV freshness checks based on grace period #27608

Are you sure you want to change the base?

Optimize Iceberg MV freshness checks based on grace period #27608

Conversation

piotrrzysko commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Additional context and related issues

Release notes

Uh oh!

findepi left a comment

Choose a reason for hiding this comment

Uh oh!

findepi Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

piotrrzysko Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

findepi Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

findepi Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

findepi Dec 11, 2025 • 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!

piotrrzysko Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

findepi Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

findepi Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

piotrrzysko commented Dec 10, 2025 •

edited

Loading

findepi Dec 11, 2025 •

edited

Loading