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

DM-39464: Broadcasting query completion/cancellation events to Qserv workers #792

Merged
merged 8 commits into from
Jun 20, 2023

Conversation

iagaponenko
Copy link
Contributor

No description provided.

Copy link
Contributor

@jgates108 jgates108 left a comment

Choose a reason for hiding this comment

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

We definitely need something to say "this query is done, clean up" and this looks like the right mechanism to do that. I think it needs to include the czar ID in the messaging and handling of messages as going back to it later could be painful.

src/czar/Czar.cc Show resolved Hide resolved
src/sql/SqlResults.cc Outdated Show resolved Hide resolved
src/xrdreq/ChunkGroupQservRequest.cc Show resolved Hide resolved
src/xrdreq/QservRequest.cc Outdated Show resolved Hide resolved
Eliminated unused definitions. Better names and return types for some
methods.
The new implementation is based on the smart pointer to self
be stored by the Qserv request classes. It would guarantee the life
span of the request objects while request processing is still going
on. Otherwse, there is a chance of running into crashes. The request
cancellaton method has been added to replace SSI's Finished(true)
to do a proper clean up of the stored pointer.
Added the corresponidng requests handler at workers. Added a new
transient class representing the API for sending query management
requests. Added a command-line tool to test requests.
…ation

Added Czar configuration options to disable this feature if needed.
The options will exist for some time before the new code will be proven
to work w/o any side effects to the query processing.
Copy link
Contributor

@fritzm fritzm left a comment

Choose a reason for hiding this comment

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

LGTM; thanks -- don't forget to rebase!

@iagaponenko iagaponenko merged commit 962db0b into main Jun 20, 2023
@iagaponenko iagaponenko deleted the tickets/DM-39464 branch June 20, 2023 22:32
@iagaponenko iagaponenko changed the title Tickets/dm 39464 DM-39464: Broadcasting query completion/cancellation events to Qserv workers Aug 18, 2023
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