Filebeat: Allow stateless and stateful ACKer on the global ack handler #7214
Conversation
ph
added
review
needs_backport
This commit introduces a change in how filebat handle ACK by default, before the ACK was using the private field of the event to retrieve a state. The updated state was sent to the registrar, and the registrar was finalizing the ACK. But with the introduction of the TCP/UDP and the Redis input, the events don't have any state attached. So in that scenario, Filebeat was not correctly acking these events to some wait group. The ACKer was modified to handle both stateless (default) and stateful events, when stateful is required, the states are sent to the registry otherwise, the waiting groups are directly updated. Fixes: elastic#7202
There was a problem hiding this comment.
The code looks good. However, I am a bit worried about updating the WaitGroup in multiple places. I would delegate the full responsibility of updating it either to the registrar or to the eventACKer. I would prefer to do it in the registrar, if that's possible. WDYT?
|
@kvch I see your point and it is a good comment. I think my original goal was If the type doesn't require a registrar like the Redis, TCP or UDP we should not delegate the ack to the specialize Acker. I see a world where a source could provide their own Acker. So actually the acker type now acts as a multiplexer of ACKs and delegates them the right acker using the type of I think maybe the problem you see is the ACKER is not generalized enough, would you be OK if we make the ACKER more generic but Keep the two acking strategy? Pseudocode |
|
I think this could be removed because of some changes in the pipeline. Also, I am not too worried about possible problems:
|
tsg
added
v6.3.0
and removed
needs_backport
|
Changes LGTM. The finishedLogger and the eventACKER decreasing the WaitGroup is a filebeat solution only. It is used to implement the As is done in this PR, stateless events should not be routed via the registry, as the registry introduces another potential delay/backpressure, even if not required. |
elastic#7214) * Filebeat: Allow stateless and stateful ACKer on the global ack handler This commit introduces a change in how filebat handle ACK by default, before the ACK was using the private field of the event to retrieve a state. The updated state was sent to the registrar, and the registrar was finalizing the ACK. But with the introduction of the TCP/UDP and the Redis input, the events don't have any state attached. So in that scenario, Filebeat was not correctly acking these events to some wait group. The ACKer was modified to handle both stateless (default) and stateful events, when stateful is required, the states are sent to the registry otherwise, the waiting groups are directly updated. Fixes: elastic#7202 * changelog (cherry picked from commit b9d2150)
#7214) (#7258) * Filebeat: Allow stateless and stateful ACKer on the global ack handler This commit introduces a change in how filebat handle ACK by default, before the ACK was using the private field of the event to retrieve a state. The updated state was sent to the registrar, and the registrar was finalizing the ACK. But with the introduction of the TCP/UDP and the Redis input, the events don't have any state attached. So in that scenario, Filebeat was not correctly acking these events to some wait group. The ACKer was modified to handle both stateless (default) and stateful events, when stateful is required, the states are sent to the registry otherwise, the waiting groups are directly updated. Fixes: #7202 * changelog (cherry picked from commit b9d2150)
elastic#7214) (elastic#7258) * Filebeat: Allow stateless and stateful ACKer on the global ack handler This commit introduces a change in how filebat handle ACK by default, before the ACK was using the private field of the event to retrieve a state. The updated state was sent to the registrar, and the registrar was finalizing the ACK. But with the introduction of the TCP/UDP and the Redis input, the events don't have any state attached. So in that scenario, Filebeat was not correctly acking these events to some wait group. The ACKer was modified to handle both stateless (default) and stateful events, when stateful is required, the states are sent to the registry otherwise, the waiting groups are directly updated. Fixes: elastic#7202 * changelog (cherry picked from commit 31c46c9)
This commit introduces a change in how filebeat handle ACK by default,
before the ACK was using the private field of the event to retrieve a
state. The updated state was sent to the registrar, and the registrar
was finalizing the ACK.
But with the introduction of the TCP/UDP and the Redis input, the events
don't have any state attached. So in that scenario, Filebeat was not
correctly acking these events to some wait group.
The ACKer was modified to handle both stateless (default) and stateful
events, when stateful is required, the states are sent to the registry
otherwise, the waiting groups are directly updated.
Fixes: #7202