Skip to content
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

(CC refactor 8) recovery: move all congestion control related fields to a new struct #1778

Closed
wants to merge 3 commits into from

Conversation

ghedo
Copy link
Member

@ghedo ghedo commented May 2, 2024

No description provided.

@ghedo ghedo requested a review from a team as a code owner May 2, 2024 13:01
@ghedo ghedo force-pushed the cc-refactor-7-detect-acked-epoch branch from 194b849 to ed2c784 Compare May 3, 2024 09:42
@ghedo ghedo force-pushed the cc-refactor-8-cc-struct branch from c52ef3d to 74d6d98 Compare May 3, 2024 09:43
Comment on lines -682 to -683
// To avoid rollback
r.lost_count += MIN_ROLLBACK_THRESHOLD;
Copy link
Contributor

Choose a reason for hiding this comment

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

This PR is pretty obtuse to review, so I just wanted to check, is the removal here was covered somewhere else?

Copy link
Member Author

Choose a reason for hiding this comment

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

AFAICT this was never actually necessary, because the rollback doesn't happen either way.

Comment on lines -1257 to -1258
// To avoid rollback
r.lost_count += MIN_ROLLBACK_THRESHOLD;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above regarding what this removal means

Copy link
Member Author

Choose a reason for hiding this comment

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

Same answer :)

@ghedo ghedo force-pushed the cc-refactor-8-cc-struct branch from 74d6d98 to 505df84 Compare May 7, 2024 13:01
@ghedo ghedo changed the base branch from cc-refactor-7-detect-acked-epoch to master May 7, 2024 13:02
@ghedo ghedo force-pushed the cc-refactor-8-cc-struct branch from 505df84 to fb68839 Compare May 9, 2024 08:34
@ghedo
Copy link
Member Author

ghedo commented Jun 25, 2024

Merged as f73f869.

@ghedo ghedo closed this Jun 25, 2024
@ghedo ghedo deleted the cc-refactor-8-cc-struct branch June 25, 2024 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants