[carloea2]

What changes were proposed in this PR?

Disable fsspec's global S3 instance cache when creating a REST catalog. PyIceberg already caches each filesystem per FileIO thread. Sharing another cached filesystem lets a second table temporarily remove the first table's request signer, causing 403 AccessDenied during materialization reads.

Before: signer replacement can interrupt an existing read.
After: FileIO instances have separate signers and retain their own filesystem caches.

This is a workaround for apache/iceberg-python#3896. It disables the global S3 instance cache for the Python process, including other fsspec S3 users. Local-filesystem caching is unchanged. It does not change credentials or retry failed reads.

Any related issues, documentation, discussions?

Closes #9128

Related to the failing loop integration check on #8957. Its timestamp change stays separate.

How was this PR tested?

Fresh detached worktree from upstream main at 98c63374b7, Python 3.12 on Windows.

The new regression first failed in two cache configurations. After the fix, all 71 focused storage tests pass, with the separately marked REST integration test excluded. The regression uses real FileIO and S3 filesystem instances, verifies distinct signers between FileIO instances, and verifies that per-thread and local-filesystem caches remain intact.

bash bin/python-proto-gen.sh
cd amber
python -m pytest src/test/python/core/storage/iceberg -m "not integration" -q --tb=short -p no:cacheprovider
python -m ruff check src/main/python src/test/python
python -m ruff format --check src/main/python src/test/python

Live verification used an isolated RustFS rc.6 and Lakekeeper 0.11.0 stack. A real Parquet read succeeds before signer replacement, fails during a controlled replacement gap, and succeeds after registration resumes. With the fix, 40 concurrent real S3 reads pass. Both Ruff checks pass for all 217 Python files.

The full engine loop suite also passes with this worktree's freshly built JVM classes and Python workers against that REST-backed stack: 8 passed, 0 failed, 0 canceled. This checks materialized row counts for single and nested loops, JVM operators, loop-variable references, reconverging branches, and state-emitting UDFs. No test assertions or timeouts were changed.

From the repository root, with the private stack's storage environment configured:

sbt -batch "WorkflowExecutionService/testOnly *LoopIntegrationSpec -- -oDF"
sbt -batch scalafmtCheckAll

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Codex

[github-actions]

Backport auto-label report

This fix: PR was checked against each actively-supported release branch. A release/* label nominates a backport target; the branch's release manager approving this PR is what sends the fix there. The required Backport Approvals check stays red until every label below is approved, so each manager either approves or removes their own label — which is why the labels left on a merged PR are exactly the branches it reached.

Release branch	Analysis
✅ release/v1.3	Already labeled — this fix is queued to backport here. @mengw15 decides: approving sends the fix here, removing this label declines it. The merge waits on one or the other.
✅ release/v1.2	Already labeled — this fix is queued to backport here. @xuang7 decides: approving sends the fix here, removing this label declines it. The merge waits on one or the other.

Auto-label run.

[github-actions]

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

• Contributors with relevant context: @aglinxinyuan
  You can notify them by mentioning @aglinxinyuan in a comment.

[carloea2]

/request-review @aglinxinyuan @Yicong-Huang

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.39%. Comparing base (98b875f) to head (7b2b04a).

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #9129      +/-   ##
============================================
+ Coverage     91.81%   92.39%   +0.58%     
+ Complexity     5066     5016      -50     
============================================
  Files          1270     1261       -9     
  Lines         55181    54590     -591     
  Branches       6984     6867     -117     
============================================
- Hits          50662    50441     -221     
+ Misses         2800     2491     -309     
+ Partials       1719     1658      -61     

Flag	Coverage Δ		*Carryforward flag
access-control-service	77.38% <ø> (-0.07%)	⬇️	Carriedforward from 2564060
agent-service	99.16% <ø> (ø)		Carriedforward from 2564060
amber	87.33% <ø> (+1.37%)	⬆️	Carriedforward from 2564060
computing-unit-managing-service	60.48% <ø> (-1.36%)	⬇️	Carriedforward from 2564060
config-service	87.37% <ø> (-0.13%)	⬇️	Carriedforward from 2564060
file-service	82.00% <ø> (+0.08%)	⬆️	Carriedforward from 2564060
frontend	96.55% <ø> (+0.05%)	⬆️	Carriedforward from 2564060
notebook-migration-service	83.73% <ø> (-0.04%)	⬇️	Carriedforward from 2564060
pyamber	98.48% <100.00%> (-0.06%)	⬇️
workflow-compiling-service	72.43% <ø> (-0.18%)	⬇️	Carriedforward from 2564060

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

⚠️ Benchmark changes need a look

🟢 0 better · 🔴 9 worse · ⚪ 6 noise (<±5%) · 0 without baseline

Compared against main 98b875f benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

	config	throughput	MB/s	latency	max Δ latest / 7d
🔴	bs=10 sw=10 sl=64	372	0.227	25,484/39,939/39,939 us	🔴 +16.2% / 🔴 +254.9%
🔴	bs=100 sw=10 sl=64	820	0.5	117,439/164,934/164,934 us	🔴 +7.1% / 🔴 +120.3%
🔴	bs=1000 sw=10 sl=64	951	0.581	1,041,702/1,186,919/1,186,919 us	🔴 +6.7% / 🔴 +68.0%

Baseline details

Latest main 98b875f from same runner

config	metric	PR	latest main	7d avg	Δ latest	Δ 7d
bs=10 sw=10 sl=64	throughput	372 tuples/sec	419 tuples/sec	1,119 tuples/sec	-11.2%	-66.8%
bs=10 sw=10 sl=64	MB/s	0.227 MB/s	0.256 MB/s	0.683 MB/s	-11.3%	-66.8%
bs=10 sw=10 sl=64	p50	25,484 us	21,936 us	8,992 us	+16.2%	+183.4%
bs=10 sw=10 sl=64	p95	39,939 us	37,107 us	11,255 us	+7.6%	+254.9%
bs=10 sw=10 sl=64	p99	39,939 us	37,107 us	13,808 us	+7.6%	+189.3%
bs=100 sw=10 sl=64	throughput	820 tuples/sec	813 tuples/sec	1,518 tuples/sec	+0.9%	-46.0%
bs=100 sw=10 sl=64	MB/s	0.5 MB/s	0.496 MB/s	0.927 MB/s	+0.8%	-46.0%
bs=100 sw=10 sl=64	p50	117,439 us	118,970 us	68,504 us	-1.3%	+71.4%
bs=100 sw=10 sl=64	p95	164,934 us	154,049 us	74,866 us	+7.1%	+120.3%
bs=100 sw=10 sl=64	p99	164,934 us	154,049 us	89,939 us	+7.1%	+83.4%
bs=1000 sw=10 sl=64	throughput	951 tuples/sec	965 tuples/sec	1,569 tuples/sec	-1.5%	-39.4%
bs=1000 sw=10 sl=64	MB/s	0.581 MB/s	0.589 MB/s	0.957 MB/s	-1.4%	-39.3%
bs=1000 sw=10 sl=64	p50	1,041,702 us	1,030,535 us	669,501 us	+1.1%	+55.6%
bs=1000 sw=10 sl=64	p95	1,186,919 us	1,112,152 us	706,487 us	+6.7%	+68.0%
bs=1000 sw=10 sl=64	p99	1,186,919 us	1,112,152 us	737,560 us	+6.7%	+60.9%

Raw CSV

config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,538.03,200,128000,372,0.227,25483.63,39939.08,39939.08
1,100,10,64,20,2439.33,2000,1280000,820,0.500,117438.88,164934.20,164934.20
2,1000,10,64,20,21020.14,20000,12800000,951,0.581,1041702.39,1186918.63,1186918.63

[carloea2]

The full local REST-backed LoopIntegrationSpec now passes all 8 tests with this fix, with no failures or cancellations. The unchanged baseline reproduced the S3 read error. The new #8282 Ubuntu failure also shows the same AccessDenied path.

[carloea2]

@aglinxinyuan, all build and test checks are green, and all 8 REST-backed loop tests pass locally. Could you review and merge once the required backport approvals are in?

@xuang7 @mengw15, could you approve or decline the backports for your releases? The v1.2 preflight needs a manual backport.

[Copilot]

🟢 Approval recommended

The targeted workaround addresses the reported race, documents its process-wide scope, and includes focused regression coverage with no unresolved findings.

No open issues 🎉

What changed in this PR

This PR addresses the S3 request-signer sharing race behind intermittent Iceberg materialization read failures. The workaround disables fsspec’s process-wide S3 instance cache while preserving PyIceberg’s per-FileIO caching.

Changes:

• Disable global S3 filesystem caching when creating a REST catalog.
• Add regression coverage for filesystem isolation across FileIO instances, retained FileIO caching, and unchanged local-filesystem caching.

File	Description
amber/​src/​test/​python/​core/​storage/​iceberg/​test_iceberg_utils_catalog.py	Test isolation and cache preservation under three initial cache configurations.
amber/​src/​main/​python/​core/​storage/​iceberg/​iceberg_utils.py	Disable shared S3 filesystem instances during REST catalog creation.

_{Review effort: Balanced}

[mengw15]

LGTM

[carloea2]

@xuang7, v1.3 is approved and all build checks pass. The remaining gate needs your v1.2 decision: could you approve the backport or remove that label if you decline? @mengw15, could you merge once that gate clears?

[xuang7]

LGTM

[xuang7]

LGTM. Removing release/v1.2: the observed trigger is the loop path, which is not on v1.2.

[github-actions]

Backport PR opened: draft #9136 (#9136) to release/v1.3, assigned to @carloea2 — needs manual work because the backported tree failed its pre-merge build.