sundapeng opened a new pull request, #10552: URL: https://github.com/apache/paimon/pull/10552
### Purpose #9715 replaced the process-wide FileIO cache of `RESTTokenFileIO` (1000 entries shared by all instances, as in Java's `FILE_IO_CACHE`) with a `TTLCache(maxsize=1)` per instance. The goal was to stop two tables in different buckets that receive the same token from sharing one bucket-bound `OssFileIO` ([review comment](https://github.com/apache/paimon/pull/9715#discussion_r3977978369)). With one entry per instance, a token refresh evicts the previous `FileIO` at the moment of the refresh. Tokens are refreshed while they still have up to one hour left, so streams opened with the previous credentials are still valid and often still open. With the Jindo backend, releasing that `FileIO` freed the native store under those streams, and the process crashed with SIGSEGV on their next read, seek, write or close. Readers that hold only a stream hit this: Avro, BLOB, Parquet/ORC by row group, the bitmap and B-tree indexes. Long-running readers crashed this way. #10536 makes Jindo streams pin their filesystem, which removes the crash, but the cache still drops a `FileIO` that is in use at every refresh, and each `RESTTokenFileIO` object builds its own `FileIO` and Jindo client. This pull request restores the process-wide cache and keeps what #9715 fixed: - One `TTLCache` shared by all instances, 1000 entries and 10 hours as before #9715. A refresh adds an entry for the new token and leaves the previous `FileIO` in place. - The key is the token plus the table path and the catalog options, the inputs `FileIO.get` builds the `FileIO` from. The same token on another bucket, or with another `fs.oss.impl`, gets a separate `FileIO`. - Entries expire 10 hours after their last use instead of their creation, like Java's `expireAfterAccess`. A `FileIO` in use is no longer replaced every 10 hours, and the previous token's `FileIO` is released 10 hours after its last use, long after its credentials expired. - `RESTTokenFileIO` objects of one table in one process share one `FileIO`. Eight objects for one table built 8 Jindo clients before (16 after one refresh) and build 1 now (2 after one refresh). The per-instance lock still serializes token refresh and lookup for one object. The shared cache has its own lock, and a missing `FileIO` is created under it, so concurrent lookups build one. The keep-alive comment in `jindo_file_system_handler.py` that named the `maxsize=1` eviction is updated. This pull request also carries the one-line flake8 fix of #10550 so its CI can pass before that merges; the identical hunks merge cleanly in either order. ### Tests - `rest_token_file_io_test.py` - A stream opened before a refresh is still readable after it. The refresh goes through `try_to_refresh_token` with less than one hour left, and the fake stream, like a pyjindo stream, does not keep its `FileIO` alive. On master this fails with "use-after-free". - The same token with another bucket or other catalog options gets a separate `FileIO`; a real `OssFileIO` keeps its own bucket. - Objects of one table, including an unpickled copy, share one `FileIO`, and concurrent lookups create one. - An entry used within 10 hours stays; the previous token's entry is released after 10 idle hours. - `test_pickle_serialization` now expects the unpickled copy to reuse the process-wide cache. - `jindo_file_system_test.py`: a refresh through `OssFileIO` and `JindoFileSystemHandler` with a fake pyjindo keeps the previous handler alive, its stream keeps reading, and two objects share one client. - On master, 6 of the new or changed tests fail. Keying by the token alone fails the two isolation tests, dropping the re-insert fails the expiry test, and dropping the cache lock fails the concurrency test. - The REST, OSS, PVFS, caching, resolving FileIO, URI reader and Jindo test files give the same results as master apart from the 7 new tests (Python 3.11). The Python 3.6 selection that CI runs (pyarrow 6) also matches master. - flake8 with `dev/cfg.ini` over the package passes. ### Verified with JindoSDK pyjindosdk 6.10.4, pyarrow 19.0.1, Python 3.11, against a local OSS-compatible server, 3 runs per cell. "without #10536" uses `jindo_file_system_handler.py` from before #10536, to show what the cache change does on its own. Streams opened through `RESTTokenFileIO.new_input_stream` / `new_output_stream` on a 4 MiB object: | Scenario | master without #10536 | master | this PR without #10536 | this PR | |---|---|---|---|---| | Token unchanged, read the open stream | ok | ok | ok | ok | | New token, then read the stream opened before it | SIGSEGV | ok | ok | ok | | Same, refreshed by `try_to_refresh_token` (token issued with 1 h + 2 s left) | SIGSEGV | ok | ok | ok | | Another thread reads while the token is refreshed 5 times | SIGSEGV | ok | ok | ok | | Write stream open across a refresh | SIGSEGV | ok | ok | ok | | Drop an unclosed stream after a refresh | SIGSEGV | ok | ok | ok | | 4 threads open and read streams while the token is refreshed 50 times | SIGSEGV or SIGABRT | ok | ok | ok | | `FileIO` idle past the TTL (shortened to 1 s) while a stream is open | SIGSEGV | ok | SIGSEGV | ok | | Jindo clients for 8 objects of one table (after one refresh) | 8 (16) | 8 (16) | 1 (2) | 1 (2) | pypaimon table reads and writes. The table is written with `FileSystemCatalog` and read or written through a `FileStoreTable` on a `RESTTokenFileIO`, whose tokens are issued with 1 h + 2 s left, so a refresh happens every 2 s: | Scenario | master without #10536 | master | this PR without #10536 | this PR | |---|---|---|---|---| | Avro read paused mid-file across a refresh | SIGSEGV | ok | ok | ok | | Avro, 4 reader threads | SIGSEGV | ok | ok | ok | | Parquet, both scenarios above (the dataset path holds the filesystem) | ok | ok | ok | ok | | Write and commit 3 snapshots while tokens roll, read back | ok | ok | ok | ok | | Jindo clients built to read 8 objects of one table | 9 | 9 | 2 | 2 | Fork behavior is unchanged. A forked child hangs in `JindoFileSystem.open` once the parent has used pyjindo, on master and with this change, whether the child reads through the inherited `RESTTokenFileIO` or a new one; it reads normally when the parent never used pyjindo. With `fs.oss.impl=legacy` the child reads normally in every case. The cache alone does not cover a stream that is held past the 10 hour TTL while nothing uses its `FileIO` (the TTL row above); #10536 covers that. ### API and Format No public API or storage format change. ### Documentation No. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
