feat(FIR-42854): Server side async query execution #415
Conversation
ptiurin
changed the title
|
SQ failing here is due to our async/sync duplication. I'm not sure what else can be done so IMO this can be ignored. |
| async def _handle_query_execution( | ||
| self, query: str, timeout_controller: TimeoutController, async_execution: bool | ||
| ) -> None: | ||
| query_params: Dict[str, Any] = {"output_format": JSON_OUTPUT_FORMAT} |
There was a problem hiding this comment.
Should we check the case for when not async_execution and query_params.get("async") is True?
There was a problem hiding this comment.
That opens a can of worms that we are still debating. At the moment enforcing this is out of scope https://packboard.atlassian.net/browse/FIR-42854
Set async=true will probably be not allowed on the backend altogether.
| "_next_set_idx", | ||
| "_set_parameters", | ||
| "_query_id", | ||
| "_query_token", |
There was a problem hiding this comment.
Again, I have a feeling we should only have this logic and these field for ConnectionV2 only.
There was a problem hiding this comment.
I've refactored to have execute_async in CursorV2 and V1 to throw an error if this method is attempted to be used. Other internal methods are a bit difficult to separate since there's plenty of shared logic and we have them mostly in the base class. Fully separating them I feel like will lead to a "ping-pong" between base and a concrete class. Happy to sync if you feel strongly about this and we can have a look.
| assert await connection.is_async_query_running(token) == True | ||
| assert await connection.is_async_query_successful(token) is None | ||
| finally: | ||
| await cursor.execute(f"DROP TABLE {table_name}") |
There was a problem hiding this comment.
Just curious what will happen if we have a running insert in async mode and will try to drop the table? Probably it should just wait for a table lock to get released or something
There was a problem hiding this comment.
Tested it via the API. INSERT INTO dummy SELECT checksum(*) FROM GENERATE_SERIES(1, 250000000000) would keep on running, despite table dummy no longer being there. Though if there's continuous insert it might behave differently, but I have no easy way of reproducing that.
| DEFAULT_API_URL: str = "api.app.firebolt.io" | ||
| PROTOCOL_VERSION_HEADER_NAME = "Firebolt-Protocol-Version" | ||
| PROTOCOL_VERSION: str = "2.1" | ||
| PROTOCOL_VERSION: str = "2.3" |
There was a problem hiding this comment.
Ok I see we did a jump here. Can you please make sure this one is also ok?
https://packboard.atlassian.net/browse/FIR-37204
There was a problem hiding this comment.
We have tests that confirm the behaviour described in there. I've just rerun them to be sure and everything passes.
|
2 participants
Following the design doc in https://docs.google.com/document/d/1KfFxCgDxeqyll9dth10QfpUVKjo-aX7yL4Mwn00CZps/