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

fix empty queue condition for queue shutdown drain #7575

Merged
merged 1 commit into from
Jul 18, 2017

Conversation

colinsurprenant
Copy link
Contributor

@colinsurprenant colinsurprenant commented Jun 29, 2017

This is a proposal to fix the second item of #7568 which aim at an acceptable fix to satisfy ReadClient#empty? without changing the semantic of Queue.isFullyAcked()

The current implementation of isFullyAcked() for both the Queue and Page classes does not work to satisfy the ReadClient#empty? condition, see #7570.

  • add missing unit tests and specs.

@colinsurprenant
Copy link
Contributor Author

Once this is merged we should move forward with #7583

@colinsurprenant colinsurprenant changed the title [WIP] fix empty queue condition for queue shutdown drain fix empty queue condition for queue shutdown drain Jul 6, 2017
@elasticsearch-bot elasticsearch-bot self-assigned this Jul 6, 2017
@colinsurprenant
Copy link
Contributor Author

ping @guyboertje

Copy link
Contributor

@guyboertje guyboertje left a comment

Choose a reason for hiding this comment

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

LGTM

@colinsurprenant colinsurprenant force-pushed the fix/queue_empty branch 2 times, most recently from d983867 to 7e521c9 Compare July 18, 2017 20:31
page test

queue test

move is_empty? at queue level

new wrapped acked queue spec
@colinsurprenant colinsurprenant merged commit aedf397 into elastic:master Jul 18, 2017
colinsurprenant added a commit that referenced this pull request Jul 18, 2017
page test

queue test

move is_empty? at queue level

new wrapped acked queue spec
colinsurprenant added a commit that referenced this pull request Jul 18, 2017
page test

queue test

move is_empty? at queue level

new wrapped acked queue spec
@colinsurprenant
Copy link
Contributor Author

merged in master, 5.x and 5.5

@seang-es
Copy link

seang-es commented Nov 10, 2017

present in 5.5.2+

@colinsurprenant
Copy link
Contributor Author

@seang-es if you mean fixed in 5.5.2+ then yes, the fixed was merged in the 5.5 branch and included starting with 5.5.2. Labels are missing here, I will label it.

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.

4 participants