[OTE-876] update roundtable loop timings for instrumentation and uncrossing #2494
Conversation
WalkthroughThe changes in this pull request involve modifications to the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
| @@ -208,7 +208,7 @@ export const configSchema = { | |||
| AGGREGATE_TRADING_REWARDS_CHUNK_SIZE: parseInteger({ default: 50 }), | |||
|
|
|||
| // Uncross orderbook | |||
| STALE_ORDERBOOK_LEVEL_THRESHOLD_SECONDS: parseInteger({ default: 10 }), | |||
| STALE_ORDERBOOK_LEVEL_THRESHOLD_SECONDS: parseInteger({ default: 10 }), | |||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
indexer/services/roundtable/src/config.ts (1)
211-211: Revert formatting change for consistencyThe extra space added before the colon in
STALE_ORDERBOOK_LEVEL_THRESHOLD_SECONDS: parseInteger({ default: 10 })introduces inconsistency in the file's formatting. Other similar lines in the file do not have this extra space.For consistency, please remove the extra space:
- STALE_ORDERBOOK_LEVEL_THRESHOLD_SECONDS: parseInteger({ default: 10 }), + STALE_ORDERBOOK_LEVEL_THRESHOLD_SECONDS: parseInteger({ default: 10 }),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- indexer/services/roundtable/src/config.ts (3 hunks)
🧰 Additional context used
🔇 Additional comments (3)
indexer/services/roundtable/src/config.ts (3)
73-74: LGTM: Reduced interval for uncrossing orderbookThe change from 30 seconds to 15 seconds for the
LOOPS_INTERVAL_MS_UNCROSS_ORDERBOOKaligns with the PR objective to make the uncrossing process more frequent. This could potentially improve market efficiency by updating the orderbook more often.To ensure this change doesn't negatively impact system performance, please monitor the following after deployment:
- CPU and memory usage of the Roundtable service
- Latency of orderbook updates
- Overall system responsiveness
If you notice any significant performance degradation, consider adjusting the interval or optimizing the uncrossing process.
82-83: LGTM: Increased interval for orderbook instrumentationThe change from 5 seconds to 1 minute for the
LOOPS_INTERVAL_MS_ORDERBOOK_INSTRUMENTATIONaligns with the PR objective to make the instrumentation process less frequent than the uncrossing process. This should reduce the system load for instrumentation tasks.To ensure this change doesn't negatively impact monitoring capabilities, please verify the following after deployment:
- The quality and usefulness of orderbook metrics at the new 1-minute interval
- Any dashboards or alerts that depend on orderbook instrumentation data
- The ability to detect and respond to rapid market changes
If you notice any significant loss of important information or reduced ability to monitor the system effectively, consider adjusting the interval or implementing additional metrics to compensate for the reduced frequency.
Line range hint
1-215: Summary: Changes align with PR objectives, post-deployment monitoring crucialThe changes in this file successfully implement the PR objective of updating loop timings to ensure that the uncrossing process occurs more frequently than the instrumentation process. The key modifications are:
- Reduced
LOOPS_INTERVAL_MS_UNCROSS_ORDERBOOKfrom 30 to 15 seconds- Increased
LOOPS_INTERVAL_MS_ORDERBOOK_INSTRUMENTATIONfrom 5 seconds to 1 minuteThese changes are approved, but it's crucial to monitor system performance and data quality post-deployment. Pay special attention to:
- System load and responsiveness
- Quality and usefulness of orderbook metrics
- Impact on monitoring capabilities and ability to detect rapid market changes
If any issues arise, consider fine-tuning these intervals or implementing additional optimizations.
Please ensure that thorough testing is conducted before deployment and that a rollback plan is in place in case of unexpected issues.
|
@Mergifyio backport release/indexer/v6.x |
✅ Backports have been created
|
|
https://github.com/Mergifyio backport release/indexer/v7.x |
✅ Backports have been created
|
…ossing (backport #2494) (#2505) Co-authored-by: Mohammed Affan <[email protected]>
…ossing (backport #2494) (#2506) Co-authored-by: Mohammed Affan <[email protected]>
Changelist
Updating values of loop timing so that uncrossing happens more frequently than instrumentation
Test Plan
[Describe how this PR was tested (if applicable)]
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.Summary by CodeRabbit