Skip to content

polish constants#914

Merged
ZanCorDX merged 1 commit intodevelopfrom
dx/inc-channel-size
Apr 9, 2026
Merged

polish constants#914
ZanCorDX merged 1 commit intodevelopfrom
dx/inc-channel-size

Conversation

@ZanCorDX
Copy link
Copy Markdown
Collaborator

@ZanCorDX ZanCorDX commented Apr 8, 2026

📝 Summary

Increased ORDER_JOURNAL_CHANNEL_SIZE x 10 since it would get filled in busy slots.
Removed 2 unused constants.

✅ I have completed the following steps:

  • Run make lint
  • Run make test
  • Added tests (if applicable)

@ZanCorDX ZanCorDX requested a review from dvush as a code owner April 8, 2026 19:35
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 8, 2026

Claude finished @ZanCorDX's task in 40s —— View job


Review of PR #914

  • Read changed files and diff
  • Review changes and provide feedback

Changes look good. The 10x increase to ORDER_JOURNAL_CHANNEL_SIZE (10K → 100K) is reasonable given the stated 30K-40K orders per busy slot, and the two removed constants (DEFAULT_RESULTS_CHANNEL_TIMEOUT, DEFAULT_INPUT_CHANNEL_BUFFER_SIZE) are confirmed unused anywhere in the codebase.

One minor issue found — typo in the comment ("slow" → "slot") — posted as an inline comment.


/// Channel size for the order journal. Orders are small and arrive frequently.
const ORDER_JOURNAL_CHANNEL_SIZE: usize = 10 * 1024;
/// A busy slow can have 30K-40K orders. 100K should be more than enough.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: typo — "slow" → "slot".

Suggested change
/// A busy slow can have 30K-40K orders. 100K should be more than enough.
/// A busy slot can have 30K-40K orders. 100K should be more than enough.

@ZanCorDX ZanCorDX merged commit fb94a25 into develop Apr 9, 2026
12 of 13 checks passed
@ZanCorDX ZanCorDX deleted the dx/inc-channel-size branch April 9, 2026 14:54
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