apache / apache/iceberg-python
Concurrent writes to a remote-signing catalog go out unsigned and fail 403
- 主要语言
- Python
- 星标
- 1.1k
- 派生
- 581
- 平均合并
- 1 天 17 小时
- 30 天内合并 PR
- 78
描述
### Apache Iceberg version
main (development) — also reproduced on 0.12.0 and 0.11.1.
### Please describe the bug 🐞
`_s3()` unregisters the S3 request signer and re-registers it on an event emitter that fsspec caches and every thread shares. A request signed in that window goes out with **no `Authorization` header** — `_s3()` has already set `config_kwargs["signature_version"] = UNSIGNED`, so botocore does not sign in its place — and the store answers `403 AccessDenied`.
`pyiceberg/io/fsspec.py`, in `_s3()`:
```python
fs = S3FileSystem(**s3_fs_kwargs)
for event_name, event_function in register_events.items():
fs.s3.meta.events.unregister(event_name, unique_id=1925) # opens the window
fs.s3.meta.events.register_last(event_name, event_function, unique_id=1925)
```
`FsspecFileIO.get_fs` caches per thread and every table gets its own `FileIO`, so a short workload makes hundreds of these cycles against the one shared emitter. PyIceberg's writer is concurrent by default, so no unusual usage is needed to reach it.
Roughly **3–4% of appends** fail against a remote-signing catalog, surfacing as an opaque `PermissionError: Access Denied` out of `s3fs`, several frames from its cause. Every unsigned request caught on the wire was a `PUT` of a manifest during commit.
Measured on Lakekeeper 0.13.1 + MinIO (path-style), 90 writes per run:
| configuration | append failures | unsigned on wire |
|---|---|---|
| as shipped | 2 / 4 / 5 | 2 / 4 / 5 |
| signer registered once (client still shared) | 0 | 0 |
| `PYICEBERG_MAX_WORKERS=1` | 0 | 0 |
The second row is the one that matters: the failures stop while the client is still shared, which separates this from a general concurrency problem. The response code is always `AccessDenied` and never `SignatureDoesNotMatch` — what an *unsigned* request produces, not a mis-signed one.
### Suggested fix
Drop the `unregister`. Botocore's `HierarchicalEmitter._register_section` returns early for a `unique_id` it already holds, so re-registering an equivalent signer was already a no-op; the unregister only opens the window.
### Reproduction
Two tests in `tests/io/test_fsspec.py`, no credentials and no network:
- `test_s3_leaves_a_signer_installed_while_reconfiguring_a_shared_client` — drives `_s3()` and observes the emitter the instant it unregisters.
- `test_the_signer_stays_installed_while_another_thread_reconfigures_s3` — the same window seen from another thread. The window is two adjacent statements, so sampling for it blind is a coin flip (20,000 observations caught it zero times); it is held open by delaying only the *re-registration*, so the timing is deterministic while the defect is not manufactured.
Both fail on `main` and pass with the fix.
### Already covered by an open PR
**#3783** (for #3625) removes this `unregister` as part of registering the signer on an `AioSession`, for a different reason — a lazily created client not inheriting handlers, failing `InvalidRequest`. I applied that PR and ran both tests above against it: **both pass**, so it fixes this as well.
Worth recording rather than closing silently, because the line is wrong for two independent reasons and #3783's own test does not cover this one. If #3783 lands, these two tests are the regression cover for the concurrency window; happy to raise them against that PR instead if a maintainer prefers.
### A second, independent defect in the same file — noted, not proposed here
`pyiceberg/io/fsspec.py:160` applies the signing service's headers with `add_header`, which appends, so every header the service echoes back appears twice — 880 of 1615 sign calls ended with a duplicate of a header named in `SignedHeaders`. On MinIO this is **latent**: de-duplicating changed nothing, and it did not contribute to the 403s above. SigV4 combines repeated headers comma-separated, so a stricter store may reject them. Mentioned so it is not lost, not to widen this issue.
### Willingness to contribute
I can contribute a fix to resolve this bug independently.
贡献指南
这个仓库没有索引到贡献指南
调研方向
从 pyiceberg/io/fsspec.py 中的 _s3() 开始,然后运行 tests/io/test_fsspec.py 中提到的两个测试。当 shared-client 重新配置时 signer 仍保持安装状态,并且两个并发回归测试都通过时,即表示完成;PR #3783 已经涵盖了所报告的修复。
由索引模型根据 Issue 内容生成。
评估
- 技术栈
- python
- 领域
- cloud
- Issue 类型
- 缺陷
- 难度
- 1/5
- 预计耗时
- 1 小时以内
- 活跃度
- 停滞
- 描述清晰度
- 描述清楚
- 新手友好度
- 35/100