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
Distributed mode phase 1 #98
Distributed mode phase 1 #98
Changes from 38 commits
37a9024
4e722c4
46442b3
8c54c37
1ac484a
4dd10ea
007124b
f099f7d
8ed44d1
0a3a22b
7e96fcc
b595242
64a804d
602383d
5f168e4
47f365b
0fff565
9dcc536
f25520b
2dbe18c
e7d4922
01ace68
0f13923
969565e
2abbeed
b862321
709a3de
9b0567b
c4b19b6
3e76c73
66316db
75e3dad
ca66702
6812909
1a46d94
b52e014
c2de939
7d81fd0
928ae0f
1d17ce2
6f4e5d9
fafce57
e1adab8
6afd126
4cbd4ad
f71f135
354fdb2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
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.
I've thought there would be no data integrity issues caused by race condition based on this separation, tho. coz all processing is about to wipe out unnecessary resources. do you have any concerns in terms of multi-threaded processing on the controller?
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.
hmm, that depends on how to define mutli-threaded support.
If we look at the whole system(controller, api, etc), I don't have concern as this will be the same as before just process/threads are running either as a whole(multi-thread) or separately(multi process). The underlying logic does not change.
If we look at just the controller, it should not have multiple threads/process because essentially it's just a checker with a forever loop and getting the states from either the DB or a k8s cluster. If we have another checker(by using multi-threading), we need to implementing something like worker queue to split the work, which right now it does not exist. So having another checker will essentially doing the same work and it could be problematic so we should avoid that.
In terms of the scalability of the controller, I don't have great concern for it for now because I expect it to be a light weight component(most of its time is likely spending on IO operations anyway)
Hopefully I answered your questions.
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.
understood. and i'm 80% sure what you've described above. but not 100%. so, it's better to visualize all operations which controller and api have provided (especially goroutines). otherwise it's a bit difficult to see entire picture. I'm about to do that on the confluence page, tho. it's a bit time-consuming and will take some time until illustrating everything. I'll make a review request to you piece by piece.