awslogs: Record log line insert order for sorting #24814
Conversation
|
Bump. I believe the Jenkins failure is unrelated to my change. |
| sort.Sort(byTimestamp(events)) | ||
| cwEvents := getCWEvents(events) |
There was a problem hiding this comment.
Perhaps I don't understand the problem, but wouldn't simply removing the sort.Sort(byTimestamp(events)) solve the problem?
If I read the code correctly, the events are added in the correct order (in collectBatch), so they should arrive here in the correct order.
However, due to sort.Sort(byTimestamp(events)), the events possibly get reordered, and end up in the wrong order (if two events have exactly the same time stamp)
Could you try if simply removing that sort resolves the issue?
There was a problem hiding this comment.
The PutLogEvents API has this requirement:
The log events in the batch must be in chronological ordered by their
timestamp.
The sorting here is to ensure that even on a system that is experiencing some changes in the clock (e.g., skew being corrected by NTP) that the call to PutLogEvents will still succeed.
There was a problem hiding this comment.
As just discussed on Slack; I overlooked that the timestamps are not generated in collectBatch, but taken from the msg, so they can arrive in collectBatch in the wrong order.
There was a problem hiding this comment.
Perhaps rename getCWEvents to something like unwrapEvents, but that's just a nit and may be just me 😄
Signed-off-by: Samuel Karp <skarp@amazon.com>
Fixes moby#24775 Signed-off-by: Samuel Karp <skarp@amazon.com>
|
@thaJeztah I've changed the name per your suggestion. The Jenkins failure (again) looks unrelated. |
|
LGTM |
1 similar comment
|
LGTM |
Fixes #24775
- What I did
When multiple log lines are emitted with the same timestamp, fall back to insertion order rather than allowing
sort.Sortto pick the order.- How I did it
Insertion order is recorded in a new wrapper struct internal to the logging driver.
Lessnow uses the insertion order when the timestamps are equal.- How to verify it
Unit tests pass.
- Description for the changelog
Fixed a bug in the
awslogslogging driver that could cause log lines to be recorded out-of-order under rare conditions.