Closed
Conversation
Closed
yevgenypats
added a commit
to cloudquery/cloudquery
that referenced
this pull request
Oct 22, 2022
yevgenypats
added a commit
to cloudquery/cloudquery
that referenced
this pull request
Oct 22, 2022
5 tasks
yevgenypats
added a commit
that referenced
this pull request
Oct 30, 2022
Trying to split this PR: #298 into smaller bits. Metrics: - This introduce Metrics as stateful struct as it should be (instead of prop drilling) + GetMetrics Protobuf API Scheduler: - I believed I've hit a bug/deadlock in our current scheduler and also for metrics to work I needed to move the scheduler from `schema` to `plugins` so I added that in the same PR. Currently the user will specify only one variable `concurrency` and the scheduler will decide on how to split it between levels. For simplicity I kept it the same way as before with concurrency for only the first level. Concurrent DFS will make sure there are no deadlocks and memory is always kept at O(goroutines) and o(h) (where h is height). Co-authored-by: Erez Rokah <erezrokah@users.noreply.github.com>
Contributor
Author
yevgenypats
added a commit
that referenced
this pull request
Oct 31, 2022
This add a type system to CloudQuery SDK. This is mandatory to support multiple destinations. Also, this found quite a few bugs where we were sending random stuff over the wire without any validation apart from maybe when we were hitting the database and then failing batches all together. In CloudQuery type system I used heavily https://github.com/jackc/pgtype and kept the license and the copyright in it's own package `cqtype`. This is a continue of this PR #298 where I split it into this one and #318 Co-authored-by: Erez Rokah <erezrokah@users.noreply.github.com> Co-authored-by: Herman Schaaf <hermanschaaf@gmail.com>
This file contains hidden or 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
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.
While fixing the PostgreSQL performance bugs found quite a few issues and rabbit holes we need to take care of (This is the quick fix around order of columns which should be good for now - cloudquery/cloudquery#2887).
Few change are required to write protocol:
Resolverfunction is in the incorrect place so I had to move it topluginsas otherwise things like stats/metrics wont be possible to implement.Because those are major changes in the protocol including incompatible protobuf changes, I changed the application level to 2.
Also found number of concurrency issues and graceful shutdown not working that is fixed in this PR.
Another big missing piece I found that is in this PR:
Also, Implemented concurrent dfs scheduler to make sure memory is limited by O(h) (h is height of the tree/tables).