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

Optimize REST API workload by using Session and slowing down polling loop #220

Merged
merged 2 commits into from
Apr 10, 2024

Conversation

chulucninh09
Copy link
Contributor

@chulucninh09 chulucninh09 commented Mar 13, 2024

Summary

The cursor implementation was using while True loop to poll for result from REST API of Dremio. Without throttling, it will jam the server easily, especially when using a high number of thread count. Since most SQL jobs need a few second to run, looping too fast to get the result is unessesary.

Also, in REST client implementation, using get, post, delete directly from requests is inefficient because it will create new HTTP session with the server everytime the polling is called.

Description

  1. Use request.Session instead of vanila requests to utilize connection pooling, so optimize networking performance
  2. Add time.sleep in while True loop to slow down the polling

Changelog

  • Added a summary of what this PR accomplishes to CHANGELOG.md

@chulucninh09 chulucninh09 marked this pull request as ready for review March 13, 2024 18:53
@chulucninh09
Copy link
Contributor Author

Hi @ravjotbrar, can you comment on this PR and make this progress

@ravjotbrar
Copy link
Contributor

Apologies for the late response @chulucninh09. I will try to get to this tomorrow.

@chulucninh09
Copy link
Contributor Author

Hi @ravjotbrar, can I get approval on this or any comment for this PR?

Copy link
Contributor

@ravjotbrar ravjotbrar left a comment

Choose a reason for hiding this comment

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

LGTM!

@ravjotbrar ravjotbrar merged commit 328df7a into dremio:main Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants