Repository navigation
fix(setup): give xet-core duration defaults a unit so they apply - #245
Merged
Merged
Conversation
xet-core parses durations with humantime, which rejects a bare number, and then keeps its own default with a WARN that the hf_mount=info filter hides. HF_XET_CLIENT_READ_TIMEOUT=30 and HF_XET_RECONSTRUCTION_TARGET_BLOCK_COMPLETION_TIME=30 were therefore never applied: reads ran with a 300 s inactivity timeout instead of 30 s, and prefetch targeted 15 minutes of transfer instead of 30 s. Use 30s. The defaults move into xet_env_defaults() so a unit test can pass each one through XetConfig::with_config, which rejects a value xet-core cannot parse and a setting it no longer knows.
XciD
marked this pull request as ready for review
September 29, 2026 21:00
Contributor
Benchmark Results |
Contributor
POSIX Compliance (pjdfstest) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
xet-core parses durations with humantime, which rejects a bare number. It then keeps its own default and logs a WARN under
xet_runtime, which the defaulthf_mount=infofilter hides. Two of the defaults set ininit_tracinghave therefore never been applied:HF_XET_CLIENT_READ_TIMEOUT=30: reads ran with the 300 s default inactivity timeout, not 30 s.HF_XET_RECONSTRUCTION_TARGET_BLOCK_COMPLETION_TIME=30: prefetch targeted 15 minutes of transfer, not 30 s.With
RUST_LOG=info, v0.13.1 logsConfiguration value 30 for read_timeout cannot be parsed into correct type; reverting to default.(same fortarget_block_completion_time).Fix
Both values are now
30s. The defaults move intoxet_env_defaults(), andxet_env_defaults_are_accepted_by_xet_corepasses each one throughXetConfig::with_configfor itsHF_XET_<GROUP>_<FIELD>path. That rejects a value xet-core cannot parse and a setting it no longer knows (for example after an upstream rename). On main, the test fails and lists exactly these two defaults. The other nine pass.Effect
With this branch, xet-core logs
read_timeout = 30s (user set)andtarget_block_completion_time = 30s (user set). The prefetch target israte x target_block_completion_time, bounded by the 256 MiB download buffer limit, so fast streams see no change and only slow streams prefetch less. Cold sequential reads of a 3.09 GB xet file (Qwen/Qwen2.5-1.5B-Instruct,--no-disk-cache, m6i.2xlarge, 8 alternating runs each) gave a median of 556 MB/s on v0.13.1 and 558 MB/s with this change, within network noise.This PR and #244 both touch the xet env defaults list, so whichever lands second needs a rebase. The new test will then also check
HF_XET_TELEMETRY_FINAL_FLUSH_TIMEOUT.