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

server: Set max refresh sessions threshold to 8 #2085

Merged
merged 2 commits into from
Nov 2, 2021

Conversation

yondonfu
Copy link
Member

@yondonfu yondonfu commented Nov 2, 2021

What does this pull request do? Explain your changes. (required)

This PR sets the max refresh sessions threshold to 8.

Prior to this PR, a B would refresh its session list (resulting in running orchestrator discovery to create a new session list) whenever the size of the list falls below numOrchs / 2 where numOrchs is the # of Os that the B is aware of. The problem with this check is illustrated by the following example:

  • numOrchs = 100
  • Only 50 Os out of 100 respond during discovery
  • As a result, the typical size of the session list is 50
  • If a single O in that session list returns an error and is suspended from the list the size of the list falls to 49
  • B refreshes its session list
  • Repeat the above process

The consequence is that B can end up running discovery/refreshing its session list really frequently.

This PR changes B's behavior so that if numOrchs / 2 > 8, then B will only refresh its session list if the size falls below 8. However, if numOrchs / 2 < 8 then B will refresh its session list if the size falls below numOrchs / 2. This means that if B is aware of a lot of Os (i.e. on the public network) it will wait until the size of the session list falls to a lower level before refreshing. But, if B is not aware of many Os (i.e. in a smaller local private network) it will refresh when the session list is at a higher level.

Specific updates (required)

See commit history.

How did you test each of these updates (required)

Tested manually. Would like to add unit tests for this area of the codebase but haven't gotten to it yet...

Does this pull request close any open issues?

Fixes #2080

Checklist:

Copy link
Contributor

@jailuthra jailuthra left a comment

Choose a reason for hiding this comment

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

LGTM

@yondonfu yondonfu merged commit 56bedf5 into master Nov 2, 2021
@yondonfu yondonfu deleted the yf/refresh-session-check branch November 2, 2021 15:11
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.

Fix cyclic CPU usage spikes on broadcasters due to discovery requests
2 participants