-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reduce down time on restart #651
Conversation
📝 WalkthroughWalkthroughThis pull request updates configuration values in both production and staging files by modifying various operational flags. In each file, numerical parameters have been adjusted—most values are reduced significantly, with one parameter in staging increased. The changes affect flags related to economic modeling, ticketing, peer connectivity, topology settings, subgraph rotation, rewards/allocations, and node operations, as well as a minor formatting fix. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
ct-app/.configs/core_staging_config.yaml (2)
36-36
: Boolean Flag Formatting Note
The lineobserveMessageQueue: On # should only be On / Off
is correct per the inline note; however, consider whether switching to a boolean value (e.g.,true
/false
) might improve consistency and reduce ambiguity across your configuration files.
21-21
: Trailing Spaces Cleanup
Static analysis flagged trailing spaces on this line. Please remove any extra whitespace to conform to YAML lint standards.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 21-21: trailing spaces
(trailing-spaces)
ct-app/.configs/core_prod_config.yaml (2)
39-41
: Peer Communication Flag Format
ThemessageRelayRequest: On
flag continues to use a string value. If your system supports boolean flags, consider standardizing (e.g.,true
/false
) for better consistency across different modules.
21-21
: Trailing Spaces Cleanup
Similar to the staging configuration, static analysis flagged trailing spaces on this line. Removing such extra whitespaces will help maintain YAML format consistency.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 21-21: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ct-app/.configs/core_prod_config.yaml
(1 hunks)ct-app/.configs/core_staging_config.yaml
(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
ct-app/.configs/core_staging_config.yaml
[error] 21-21: trailing spaces
(trailing-spaces)
ct-app/.configs/core_prod_config.yaml
[error] 21-21: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and push container image
🔇 Additional comments (14)
ct-app/.configs/core_staging_config.yaml (5)
7-8
: Parameter Adjustments in the “core” Section
The values forapplyEconomicModel: 30
andticketParameters: 60
have been reduced (halved) to meet the intended operational adjustments. Please verify that these lower thresholds still satisfy the intended economic modulation without unexpectedly affecting distribution timing.
10-11
: Network Configuration Updates
The changes forconnectedPeers: 30
andtopology: 30
are consistent with the overall reduction strategy. Confirm that these reduced values maintain sufficient connectivity for your staging environment.
13-13
: Subgraph Rotation Frequency
The update torotateSubgraphs: 60
represents a significant reduction. Ensure that the increased frequency (or decreased interval) does not strain related services or impact stability.
15-20
: Core Reward and Allocation Settings
The modifications forpeersRewards
,registeredNodes
,allocations
,EOABalances
,NFTHolders
, andsafeFundings
(all set to 300) appear uniformly applied. It’s important to test these values in operational scenarios to verify they produce the anticipated reduction in delay before distribution post-restart.
23-25
: Node Section Adjustments
The updates—healthcheck: 15
,retrievePeers: 60
, andretrieveChannels: 120
—reduce the operational waiting periods. Verify that these changes provide improved responsiveness without compromising service stability.ct-app/.configs/core_prod_config.yaml (9)
7-8
: Production “core” Section Parameter Updates
The changes forapplyEconomicModel: 30
andticketParameters: 60
mirror the staging adjustments and appear consistent. Confirm that these revised values align with production performance expectations.
10-11
: Adjusted Network Parameters
ReducingconnectedPeers
to 30 andtopology
to 30 is a drastic shift from previous configurations. Make sure that these lower numbers do not inadvertently limit network resilience during high load in production.
13-13
: Subgraph Rotation Setting
The production change torotateSubgraphs: 60
should be evaluated to ensure it does not lead to overly frequent subgraph updates which might affect downstream services or synchronization.
15-20
: Core Rewards and Allocation Adjustments
The parameters forpeersRewards
,registeredNodes
,allocations
,EOABalances
,NFTHolders
, andsafeFundings
have been uniformly set to 300. This consistency is good; please validate that these values integrate well into the broader production logic.
23-25
: Node Configuration Revisions
The updatedhealthcheck: 15
andretrievePeers: 60
(withretrieveChannels: 120
andretrieveBalances: 300
) reduce service delay. Double-check that these adjustments contribute effectively to reducing the delay before distribution after restart while not affecting critical node health assessments.
28-32
: Channel Operations Settings
The values for channel operations (fromopenChannels
tocloseIncomingChannels: Off
) remain unchanged. Ensure these default values still work harmoniously with the reduced delays in other sections.
34-37
: Message Queue and Relay Configurations
The settingsobserveMessageQueue: On
andobserveRelayedMessages: 15
appear appropriate. They should be tested in context to verify that message processing and relaying behaviors meet the revised performance criteria.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 35-35: trailing spaces
(trailing-spaces)
47-57
: Economic Model Legacy Parameters in Production
The legacy economic model settings—includingNFTThreshold: 30000
,apr: 12.5
, and coefficient adjustments (c: 75000
,l: 10000
)—differ significantly from the staging defaults. Please ensure these production-specific modifications have been thoroughly reviewed with the economic model stakeholders to avoid unintended financial implications.
68-72
: Sigmoid Model Adjustments for Production Scaling
The updates to the sigmoid parameters (maxAPR: 15
,networkCapacity: 2000
, andtotalTokenSupply: 450000000
) represent a major shift for production scaling. It is advisable to simulate these changes to confirm that they yield the intended impact on system behavior and financial modeling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
ct-app/.configs/core_staging_config.yaml (2)
21-21
: Remove Trailing Spaces.
YAMLlint has flagged trailing spaces on this line. Please remove any extraneous whitespace to comply with YAML formatting standards.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 21-21: trailing spaces
(trailing-spaces)
36-36
: Observation Queue Formatting.
The settingobserveMessageQueue
is set to "On" with an inline comment noting the acceptable values. To avoid potential parsing issues, ensure that the value is strictly "On" (or "Off") without additional formatting artifacts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ct-app/.configs/core_staging_config.yaml
(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
ct-app/.configs/core_staging_config.yaml
[error] 21-21: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and push container image
🔇 Additional comments (6)
ct-app/.configs/core_staging_config.yaml (6)
7-8
: Parameter Reduction for Economic Model and Ticket Settings.
The values forapplyEconomicModel
(30) andticketParameters
(60) are reduced, likely to help minimize restart delays. Please validate these adjustments with relevant performance benchmarks.
10-11
: Updated Core Network Parameters.
TheconnectedPeers
andtopology
values have been lowered to 30. Confirm that these reduced thresholds continue to support necessary network connectivity and do not adversely impact system stability.
13-13
: Rotation Parameter Tuning.
The value forrotateSubgraphs
is now set to 60, which should quicken subgraph rotations. Ensure that this new rate meets operational requirements without introducing instability in the subgraph management.
15-20
: Uniform Value Setting for Reward and Funding Parameters.
ParameterspeersRewards
,registeredNodes
,allocations
,EOABalances
,NFTHolders
, andsafeFundings
are all updated to 300. Confirm that these uniform reductions align with your performance objectives and do not adversely affect related processes.
23-25
: Node Configuration Adjustments.
Within thenode
section,healthcheck
is decreased to 15 andretrievePeers
to 60, which is consistent with the overall reduction theme. However, note thatretrieveChannels
is increased to 120. Please verify that this increase is intentional and calibrated to support channel processing requirements.
47-47
: Clarification on NFTThreshold Value.
The lineNFTThreshold: ~
uses the YAML null indicator~
. Please confirm whether this is intended to represent a null value or if a specific numeric threshold should be provided.
With those parameters, the average down time during a restart of the CT pod is around 2min, which is a nice improvement from the delay on prod until now which is around 15min.
As the staging and prod parameters set for execution delays are the same, the same delay is expected on prod.
Summary by CodeRabbit
Summary by CodeRabbit