Skip to content

add 'stopped' check and handling to HttpLoadQueuePeon load and drop segment methods - #5555

Merged
gianm merged 5 commits into
apache:masterfrom
clintropolis:http-load-queue-peon-fix
Apr 3, 2018
Merged

add 'stopped' check and handling to HttpLoadQueuePeon load and drop segment methods#5555
gianm merged 5 commits into
apache:masterfrom
clintropolis:http-load-queue-peon-fix

Conversation

@clintropolis

Copy link
Copy Markdown
Member

Fixes #5542 by checking if HttpLoadQueuePeon is stopped before loading or dropping a segment, and adds a test to ensure callbacks execute.

@gianm gianm left a comment

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.

Some minor comments.

public void onFailure(Throwable t)
{
try {
responseHandler.description = t.getMessage();

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.

t.toString() is more informative than t.getMessage(), since typically it includes the type of the exception but getMessage does not.

synchronized (lock) {
if (stopped) {
log.warn(
"Server[%s] failed to drop segment[%s] because load queue peon is stopped.",

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.

Instead of "failed to drop", "cannot drop" is a better log message.

synchronized (lock) {
if (stopped) {
log.warn(
"Server[%s] failed to load segment[%s] because load queue peon is stopped.",

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.

Instead of "failed to load", "cannot load" is a better log message.

@gianm gianm left a comment

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.

LGTM after fixing the log message.

if (stopped) {
log.warn(
"Server[%s] failed to drop segment[%s] because load queue peon is stopped.",
"Server[%s] cannot to drop segment[%s] because load queue peon is stopped.",

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.

“cannot to” should be “cannot”

@gianm
gianm merged commit a81ae99 into apache:master Apr 3, 2018
jon-wei pushed a commit to implydata/druid-public that referenced this pull request Apr 4, 2018
…egment methods (apache#5555)

* add stopped check and handling to HttpLoadQueuePeon load and drop segment methods

* fix unrelated timeout :(

* revert unintended change

* PR feedback: change logging

* fix dumb
@jihoonson jihoonson added this to the 0.12.2 milestone Jul 5, 2018
jihoonson pushed a commit to jihoonson/druid that referenced this pull request Jul 6, 2018
…egment methods (apache#5555)

* add stopped check and handling to HttpLoadQueuePeon load and drop segment methods

* fix unrelated timeout :(

* revert unintended change

* PR feedback: change logging

* fix dumb
gianm pushed a commit that referenced this pull request Jul 9, 2018
…egment methods (#5555) (#5960)

* add stopped check and handling to HttpLoadQueuePeon load and drop segment methods

* fix unrelated timeout :(

* revert unintended change

* PR feedback: change logging

* fix dumb
leventov pushed a commit to metamx/druid that referenced this pull request Jul 20, 2018
…egment methods (apache#5555) (apache#5960)

* add stopped check and handling to HttpLoadQueuePeon load and drop segment methods

* fix unrelated timeout :(

* revert unintended change

* PR feedback: change logging

* fix dumb
@clintropolis
clintropolis deleted the http-load-queue-peon-fix branch August 6, 2018 21:29
riovic918data pushed a commit to riovic918data/druid that referenced this pull request Jun 12, 2026
…egment methods (apache#5555)

* add stopped check and handling to HttpLoadQueuePeon load and drop segment methods

* fix unrelated timeout :(

* revert unintended change

* PR feedback: change logging

* fix dumb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants