[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 91.93%. Comparing base (98c6337) to head (086837d).

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #9129      +/-   ##
============================================
+ Coverage     91.87%   91.93%   +0.05%     
  Complexity     5049     5049              
============================================
  Files          1265     1265              
  Lines         54915    55459     +544     
  Branches       6940     7046     +106     
============================================
+ Hits          50453    50986     +533     
- Misses         2769     2780      +11     
  Partials       1693     1693              

Flag	Coverage Δ		*Carryforward flag
access-control-service	77.38% <ø> (ø)		Carriedforward from 98c6337
agent-service	99.16% <ø> (ø)		Carriedforward from 98c6337
amber	86.06% <ø> (ø)		Carriedforward from 98c6337
computing-unit-managing-service	60.48% <ø> (ø)		Carriedforward from 98c6337
config-service	87.37% <ø> (ø)		Carriedforward from 98c6337
file-service	82.00% <ø> (ø)		Carriedforward from 98c6337
frontend	96.50% <ø> (ø)		Carriedforward from 98c6337
notebook-migration-service	83.73% <ø> (ø)		Carriedforward from 98c6337
pyamber	98.48% <100.00%> (-0.06%)	⬇️
workflow-compiling-service	72.43% <ø> (ø)		Carriedforward from 98c6337

*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 · 🔴 8 worse · ⚪ 7 noise (<±5%) · 0 without baseline

Compared against main 98c6337 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	375	0.229	26,513/33,984/33,984 us	🔴 -15.0% / 🔴 +138.8%
🔴	bs=100 sw=10 sl=64	837	0.511	117,615/147,377/147,377 us	🔴 +12.6% / 🔴 +52.8%
⚪	bs=1000 sw=10 sl=64	991	0.605	1,012,108/1,066,711/1,066,711 us	⚪ within ±5% / 🔴 +17.2%

Baseline details

Latest main 98c6337 from same runner

config	metric	PR	latest main	7d avg	Δ latest	Δ 7d
bs=10 sw=10 sl=64	throughput	375 tuples/sec	441 tuples/sec	879.16 tuples/sec	-15.0%	-57.3%
bs=10 sw=10 sl=64	MB/s	0.229 MB/s	0.269 MB/s	0.537 MB/s	-14.9%	-57.3%
bs=10 sw=10 sl=64	p50	26,513 us	24,620 us	11,504 us	+7.7%	+130.5%
bs=10 sw=10 sl=64	p95	33,984 us	33,951 us	14,234 us	+0.1%	+138.8%
bs=10 sw=10 sl=64	p99	33,984 us	33,951 us	17,338 us	+0.1%	+96.0%
bs=100 sw=10 sl=64	throughput	837 tuples/sec	902 tuples/sec	1,147 tuples/sec	-7.2%	-27.1%
bs=100 sw=10 sl=64	MB/s	0.511 MB/s	0.55 MB/s	0.7 MB/s	-7.1%	-27.0%
bs=100 sw=10 sl=64	p50	117,615 us	110,117 us	88,987 us	+6.8%	+32.2%
bs=100 sw=10 sl=64	p95	147,377 us	130,901 us	96,473 us	+12.6%	+52.8%
bs=100 sw=10 sl=64	p99	147,377 us	130,901 us	102,791 us	+12.6%	+43.4%
bs=1000 sw=10 sl=64	throughput	991 tuples/sec	996 tuples/sec	1,182 tuples/sec	-0.5%	-16.2%
bs=1000 sw=10 sl=64	MB/s	0.605 MB/s	0.608 MB/s	0.721 MB/s	-0.5%	-16.1%
bs=1000 sw=10 sl=64	p50	1,012,108 us	1,006,559 us	872,751 us	+0.6%	+16.0%
bs=1000 sw=10 sl=64	p95	1,066,711 us	1,047,548 us	910,151 us	+1.8%	+17.2%
bs=1000 sw=10 sl=64	p99	1,066,711 us	1,047,548 us	941,479 us	+1.8%	+13.3%

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,533.67,200,128000,375,0.229,26512.56,33984.40,33984.40
1,100,10,64,20,2389.52,2000,1280000,837,0.511,117615.48,147377.04,147377.04
2,1000,10,64,20,20191.73,20000,12800000,991,0.605,1012108.24,1066710.57,1066710.57

[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