Skip to content

MSQ WorkerImpl: Ignore ServiceClosedException on postCounters. - #14707

Merged
cryptoe merged 2 commits into
apache:masterfrom
gianm:msq-worker-postcounters-race
Aug 2, 2023
Merged

cryptoe merged 2 commits into
apache:masterfrom
gianm:msq-worker-postcounters-race

Conversation

@gianm

@gianm gianm commented Jul 31, 2023

Copy link
Copy Markdown
Contributor

A race can happen where postCounters is in flight while the controller goes offline. When this happens, we should ignore the ServiceClosedException and continue without posting counters.

A race can happen where postCounters is in flight while the controller
goes offline. When this happens, we should ignore the ServiceClosedException
and continue without posting counters.
@gianm gianm added the Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 label Jul 31, 2023
log.debug(e, "Ignoring failure on postCounters, because controller has gone away.");
}

throw e;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

won't this just rethrow the ServiceClosedException with a extra log?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oops. Yes. Fixed. Thanks.

@gianm

gianm commented Aug 1, 2023

Copy link
Copy Markdown
Contributor Author

CI is failing due to code coverage. This particular section of code isn't super easy to write a test for. I could do it, but before tackling that I wanted to ask if people would be OK with merging this without coverage?

@georgew5656

Copy link
Copy Markdown
Contributor

CI is failing due to code coverage. This particular section of code isn't super easy to write a test for. I could do it, but before tackling that I wanted to ask if people would be OK with merging this without coverage?

works for me

@cryptoe

cryptoe commented Aug 2, 2023

Copy link
Copy Markdown
Contributor

Thanks for the PR @gianm!!

@cryptoe
cryptoe merged commit 72c151a into apache:master Aug 2, 2023
@gianm
gianm deleted the msq-worker-postcounters-race branch August 2, 2023 16:17
@LakshSingla LakshSingla added this to the 28.0 milestone Oct 12, 2023
riovic918data pushed a commit to riovic918data/druid that referenced this pull request Jun 12, 2026
…e#14707)

* MSQ WorkerImpl: Ignore ServiceClosedException on postCounters.

A race can happen where postCounters is in flight while the controller
goes offline. When this happens, we should ignore the ServiceClosedException
and continue without posting counters.

* Fix style and logic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants