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

include go routines #304

Merged
merged 5 commits into from
Jul 12, 2024
Merged

include go routines #304

merged 5 commits into from
Jul 12, 2024

Conversation

PriyaSudip
Copy link
Contributor

@PriyaSudip PriyaSudip commented Jun 25, 2024

Adding go routines to run queue agent querying in parallel.

With agents:
image
Without queue:
image

@PriyaSudip PriyaSudip marked this pull request as ready for review June 25, 2024 02:12
Copy link
Contributor

@mcncl mcncl left a comment

Choose a reason for hiding this comment

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

I wonder if we should be using more context.WithTimeout in our code to handle long API call issues.

@@ -27,18 +28,37 @@ func QueryCluster(ctx context.Context, OrganizationSlug string, ClusterID string
Queues: []Queue{},
}

queueChannel := make(chan Queue, len(q.Organization.Cluster.Queues.Edges))
errorChannel := make(chan error, len(q.Organization.Cluster.Queues.Edges))
Copy link
Contributor

Choose a reason for hiding this comment

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

This error channel doesn't seem to ever be read from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What would be the best place to send this information?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used errgroup to handle this

Copy link
Contributor

@jradtilbrook jradtilbrook left a comment

Choose a reason for hiding this comment

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

This looks okay but its all done manually. This page https://gobyexample.com/waitgroups links out to the errgroup package which will make this much simpler: https://pkg.go.dev/golang.org/x/sync/errgroup

If you search the codebase, you'll see an example of it already in use as well I think

@PriyaSudip
Copy link
Contributor Author

This looks okay but its all done manually. This page https://gobyexample.com/waitgroups links out to the errgroup package which will make this much simpler: https://pkg.go.dev/golang.org/x/sync/errgroup

If you search the codebase, you'll see an example of it already in use as well I think

Thanks @jradtilbrook as discussed I have made changes to include err group and instead of channel added array of queues and go routines based on the queue index

Copy link
Contributor

@jradtilbrook jradtilbrook left a comment

Choose a reason for hiding this comment

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

Sweet 👍

@PriyaSudip PriyaSudip merged commit 68dccc3 into 3.x Jul 12, 2024
1 check passed
@PriyaSudip PriyaSudip deleted the add/go-routines branch July 12, 2024 05:06
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.

3 participants