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.
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
schedule: support patrol region concurrency #8094
schedule: support patrol region concurrency #8094
Changes from 8 commits
d1f4b8a
6d0fbd4
9b43d61
b2b4f39
cb285f6
2e14405
351ef5c
c198b08
a0ec33d
ab9ef1e
97a40a9
438efce
c59b47c
9f57397
bbc1362
b0eab80
5c442a3
0d02d8b
9896228
a638cce
6147373
2a86197
82785c2
a21ef83
cd1cd8b
5668d98
78e3ba5
cf01076
cc51a2e
9f7406a
bd4ca79
ae0778f
ecb8d8b
8259ad0
18db300
5ead31e
6bdb436
40a2e02
8cd8825
bcd5018
acb4244
457da3d
64abc3c
e570e3c
74d4fdc
aeabc52
7e5813d
240f902
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Maybe we can use max(128,region_count/1024)
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.
Shall we check it first?
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.
No, we need to update config. If we check IsSchedulingHalted firstly, it will skip updating config.
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.
If IsSchedulingHalted is true, do we need to update config immediately?
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.
If
IsSchedulingHalted
is true while checking first, we won't be able to update both configurations because the L300 will skip the later code.However, if IsSchedulingHalted is true, it seems we don't need to update the configurations either?
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.
Does it still mean a round for all regions?
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.
yes
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.
What if the channel is full?
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.
In the current code, it will block here until patrol region workers receive regions.