Skip to content

Add missing query transition to RUNNING state with task-level retries#11198

Merged
losipiuk merged 1 commit intotrinodb:masterfrom
losipiuk:lo/running-state-task
Mar 2, 2022
Merged

Add missing query transition to RUNNING state with task-level retries#11198
losipiuk merged 1 commit intotrinodb:masterfrom
losipiuk:lo/running-state-task

Conversation

@losipiuk
Copy link
Copy Markdown
Member

@losipiuk losipiuk commented Feb 25, 2022

Description

Add missing query transition to RUNNING state with task-level retries

Is this change a fix, improvement, new feature, refactoring, or other?

fix

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

core query engine

How would you describe this change to a non-technical end user or system administrator?

Without the fix queries were stuck in STARTING state for whole execution if task-level retries were enabled.

Related issues, pull requests, and links

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

( ) No release notes entries required.
(x) Release notes entries required with the following suggested text:

# Section
* Fix bug where queries were not transitioned to RUNNING state when task-level retries were enabled. ({issue}`11198`)

@cla-bot cla-bot bot added the cla-signed label Feb 25, 2022
Copy link
Copy Markdown
Member

@findepi findepi left a comment

Choose a reason for hiding this comment

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

Is it testable?

(see also #10360)

@losipiuk losipiuk force-pushed the lo/running-state-task branch from 68e9d41 to ad6ae79 Compare February 28, 2022 15:52
@losipiuk
Copy link
Copy Markdown
Member Author

@sopel39 @findepi added test (based on #11224)

@losipiuk losipiuk requested review from findepi and sopel39 February 28, 2022 15:53
@losipiuk losipiuk force-pushed the lo/running-state-task branch from ad6ae79 to 04ad16f Compare February 28, 2022 15:55
Copy link
Copy Markdown
Member

@findepi findepi left a comment

Choose a reason for hiding this comment

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

Last commit LGTM
rest is in some other PR.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"Listening" is redundant

@electrum
Copy link
Copy Markdown
Member

The first two commit message titles are too long. Notice how GitHub truncates them.

@losipiuk
Copy link
Copy Markdown
Member Author

losipiuk commented Mar 1, 2022

The first two commit message titles are too long. Notice how GitHub truncates them.

Yeah I know - I suck at that. I will make them shorter in the other PR this one is based on.

@losipiuk losipiuk force-pushed the lo/running-state-task branch 4 times, most recently from b687eab to dd46045 Compare March 1, 2022 13:09
@losipiuk losipiuk force-pushed the lo/running-state-task branch from dd46045 to ad43a37 Compare March 1, 2022 19:50
@losipiuk losipiuk force-pushed the lo/running-state-task branch from 8054a9a to 9f1efdd Compare March 2, 2022 05:46
@losipiuk losipiuk merged commit 91cca41 into trinodb:master Mar 2, 2022
@github-actions github-actions bot added this to the 372 milestone Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants