Conversation
|
PR Review for chore(epoxy): clean up kv get Well-executed cleanup. Summary of findings: POSITIVE: Returning Option directly removes the Output wrapper struct. Moving read_local_value into get_local.rs improves locality. key slice parameter removes gratuitous clones. tokio::try_join! reduces worst-case latency from 3x RTT to 1x RTT. The .or() combinator usage is idiomatic. TRADE-OFF: The old sequential code short-circuited on hit (legacy/cache reads skipped if V2 value found). The new parallel code always fires all three reads. Acceptable for a cleanup but worth revisiting after full backfill migration. SUGGESTION: responses.first() relies on an invariant that should be a debug_assert_eq!(responses.len(), 1) rather than just a comment. NOTE: get_optimistic.rs still has a glob import from anyhow despite CLAUDE.md recommending specific imports. This was pre-existing but the file was touched and it would have been a good opportunity to fix it. CORRECTNESS: tokio::try_join! on shared tx references relies on tx.get taking shared self. Appears correct given the transaction API design. VERDICT: Clean, well-scoped refactor. No functional regressions. All suggestions are minor and non-blocking. |
cd89bc9 to
f4762f2
Compare
e68c174 to
0ac1091
Compare
42b7a5c to
0b43aa9
Compare
| mutable: bool | ||
| } | ||
|
|
||
| type CommittedEntry struct { |
f4762f2 to
002100d
Compare
0b43aa9 to
4cd09cd
Compare
002100d to
fc3149f
Compare
4cd09cd to
1b7c05c
Compare
1b7c05c to
570ddec
Compare
fc3149f to
26f63c1
Compare
Merge activity
|
570ddec to
26e98ec
Compare

Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: