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

Cherry-pick #5532 to master: Fix panic if event is dropped by queue #5536

Merged
merged 1 commit into from
Nov 8, 2017

Conversation

tsg
Copy link
Contributor

@tsg tsg commented Nov 7, 2017

Cherry-pick of PR #5532 to master branch. Original message:

Resolves: #5524

  • Only increase event seq-no if event has been pushed
  • Fix invalid ACK list if empty list is given to concat
  • Only execute ack handlers if number of events being ACKed > 0
  • Add missing debug log when collecting first ACK in ACK list

- Only increase event seq-no if event has been pushed
- Fix invalid ACK list if empty list is given to concat
- Only execute ack handlers if number of events being ACKed > 0
- Add missing debug log when collecting first ACK in ACK list

(cherry picked from commit 470b3d3)
Copy link
Contributor

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

LGTM as the identical changes went into master. WFG.

@@ -53,13 +53,20 @@ func (l *ackLoop) run() {
count, events := lst.count()
l.lst.concat(&lst)

// log.Debugf("ackloop: scheduledACKs count=%v events=%v\n", count, events)
// log.Debug("ACK List:")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this commented out on purpose?

Copy link

Choose a reason for hiding this comment

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

Yes. debug creates quite some noise if not required. There are a many debug statements in this package being commented out and only selectively uncommented when required (when debugging locally).

@urso
Copy link

urso commented Nov 8, 2017

Jenkins, test it.

@ruflin ruflin merged commit ee862c0 into elastic:master Nov 8, 2017
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