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

Added heartbeat to cloud download #173

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rcypher-databricks
Copy link
Contributor

While downloading cloud files a heartbeat will query operation status at regular intervals to keep the connection alive.
Implemented heartbeat type which takes an instance of driver.Pinger and calls it at a regular interval.
Implemented statusGetter type which implements driver.Pinger by calling GetOperationStatus()
Updated NewBatchLoader() to take an argument of type driver.Pinger and updated batchLoader to instantiate a heartbeat instance using the pinger.
Updated NewRows() to create an instance of statusGetter and updated the various constructor functions to take a diver.Pinger argument.
Added interface fetcher.Overwatch. NewConcurrentFetcher() now takes an Overwatch argument. An instance of Overwatch will be started/stopped in conjunction with fetcher start/stop.

While downloading cloud files a heartbeat  will query operation status at regular intervals to keep the connection alive.
Implemented heartbeat type which takes an instance of driver.Pinger and calls it at a regular interval.
Implemented statusGetter type which implements driver.Pinger by calling GetOperationStatus()
Updated NewBatchLoader() to take an argument of type driver.Pinger and updated batchLoader to instantiate a heartbeat instance using the pinger
Updated NewRows() to create an instance of statusGetter and updated the various constructor functions to take a diver.Pinger argument.
Added interface fetcher.Overwatch.  NewConcurrentFetcher() now takes an Overwatch argument. An instance of Overwatch will be started/stopped in conjunction with fetcher start/stop.

Signed-off-by: Raymond Cypher <raymond.cypher@databricks.com>
Copy link
Contributor

@andrefurlan-db andrefurlan-db left a comment

Choose a reason for hiding this comment

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

Core review: can you be triple sure that we're not leaking go routines?

Feature review: why is this needed? how can the session timeout while we're downloading results?

@yunbodeng-db
Copy link
Contributor

yunbodeng-db commented Nov 21, 2023

Core review: can you be triple sure that we're not leaking go routines?

Feature review: why is this needed? how can the session timeout while we're downloading results?

During cloud fetch, the client downloads cloud files without the involvement of DBSQL or cluster so the server may shut down due to inactivity. Heartbeat thread would keep it alive.

@rcypher-databricks
Copy link
Contributor Author

But as I said before. The operation/session are open until the client reads the entire result set or manually closes. So the server shouldn't be shutting down due to inactivity.

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