Skip to content

Fix loadRule when one of the tiers had no available servers#2240

Merged
drcrallen merged 1 commit intoapache:masterfrom
metamx:fix-load-rule
Jan 11, 2016
Merged

Fix loadRule when one of the tiers had no available servers#2240
drcrallen merged 1 commit intoapache:masterfrom
metamx:fix-load-rule

Conversation

@nishantmonu51
Copy link
Copy Markdown
Member

When one of the tiers have no servers, LoadRule should ignore that tier
and continue to load/drop segments in other available tiers.

the bug causes wierd behavior with LoadRule when some tier has no servers where the segment balancer keeps on moving segments to other nodes in existing tiers but the extra segment copies are never dropped eventually leading to all the tiers getting full .

When one of the tiers have no servers, LoadRule should ignore that tier
and continue to load/drop segments in other available tiers.

the bug also causes whacky behavior with LoadRule with non existent
tier where the segment balancer keeps on moving segments to other nodes
in existing tiers but the extra segment copies are never dropped
eventually leading to all the tiers getting full .
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.

Do you need to have a logging emitter here? It seems to me it would make more sense to have either something you can capture events, or something that's a NoOp

@drcrallen
Copy link
Copy Markdown
Contributor

Minor questions from my side, ok as is if they are rejected by author 👍

@pjain1
Copy link
Copy Markdown
Member

pjain1 commented Jan 11, 2016

👍

drcrallen added a commit that referenced this pull request Jan 11, 2016
Fix loadRule when one of the tiers had no available servers
@drcrallen drcrallen merged commit ea623e4 into apache:master Jan 11, 2016
@drcrallen drcrallen deleted the fix-load-rule branch January 11, 2016 18:05
@xvrl xvrl added this to the 0.8.3 milestone Jan 11, 2016
@gianm gianm mentioned this pull request Jan 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants