Add retry to VictoriaLogs data source #3654
Conversation
If the original HTTP get for the tail endpoint is successful, but then the connection is lost, no retry is done. I updated the Tail method to also retry in this case.
Upon review, the added code was pretty different in the approach used to keep retrying compared to the approach for the QueryRange method. I updated the method to create a new doTail that has the same style as doQueryRange and updated Tail to use it. This has the following effects: - doTail will keep trying after losing a connection - the retry interval will grow (with an upper limit) and shrink (with a lower limit) as connections are made and broken - the time in the request is updated to avoid overlapping with previous data that was returned (missing in the first fix)
Bringing in changes from master.
Keeping up with changes from origin
The use of the ticker was unnecessary. I updated doTail to use a backoff interval with time.After.
|
@thebondo: There are no 'kind' label on this PR. You need a 'kind' label to generate the release automatically.
DetailsI am a bot created to help the crowdsecurity developers manage community feedback and contributions. You can check out my manifest file to understand my behavior and what I can do. If you want to use this for your project, you can check out the BirthdayResearch/oss-governance-bot repository. |
|
@thebondo: There are no area labels on this PR. You can add as many areas as you see fit.
DetailsI am a bot created to help the crowdsecurity developers manage community feedback and contributions. You can check out my manifest file to understand my behavior and what I can do. If you want to use this for your project, you can check out the BirthdayResearch/oss-governance-bot repository. |
|
/kind fix |
|
Hello @thebondo, do you think you will have some time to continue work on this PR? |
|
Hello @zekker6. Yes, I can still contribute on this. I have a question about the approach for the final fix. As I understand it, the tail endpoint is expected to stay open and report log entries when received (after some possible delay, which helps keep entries sorted by time). However, the connection is sometimes lost. Ideally, the tail consumer would like to see all log entries exactly once. However, there does not seem to be a good way to start a new tail query that will both
The solution I have so far is to keep track of the latest timestamp seen while the tail connection is open. If the connection is lost, then compute This has the drawback that it can potentially provide a log entry more than once. From looking at the VictoriaLogs source, the start time for the tail interval appears to be calculated as Does this all sound correct to you? If so, I will update the code to use |
Keeping the fork up to date with the origin repository.
The tail query endpoint does not support start, but start_offset. I updated the doTail method to use this parameter, and calculate the required value each time the query is attempted based on the desired start time for the results returned from the query.
|
@thebondo Thank you for an update!
Yes, this sounds correct to me. |
The current VictoriaLogs data source will exit if the data source goes away once a tail based connection has been established. This causes the whole crowdsec service to exit as well.
This update fixes the Tail method on the VictoriaLogs client so that it tries to reconnect with a backoff if the connection is lost.
Resolves #3653