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

AckingProtocolV1 only ever sends one ACK #26

Open
rdharrison2 opened this issue Jun 9, 2016 · 1 comment
Open

AckingProtocolV1 only ever sends one ACK #26

rdharrison2 opened this issue Jun 9, 2016 · 1 comment

Comments

@rdharrison2
Copy link

A while ago I wrote a Python lumberjack client for logstash-1.4.2 which uses ruby-lumberjack version 0.0.22. I recently upgraded to logstash-2.3.2 and this pulled in version 0.0.26 which seems to no longer send more than one ACK.

I've debugged it and have found that there is a bug in the AckingProtocolV1 logic which means it will only ever send one ACK. AckingProtocolV1 sets the sequence number for the next ACK as @next_ack in method ack? but this is only set on the first time when @next_ack is nil, and not updated when the server has sent an ACK. The only time it would be updated would be if the client sent another window_size frame (which mine doesn't).

@nhjm449
Copy link

nhjm449 commented Jun 9, 2016

I just started using the lumberjack logstash input plugin and was surprised to run into this issue.

But a simple patch helps:

--- lib/lumberjack/server.rb       2016-06-09 12:26:04.825827010 -0500
+++ lib/lumberjack/server.rb       2016-06-09 12:31:21.257811427 -0500
@@ -392,7 +392,12 @@
       # The first encoded event will contain the sequence number 
       # this is needed to know when we should ack.
       @next_ack = compute_next_ack(sequence) if @next_ack.nil?
-      sequence == @next_ack
+      if sequence == @next_ack
+        @next_ack = compute_next_ack(sequence + 1)
+        true
+      else
+        false
+      end
     end

     def ack_frame(sequence)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants