perf(import): cut receive-pack filepath writes and root-lock wait - #2177
Conversation
Batch blob file_path updates, hold the monorepo root lock only for the CAS attach (10ms Redis retry), and replace git_repo LIKE seq scans with index range / parent-walk lookups. crates-sync ETA now uses push rate instead of skip-inflated throughput.
Serialize 1000-row chunks, insert hashes in sorted order, and retry Postgres 40P01/40001 so concurrent receive-packs stop deadlocking on git_blob unique indexes. Reuse the persisted git_repo row on repo_path conflict instead of the caller's unused snowflake id.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa668b747f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let result = if let Some(txn) = txn { | ||
| insert.exec(txn).await |
There was a problem hiding this comment.
When this helper receives a DatabaseTransaction, a PostgreSQL deadlock or serialization failure aborts the entire transaction; retrying insert.exec(txn) on the same handle can only produce current transaction is aborted until rollback. Transactional callers such as attach_to_monorepo_parent_in_txn therefore still fail, but only after holding resources and sleeping through the retry loop. Propagate the conflict so the caller can roll back and retry the whole transaction, or create a fresh transaction for each attempt.
Useful? React with 👍 / 👎.
| window = _pushes_per_sec_last_60s() | ||
| if _push_ok_last_60s() >= 5 and window > 0: | ||
| return window |
There was a problem hiding this comment.
During the first 60 seconds, once five pushes complete this branch selects _pushes_per_sec_last_60s(), which always divides the count by 60 rather than by the elapsed window. For example, five pushes in five seconds are treated as 0.083 pushes/s instead of 1 push/s, inflating the displayed ETA by 12× precisely during startup. Continue using the lifetime rate until 60 seconds have elapsed, or divide the window count by the actual observed duration.
Useful? React with 👍 / 👎.
Hardcoded worker_id(1) made every replica share one partition, so concurrent generate_id() hit git_repo_pkey. Claim mega:snowflake:worker:N in Redis at boot (8+8 bits), with env/hash fallback when Redis is down.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9cf471de5a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Err(e) => { | ||
| tracing::warn!( | ||
| error = %e, | ||
| "snowflake worker slot claim failed; falling back to hash" | ||
| ); | ||
| return None; |
There was a problem hiding this comment.
When a slot claim encounters a transient Redis error, or when all 256 slots are occupied, returning None makes resolve_worker_id silently use an uncoordinated hash. That hash can select a worker ID already used by a live process—and necessarily does when every slot is occupied—so concurrent generators can emit identical Snowflake IDs in the same millisecond, causing primary-key conflicts or incorrect writes. Retry or fail startup rather than generate IDs without an exclusive slot.
Useful? React with 👍 / 👎.
| Err(e) => { | ||
| tracing::warn!(key = %key, error = %e, "snowflake worker slot refresh failed"); | ||
| break; |
There was a problem hiding this comment.
If any refresh attempt fails during a brief Redis interruption, this task exits permanently while the process keeps generating IDs with the claimed worker ID. After the 30-second TTL expires, another process can claim that slot and both processes may produce identical Snowflake IDs; transient refresh errors should be retried within the remaining lease window, and the process must stop generating IDs if ownership can no longer be guaranteed.
Useful? React with 👍 / 👎.
2 participants
Batch blob file_path updates, hold the monorepo root lock only for the CAS attach (10ms Redis retry), and replace git_repo LIKE seq scans with index range / parent-walk lookups. crates-sync ETA now uses push rate instead of skip-inflated throughput.