-
Notifications
You must be signed in to change notification settings - Fork 686
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
[v2.4] Port commits from PR #4488 #4514
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TestBootStrapNoNotifyBeforeSync creates ConfigMaps during its tests which doesn't get cleaned up afterwards potentially infecting other tests. Add t.Cleanup to the test to clean up those ConfigMaps after the test is finished. Signed-off-by: Hamzah Qudsi <[email protected]>
The Accumulator struct attempts to coalece changes into a single snapshot update as a way to do graceful load shedding. However, while this was the behavior on bootstrap, it didn't always happen mid-watch - each event that was received turned into a single snapshot update, thus not really satisfying this requirement. We add a new option to batch changes for a specified window interval before sending a snapshot update. The batching behavior is as follows: - The Accumulator will receive raw changes up until the window period where it will then send a change, even if new updates are still coming in. This is to prevent the potential of a scenario where a change is never sent due to an extremely volatile cluster. While there may be a way to be more dynamic in how long to wait before sending this change, this approach is simpler and more predicable. - If an isolated updated comes in (e.g. last change was submitted an hour ago but the window period is set to 10s), it may not neccessarily wait until the window period before sending change, it can send immediately. - The default interval is set to 1s to be inline with current change velocity. - A snapshot update won't be sent until all resources are fully bootstrapped, regardless of what interval is set. This is the ensure that the other requirements for the Accumulator are still satisfied. For testing, we add new test cases. Signed-off-by: Hamzah Qudsi <[email protected]>
AMBASSADOR_RECONFIG_MAX_DELAY controls the interval to wait before sending snapshot updates when listening for K8s resources, especially when many resources are updated in quick succession. Signed-off-by: Hamzah Qudsi <[email protected]>
Signed-off-by: Hamzah Qudsi <[email protected]>
haq204
force-pushed
the
hqudsi/kates-cherrypick
branch
from
September 15, 2022 17:12
335b9cf
to
091be77
Compare
ddymko
approved these changes
Sep 15, 2022
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.
cherrypicked files - looks good
LanceEa
approved these changes
Sep 15, 2022
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.
lgtm
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Cherry-pick from #4488
091be77 updates 2.4 release notes
Related Issues
N/A
Testing
CI
Checklist
I made sure to update
CHANGELOG.md
.Remember, the CHANGELOG needs to mention:
This is unlikely to impact how Ambassador performs at scale.
Remember, things that might have an impact at scale include:
My change is adequately tested.
Remember when considering testing:
I updated
DEVELOPING.md
with any any special dev tricks I had to use to work on this code efficiently. - N/AThe changes in this PR have been reviewed for security concerns and adherence to security best practices.