deduplicate storage requests when loading views. by evindj · Pull Request #3488 · apache/polaris

evindj · 2026-01-20T21:09:23Z

The PR addresses issue #3061 where loadview makes multiple requests for the same file.
This PR has two main changes to avoid making too many storage requests when loading tables and views.
The first change is aimed at reusing fileIO in case refreshFromMetadataLocation is retried. this is done for both tables and views.

The second change is related to loadView only it overrides loadView in BaseMetastoreViewCatalog to reuse viewOps instead of creating them twice.

Checklist

🛡️ Don't disclose security issues! (contact security@apache.org)
🔗 Clearly explained why the changes are needed, or linked related issues: Fixes #
🧪 Added/updated tests with good coverage, or manually tested (and explained how)
💡 Added comments for complex logic
🧾 Updated CHANGELOG.md (if needed)
📚 Updated documentation in site/content/in-dev/unreleased (if needed)

dimas-b

Thanks for your contribution, @evindj ! Nice find about double view ops! Some minor comments below.

dimas-b · 2026-01-22T01:43:59Z

runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java

                new AttributeMap()
                    .put(EventAttributes.CATALOG_NAME, catalogName)
                    .put(EventAttributes.TABLE_IDENTIFIER, tableIdentifier)));
+        // Create FileIO once outside the retry lambda to avoid redundant storage requests


How does this help with reducing storage requests? Creating a FileIO should not hit storage, AFAIK 🤔

This is just about the comment. Code change LGTM.

Prior to the change, if refreshFromMetadataLocation was retried, it will created another FileIO, I can update the comment.

dimas-b · 2026-01-22T01:45:42Z

runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java

  }

+  /**
+   * Override to fix a bug in {@link BaseMetastoreViewCatalog#loadView} where {@link #newViewOps} is


tangential: Could you also double check for something like this in tables?

@evindj : any insight here? it's not required for current PR, but since you're involved in this issue already, I thought it might be easy for you to quickly double check that 😉

yeah, it is my understanding that this would not happen for LoadTable, because loadtable directly uses the operations from the table to construct the response. See https://github.com/apache/polaris/blob/main/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/CatalogHandlerUtils.java#L387

dimas-b · 2026-01-22T01:47:15Z

.../java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogLoadViewOptimizationTest.java

+    ViewMetadata view = response.metadata();
+
+    // Assert: newViewOps() should be reused (called exactly once)
+    verify(spyCatalog, times(1)).newViewOps(VIEW_ID);


Running a @QuarkusTest (which builds a full server) to do some mock checks looks like an overkill to me.

Could we leverage MeasuredFileIO instead and move this test to an existing Quarkus test (to reduce the number of test servers)?

Also, if testing this particular bug turns out to be cumbersome, I think manual verification is sufficient.

Existing CI should be sufficient to ensure the changed code works well.

Yeah it would be a bit challenging to test without turning up an environment because, the duplicate request happens while loading the view and while returning the response. I have verified manually so I am ok removing this expensive test.

dimas-b · 2026-01-22T18:44:49Z

runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java

                new AttributeMap()
                    .put(EventAttributes.CATALOG_NAME, catalogName)
                    .put(EventAttributes.VIEW_IDENTIFIER, identifier)));
+        // Create FileIO once outside the retry lambda to avoid redundant storage requests


Same concern here - not re-creating FileIO is good, but I do not think it relates to duplicate storage requests...

To be completely clear wrt #3061, WDYT about making the FileIO optimization change in a separate PR?

Other than that this PR LGTM 👍 :)

I can move the FileIO optimization to a separate PR. let me remove then here.

dimas-b · 2026-01-22T21:40:41Z

runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java

              // then we should use the actual current table properties for IO refresh here
              // instead of the general tableDefaultProperties.
+              String latestLocationDir =
+                  latestLocation.substring(0, latestLocation.lastIndexOf('/'));


one last thing, please - would you mind removing these changes now that there are not necessary for the purpose of this PR?

MonkeyCanCode · 2026-01-23T01:35:05Z

runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java

                      new HashMap<>(tableDefaultProperties),
                      Set.of(PolarisStorageActions.READ, PolarisStorageActions.LIST));
-
+                      


Please run .//gradlew spotlessApply or make spotless-apply to fix the lint then CI will move forward.

* dedup metadata fetch by reusing ops

* (doc) Outdated changelog (apache#3503) * deduplicate storage requests when loading views. (apache#3488) * dedup metadata fetch by reusing ops * Update Gradle to 9.3.0 (apache#3514) * Site: add tool to mark versioned-docs as "do not index" (apache#3485) Adds a shell script to add the front-matter tags `robots: noindex` (HTML META tag) and `exclude_search: true` (local site search) for versioned docs for a specific version. The rudimentary script handles only markdown files, not asciidoc files, because there are currently only .md files in the versioned-docs. * feat(metrics): Evolve PolarisMetricsReporter interface with timestamp parameter and comprehensive documentation (apache#3468) Enhance the `PolarisMetricsReporter` SPI interface by adding a timestamp parameter to the `reportMetric()` method, enabling accurate time-series metrics reporting to external systems. * Make python version configuration in Makefile (apache#3510) * Update max supported python version (apache#3509) * (doc) Add doc for couple new feature flags introduced recently (apache#3511) * Add doc for couple new feature flags * Add doc for couple new feature flags * (nit) Site: Fading anchor (apache#3522) * Fading anchor * Fading anchor * Update doc to use /Users/richardliu rather than quoted tilde (apache#3472) * Remove KMS policies when KMS is not configured and improved default KMS permission for RO/RW (apache#3493) * Update triggers in "Hugo Site" workflow (apache#3518) The `site.yml` workflow is currently triggered in the following scenarios: 1. A push to the `main` branch, using the state of the site and the workflow as on `main`. 2. A push to a `release/*`branch, using the state of the site and the workflow as on that release branch. Notice that workflows get the repo state (from the checkout actions) and the workflow state as its on that particular branch. Put in other words: if we'd have a change to some old `release/1.0.x` branch, the web site would be updated as it is defined on that `release/1.0.x` branch, which is wrong and not the intended behavior. This change updates the workflow triggers to only run for pushes to the `main` branch, plus PRs against the `main` branch and when called from another workflow. This is part of apache#3516 * Let `site/bin/checkout-releases.sh` pull the latest state (apache#3517) The script `site/bin/checkout-releases.sh` is a convenience to get the `versioned-docs` branch locally. It is missing a `git pull` to get the latest state of that branch though, which can be confusing. * Last merged commit 1bf72bc --------- Co-authored-by: Yong Zheng <yongzheng0809@gmail.com> Co-authored-by: Innocent Djiofack <djiofack007@gmail.com> Co-authored-by: Anand K Sankaran <lists@anands.net> Co-authored-by: Richard Liu <35082658+RichardLiu2001@users.noreply.github.com>

evindj added 2 commits January 17, 2026 22:24

create file io out side of lambdas

7d097a1

dedup metadata fetch by reusing ops

b3648e2

github-project-automation bot added this to Basic Kanban Board Jan 20, 2026

github-project-automation bot moved this to PRs In Progress in Basic Kanban Board Jan 20, 2026

evindj marked this pull request as draft January 20, 2026 21:09

evindj added 2 commits January 20, 2026 20:04

Added unit tests

8aa8074

spotless apply

a3bb73a

evindj marked this pull request as ready for review January 21, 2026 20:05

dimas-b reviewed Jan 22, 2026

View reviewed changes

review comments

68e16a0

dimas-b reviewed Jan 22, 2026

View reviewed changes

keep inefficient retry for a different PR

eeaae08

dimas-b reviewed Jan 22, 2026

View reviewed changes

evindj added 2 commits January 22, 2026 14:43

move comment location

00a5d91

cleanup

ac443da

dimas-b previously approved these changes Jan 23, 2026

View reviewed changes

github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Jan 23, 2026

MonkeyCanCode reviewed Jan 23, 2026

View reviewed changes

cleanup

3b7ceda

evindj dismissed dimas-b’s stale review via 3b7ceda January 23, 2026 03:41

dimas-b approved these changes Jan 23, 2026

View reviewed changes

dimas-b enabled auto-merge (squash) January 23, 2026 15:29

dimas-b merged commit 2f4931c into apache:main Jan 23, 2026
15 checks passed

github-project-automation bot moved this from Ready to merge to Done in Basic Kanban Board Jan 23, 2026

dimas-b mentioned this pull request Jan 23, 2026

loadView makes multiple storage requests for the same file #3061

Closed

evindj added a commit to evindj/polaris that referenced this pull request Jan 26, 2026

deduplicate storage requests when loading views. (apache#3488)

102ad84

* dedup metadata fetch by reusing ops

		new HashMap<>(tableDefaultProperties),
		Set.of(PolarisStorageActions.READ, PolarisStorageActions.LIST));

Comments

Conversation

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

Uh oh!

Checklist

Uh oh!

dimas-b left a comment

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!

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

evindj commented Jan 20, 2026 •

edited

Loading