Skip to content

chore(epoxy): clean up kv get#4604

Merged
MasterPtato merged 1 commit intomainfrom
04-09-fix_epoxy_add_list_operation
Apr 13, 2026
Merged

chore(epoxy): clean up kv get#4604
MasterPtato merged 1 commit intomainfrom
04-09-fix_epoxy_add_list_operation

Conversation

@MasterPtato
Copy link
Copy Markdown
Contributor

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Copy link
Copy Markdown
Contributor Author

MasterPtato commented Apr 10, 2026

@claude
Copy link
Copy Markdown

claude bot commented Apr 10, 2026

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.

@MasterPtato MasterPtato mentioned this pull request Apr 11, 2026
11 tasks
@MasterPtato MasterPtato marked this pull request as ready for review April 11, 2026 22:07
@MasterPtato MasterPtato force-pushed the 04-09-fix_pb-envoy_reduce_round_trips_for_ws_connection branch from cd89bc9 to f4762f2 Compare April 12, 2026 19:42
@MasterPtato MasterPtato force-pushed the 04-09-fix_epoxy_add_list_operation branch from e68c174 to 0ac1091 Compare April 12, 2026 19:42
@MasterPtato MasterPtato changed the title fix(epoxy): add list operation chore(epoxy): clean up kv get Apr 12, 2026
@MasterPtato MasterPtato force-pushed the 04-09-fix_epoxy_add_list_operation branch 3 times, most recently from 42b7a5c to 0b43aa9 Compare April 12, 2026 20:39
mutable: bool
}

type CommittedEntry struct {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

@MasterPtato MasterPtato force-pushed the 04-09-fix_pb-envoy_reduce_round_trips_for_ws_connection branch from f4762f2 to 002100d Compare April 12, 2026 21:21
@MasterPtato MasterPtato force-pushed the 04-09-fix_epoxy_add_list_operation branch from 0b43aa9 to 4cd09cd Compare April 12, 2026 21:22
@MasterPtato MasterPtato force-pushed the 04-09-fix_pb-envoy_reduce_round_trips_for_ws_connection branch from 002100d to fc3149f Compare April 12, 2026 23:38
@MasterPtato MasterPtato force-pushed the 04-09-fix_epoxy_add_list_operation branch from 4cd09cd to 1b7c05c Compare April 12, 2026 23:38
@MasterPtato MasterPtato force-pushed the 04-09-fix_epoxy_add_list_operation branch from 1b7c05c to 570ddec Compare April 13, 2026 00:20
@MasterPtato MasterPtato force-pushed the 04-09-fix_pb-envoy_reduce_round_trips_for_ws_connection branch from fc3149f to 26f63c1 Compare April 13, 2026 00:20
Copy link
Copy Markdown
Contributor Author

MasterPtato commented Apr 13, 2026

Merge activity

  • Apr 13, 12:21 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Apr 13, 12:32 AM UTC: Graphite rebased this pull request as part of a merge.
  • Apr 13, 12:32 AM UTC: @MasterPtato merged this pull request with Graphite.

@MasterPtato MasterPtato changed the base branch from 04-09-fix_pb-envoy_reduce_round_trips_for_ws_connection to graphite-base/4604 April 13, 2026 00:28
@MasterPtato MasterPtato changed the base branch from graphite-base/4604 to main April 13, 2026 00:30
@MasterPtato MasterPtato force-pushed the 04-09-fix_epoxy_add_list_operation branch from 570ddec to 26e98ec Compare April 13, 2026 00:31
@MasterPtato MasterPtato merged commit 56a630a into main Apr 13, 2026
5 of 7 checks passed
@MasterPtato MasterPtato deleted the 04-09-fix_epoxy_add_list_operation branch April 13, 2026 00:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants